Integration with 3Dconnexion input devices #396
Conversation
Signed-off-by: Patryk Skowroński <[email protected]>
avogadro/tdxcontroller.h
Outdated
| This source code is released under the 3-Clause BSD License, (see "LICENSE"). | ||
| ******************************************************************************/ | ||
|
|
||
| #pragma once |
There was a problem hiding this comment.
For consistency, can we please stick to #ifndef like the other headers? Thanks.
avogadro/menubuilder.h
Outdated
There was a problem hiding this comment.
If it's useful API, just add it, not #ifdef
avogadro/icons/3dx_pivot.png
Outdated
There was a problem hiding this comment.
This seems kind of small. Does this work okay on a high-resolution / Retina screen?
There was a problem hiding this comment.
Yes, it is fine, this icon is meant to be so small.
|
@pskowronskiTDx - comments on the requested changes? I'd certainly love to merge these. |
Signed-off-by: Patryk Skowroński <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| } | ||
|
|
||
| for (auto& pNode : pNode->m_children) { |
Check warning
Code scanning / Cppcheck (reported by Codacy)
Local variable 'pNode' shadows outer argument Warning
| } | ||
| } | ||
|
|
||
| for (auto& pNode : pNode->m_children) { |
Check warning
Code scanning / Cppcheck (reported by Codacy)
Variable 'pNode' can be declared as reference to const Warning
| } | ||
|
|
||
| for (auto& pNode : pNode->m_children) { | ||
| if (pNode->m_nodeName == childName) { |
Check warning
Code scanning / Cppcheck (reported by Codacy)
Consider using std::find_if algorithm instead of a raw loop. Warning
| @@ -0,0 +1,3 @@ | |||
| This directory contains part of the the 3Dconnexion 3DxWare_SDK v4 | |||
| The full sdk can be obtained from https://3dconnexion.com/software-developer-program/ | |||
There was a problem hiding this comment.
Is there a good way to keep this up-to-date (e.g., when v5 comes out?)
There was a problem hiding this comment.
No, as we are shipping some SDK's headers with the solution they will need to be updated manually when a new release comes out.
avogadro/tdxcontroller.cpp
Outdated
|
|
||
| std::vector<bool> flags; | ||
|
|
||
| m_pGLRenderer->scene().getBoundingBox(extents.min.x, |
There was a problem hiding this comment.
See, I think you can get this from the molecule here and avoid having to save all those spheres in the render pipeline.
Just implement core/molecule.*:getBoundingBox() instead.
avogadro/tdxcontroller.cpp
Outdated
| for (uint32_t i = 0u; i < (*m_ppMolecule)->atomCount(); i++) | ||
| flags[i] = (*m_ppMolecule)->atomSelected(i); | ||
|
|
||
| m_pGLRenderer->scene().getBoundingBox(extents.min.x, |
There was a problem hiding this comment.
Ditto. You're already going through the atoms in the loop above.
for (Index i = 0; i < molecule->atomCount(); ++i) {
if (molecule->atomSelected(i) {
// calculate the bounding box directly
}
}
avogadro/tdxcontroller.cpp
Outdated
|
|
||
| long Avogadro::TDxController::GetIsSelectionEmpty(navlib::bool_t &empty) const | ||
| { | ||
| if (*m_ppMolecule == nullptr) |
There was a problem hiding this comment.
Use Molecule:isSelectionEmpty()
|
Looks pretty close - sorry for the delay. If you could run clang-format on your changed files in |
|
Thank you for your sugesstions, I have introduced some changes. The double pointer for Molecule object was used to keep track on the active molecule (selected in the explorer). However, I have reworked it by introducing updateMolecule method, connected to the moleculeChanged signal. The pointer to the active molecule updates automatically now, without referencing "pp" anymore. |
Signed-off-by: Patryk Skowroński <[email protected]>
|
Assuming this builds cleanly, I'll merge this as well. Thanks! |
|
Sorry for the delay - I thought I had previously merged this before leaving for a conference. I'll update the clang-format myself in a new pull. Thanks! |
|
Congrats on merging your first pull request! 🎉 Thanks for making Avogadro better for everyone! |
|
No problem, I am happy to see the changes finally merged. Hope the integration will be useful. Thank you as well! |
Developer Certificate of Origin
Version 1.1
Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129
Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file.
I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.