RFC: Modification of the manipulation tool

Hi everyone,
I want to look into improving the general molecule editing workflow within Avogadro 2 and based on my experience so far, the manipulation tool is currently implemented in a rather rudimentary way.

Trying to be a good Open Source citizen I figured I might give a shot at improving the situation myself, but before I dive into the code, I wanted to present my ideas here and gather some feedback on them first.

The first improvement that I have in mind would be to introduce modification “gizmos” as is often encountered in other 3D editors (e.g. game engines). See for instance

Then for actually performing any kind of manipulation (be it rotation or translation) one would have to use these gizmos.

In addition, I think the center of rotation should
a) be displayed
b) be customizable

As a further extension I thought that it could be beneficial to allow the user to define arbitrary help-lines and/or -planes (e.g. by selecting two atoms and then creating a line that connects those two or by selecting three atoms and creating the plane which they form) which can be used to restrain the manipulation (e.g. keep atoms within the plane, move them along the line, rotate around axis defined by this line, etc.). Note however that these helper geometries should also be definable by arbitrary points and not necessarily by existing atoms or bonds (that way one could e.g. define a rotation axis that goes through the center of mass between three atoms, perpendicular to the plane that they span).

Furthermore, I am wondering whether it really makes sense to keep the bond-centric manipulation tool as a separate tool. I could imagine that it might be beneficial to unify all currently available editing functionality into a single tool and then extend that tool by the things mentioned above.

The result would be a single modification tool that should cover free-form modifications as well as bond-centric manipulations.
I believe that for a user this would greatly improve the current workflow.

While we’re at it, I think there would also be value in essentially assimilating the functionalities of the select tool into this unified tool as well.
By using transformation gizmos, we’d have to have a way of selecting atoms and bonds anyway and having to switch the used tool for this every time seems rather cumbersome.

So I think in the end, what I would propose is a modification tool that combines and extends the functionality of the current tools manipulate, select and bond-centric manipulation.
Concretely I would imagine a typical workflow to be the following:

  • Click on an atom. This selects the atom and shows the combined translation/rotation gizmos (though for a single atom rotation probably doesn’t make sense, so we could restrict ourselves to showing only translation gizmos)
  • Alternatively drag the mouse to select a group of atoms. Show the translation/rotation gizmos and use the center of mass as the default rotation pivot point (and visibly mark it!)
  • From here the user could either drag on the translation gizmo (arrow) to move the entire group of atoms along the respective axis or drag on the center of the gizmo to freely move the selection parallel to the camera’s plane
    Alternatively the user could drag on one of the rotation gizmos in which case the selection is rotated around the respective respective axis going through the rotation pivot
    Furthermore it would be possible to move the rotation pivot itself by either dragging it through space or by letting it snap to a specific atom or bond
  • There should be the possibility of changing the used axis system for the abovementioned processes. The choices should include “global space” aka the cartesian coordinate axis, “local space” which should somehow be defined by the bonds coming out of the selected atom (though there is some ambiguity if there are more than three bonds) as well as some way of essentially freely defining the coordinate system (similarly to how the manipulation plane can currently be changed in the bond-centric manipulation tool).
  • If instead of an atom, the user clicks on a bond, select that bond instead and provide options for changing the bond-length, the bond- and/or dihedral angle (aka: Essentially reuse what the bond-centric manipulation tool currently does)

The ability to create helper lines and planes might also be worth to keep around as a separate tool as these things could also be interesting for e.g. the measure tool (measure the angle between two arbitrary lines that the user created, without those necessarily having to be defined as passing through two atoms).

Please let me know what you think of this idea and/or if this needs more clarification.

1 Like

Welcome - and thanks a lot for the thoughtful suggestions!

One quick comment is that Avo1 had “eye candy” similar to your description – it just hasn’t been ported over to Avo2.

Agreed that this makes the manipulation tools less intuitive, and that the center of rotation should be tailorable (e.g., an atom, the center of the molecule or selection, or the origin).

I’m always a little reluctant to remove tools, since I’ve seen new users tend to think in terms of “modes” and if anything, there’s some benefit to more tools / modes (e.g. “brush to select” or a “lasso select” or the lines / planes you mention).

Otherwise, I think you’re describing a bunch of useful improvements for the manipulate tool(s).

I’m heading out, but I’ll comment more this weekend. If you’d like some suggestions on the code / where to poke and prod.

A first suggestion might be to bring back the various “eye candy” rendering - either as you draw it or in the Avogadro v1 code.

Happy to help and thanks for the great suggestions!

Right, I remembered having seen some gizmos in Avogadro back in the days ^^

Well then for the time being maybe the way to go would be to add a new “super-modification tool” (we’ll require a better name of course xD) and then time will tell if users will still use the other (old) modification tools or if they stick exclusively to the new one. In the latter case the old tools could still be removed afterwards.
It only kinda duplicates tool scopes (aka: what tool can be used for what) but at least for a time this seems to not be too bad.

