nglviewer / ngl

WebGL protein viewer
http://nglviewer.org/ngl/
MIT License
669 stars 170 forks source link

Feature/rotate selection and pivot #909

Open garyo opened 2 years ago

garyo commented 2 years ago

This PR adds a pivot point around which rotations happen to a component. This makes it easier to rotate a component interactively around an atom, the component's center, or any other point.

Based on that, it adds a CenterMode to StructureComponent to allow interactively rotating around the current selection or the structure's center.

This PR also allows the viewer.rotationGroup to have a non-unit scale factor to scale the whole scene, for when it is included in a larger three.js scene. It simply extracts the rotation part only in trackball-controls.ts. That seemed a lot simpler (and a lot less code change) than introducing a new viewer.scaleGroup.

Adds an example to show how it works. There is some complexity to handling selection-changes so it seemed worth creating the fully worked example.

fredludlow commented 2 years ago

btw - it builds fine and tests pass etc: https://fredludlow.com/ngl/pr-909/examples/webapp.html?script=test/pivot

garyo commented 2 years ago

I did try moving some of this into the trackball code, but since my main application for this is animated motion tracks for the molecules, having it there makes that harder. I wish the pre-transform to compensate for selection changes wasn't needed but I don't think there's any good way to avoid it. As for doing it in setPivot, ideally you want both changing the pivot and changing the selection to avoid jumping around. I don't know how to do both of those other than the way it is now (without huge refactoring of the matrix code)... but I'm open to alternatives.

fredludlow commented 2 years ago

Right, I think I understand now:

As you said: When you update selection, the center changes. We still want to keep the pivot as a relative offset from the center (the clear case for this is that if it's at 0,0,0 for the whole structure and you select only one chain, you don't want pivot jumping to some offset and negating the nice behaviour of rotating around the center of the selection).

Your example code recalculates component.transform such that the final position is unchanged (counteracting the change in center+pivot).

Aside: matrix inversions get a bad rep but I think we're good here - it would be possible to calculate the new transform without the inversion from a combination of old and new centers, but this is shorter (otherwise you need to handle scale too)

I tried moving the selection change code from your pivot example into the selection.signals.stringChanged handler in structure-component.ts, i.e:

      this.selection.signals.stringChanged.add(() => {
      this.structureView.setSelection(this.selection)

      this.rebuildRepresentations()
      this.rebuildTrajectories()
      if (this.centerMode === 'selection') {
        // Selection's center point has changed, and in center mode we use
        // the selection center in the matrix (via getCenterUntransformed),
        // so we need to recalculate the matrix.

        // Additionally:
        // When the selection changes, we don't want the molecule to move
        // around. So compute a pre-transform that will take the new matrix,
        // based on the new center point, back to the old matrix. At this
        // point, it's important that the matrix hasn't yet been updated to
        // reflect the new selection's center point.

        const oldMatrix = this.matrix.clone()

        this.updateMatrix(true) // get matrix w/ new selection-center (silently, no signals)

        // Update pre-transform to make final result same as m0 (old matrix)
        // T' = m0 * m1^-1 * T
        tmpMat4.getInverse(this.matrix)
        this.transform.premultiply(tmpMat4).premultiply(oldMatrix)

        this.updateMatrix()
      }

    })

That seems to work for me, though as always I'm probably overlooking something...

fredludlow commented 2 years ago

As for doing it in setPivot, ideally you want both changing the pivot and changing the selection to avoid jumping around.

Yes, this would also be nice, I guess a similar fudge to transform could be done in the setPivot code? That said, if you're playing around with the pivot at least you would understand (or could make an educated guess) why your molecule has moved, but jumping around on selection change is definitely more surprising.