jMonkeyEngine-Contributions / Lemur

Lemur is a jMonkeyEngine-based UI toolkit.
http://jmonkeyengine-contributions.github.io/Lemur/
BSD 3-Clause "New" or "Revised" License
116 stars 32 forks source link

DragHandler erroneous starting offset #64

Closed noncom closed 5 years ago

noncom commented 5 years ago

There is a bug in the DragHandler class. The

basePotition = capture.getWorldTranslation().clone()

line in startDrag() should probably instead be

basePosition = capture.getLocalTranslation().clone()

because everywhere else the handler is working with the local translation and getting the world translation here introduces an abnormal initial offset on drag start if the spatial is a child of some other spatial and its relative location is not 0, 0, 0.

It is easy to demonstrate this behavior by modifying the com.simsilica.lemur.demo.BasicDemo class: replace the line

guiNode.attachChild( testPanel );

in simpleInitApp() with

Node node = new Node();
node.attachChild(testPanel);
node.setLocalTranslation(200, 200, 0);
guiNode.attachChild(node);

and try to drag the testPanel.

pspeed42 commented 5 years ago

I think the problem is unfortunately more complicated than that. Dragging is done in world space... so basing dragging on the world space coordinates is correct. The issue is that the results are not applied in a local-space friendly way.

For example, I think if you rotate your node then things will behave very strangely. Essentially, DragHandler is not written to properly handle children where local and world translation are not the same. It could be... but it's more complicated than just basing the starting position on local space. At least I think. It's possible some other magic makes it work if node is rotated but I don't think so.

pspeed42 commented 5 years ago

Note: I'm going to leave this issue open because I would like DragHandler to be able to handle these child cases.

noncom commented 5 years ago

Well, my guess would be what about something along these lines?:

When the mouse is pressed:

  1. Hit the GUI element E with the ray
  2. Remember the offset of the hit in the local coords of the parent of E
  3. Project the hit coords to the world coords (projection A)
  4. Create a new temporary node T as a child of the root GUI node at these coords (the local and absolute do match for it)

When moving the mouse, dragging the element:

  1. Move the temporary node placing it exactly at mouse coords,
  2. Always project its absolute=local coords back to the local space of the parent P (projection B)
  3. Position the element E at these coordinates plus the offset

When releasing the mouse:

  1. Destroy or stash the temporary node T.

That should work with rotated parents too? The whole idea is to have an intermediate entity that can handle the projections easily.

pspeed42 commented 5 years ago

I don't think it requires all that. I haven't tested it or anything but I believe it's as simple as: -continue calculating dragging in world space as before -instead of calling spatial.setLocalTranslation() with the world space coordinate, convert that world space coordinate to local space with spatial.getParent().worldToLocal()

pspeed42 commented 5 years ago

Fix committed and confirmed it works in the demo. Closing this issue.