Yes that would be much appreciated input. I already poked around in the source code a little and from my preliminary exploration it seems that it would probably be useful to check out the implementation of the existing manipulation tools at avogadrolibs/avogadro/qtplugins at master · OpenChemistry/avogadrolibs · GitHub.
However, I was thinking that at least for the lines/planes feature there is probably the need to create some (semi)-permanent additions to the model such that they persist even when closing the current tool and that they are selectable and usable by other tools. I was thinking that maybe we could work with some sort of dummy-atoms that could be used to define points in space, lines and planes. But I assume that this will require some code changes outside the plugins directory.

Could you link to the respective code parts in Avo1. If nothing else one could at least take inspiration from there - probably even shamelessly steal some parts of it.

I’m literally on the road until Sat evening US time, but I’ll post some pointers then.

Of course. For now, you could obviously copy the manipulate in the qtplugins directory to supertool (hah, my auto-correct called that “supercool” :wink: ) … or something like that. Coming up with tool icons is also surprisingly challenging…

Dummy atoms are easy. I created a script for defining a centroid:

It’s probably a good idea to actually create this and center-of-mass as actual C++ plugins. If you want to code those up (e.g., a centroid directory with center-of-geometry and center-of-mass) it might be an easy first step.

For lines, a first step would probably just be to add a begin and end point as dummy atoms and then a “bond” between them. On the other hand for planes, we’d need some sort of render for that (e.g., the symmetry code has symmetryscene.cpp and defines mirror planes).

Yep, there’s actually an issue for it: Port "eye candy" from Avogadro v1 · Issue #222 · OpenChemistry/avogadrolibs · GitHub

Code is here:

The more “modern” method would be to create a line strip like the class QuadOutline code in bondcentrictool.cpp

Edit: Actually, a better choice to try is ArcStrip currently in bondcentrictool.cpp which requires:

  • origin of the circle
  • start of the arc (e.g., to one side of the arc)
  • degrees of the arc (e.g., 180° or 270°)
  • normal to the plane
  • line width

Another good task would be to take some of these LineStripGeometry classes out of plugins so they’re more obvious. I’ll see if I can do that in the next day or three.

Awesome - thanks for the hints!
I’ll probably not be able to actually start working on this for about a week or so, but then I should be able to give this a shot.

In that case we might as well do a more general renderer that can render lines and points as well. That way we can completely decouple those from regular atoms and bonds. My guess would be that otherwise there might be some upcoming issues if we want to style atoms and bonds differently from generic points and lines.

:man_shrugging: In my experience it’s always good to start with “whatever works” and for points, dummy atoms are probably a good starting point. They’re often understood by other programs and often useful for other tasks (e.g., drawing a ferrocene to indicate the “bond” between the iron atom and the ring).

In any case, I’ll be happy to help break the project into some manageable pieces and we can keep in touch as you get going.

In the meantime, thanks for the great suggestions and I look forward to seeing this take shape!

I set a reminder bookmark - you said that you might be able to try some things out a bit later in August.

Need any pointers / help?

I added code for centroids, and I’d be happy to help break the project into smaller pieces, maybe something like:

  • adding the “eye candy” from Avo1
  • adding code to get a best-fit for lines / rays (since centroids and best-fit planes already exist)
  • start with a copy of the manipulate tool to add your “drag to select a group” code
  • etc.

Yeah, I was already looking into the code a bit but got deterred by the uncommon layout of the repositories and the fact that essentially the components can’t be built on their own (only embedded in the super-repo). That seems to make working with the code rather tedious.
What is the recommended way to work with this structure? Normally I would fork the avogadrolib repo, create a new feature branch in my fork and start messing around from there. But since that repo doesn’t build on its own, it seems like the only way to go is to go through the super-project, but there all the sub-modules point to the upstream repos.

Also I saw that when compiling the code, there are tons and tons of compiler warnings that are emitted - beyond the point where one could hope to fix the warnings with a reasonable amount of effort. To me this is usually a clear sign of code-smell…

Plus there has been a lot going on in another project that I am working on (we had a new release and several new issues have been encountered in it), so I didn’t have the time to further look into this.

The “openchemistry” repo is simply designed to make sure you have the third party packages and that you compile both avogadrolibs and avogadroapp.

The sub-directories are simply git repos themselves. … and they’re just directories. There’s nothing particularly special about them. You could remove them and clone from avogadrolibs or … whatever you want to do in them.

git clone https://github.com/OpenChemistry/openchemistry.git
cd avogadrolibs
git checkout master
git pull origin master
git checkout -b new-modification-tool
# etc

I don’t know how much “large project development” you’ve done. I started contributing to open source C++ packages back in 1997. In that time, compilers have changed probably around 8-10 times (i.e., more than just the C++ formal standards). Moreover, Qt which we use heavily, has changed, sometimes spitting out its own pile of warnings. (Qt 5.15 for example, spits out dozens of warnings … which we can’t fix until we switch uniformly to Qt6.)

In short, between “we can’t change this to C++20 syntax because we promise C++17 compiler support” and “we can’t change to new Qt 5.15 methods because we still want to support AppImage and Qt 5.12” there are, of course warnings.

There are, of course, ways to fix all these.

But as you point out, it takes effort, and my general assumption this summer is that the user base as a whole would rather we finish features than take time to fix warnings. (And of course I fix warnings where possible as I go.)

No worries. I just wanted to touch base and offer help. Please reach out when you’ve got a bit of time.