Skip to content

Conversation

@Rishabh-Sukhwani
Copy link
Contributor

PR Description: Update Gizmo Scaling Behavior to match Ray-Based Translation

Closes #488

Problem:
The gizmo's scaling operation previously felt different in magnitude and responsiveness compared to the translation gizmo. This was because the translation gizmo had been updated to use a more accurate ray-based interaction model (as per #306), while the scaling gizmo still relied on older screen-space mouse movement calculations. This inconsistency could make precise scaling less intuitive.

Description of Changes:
This PR updates the scaling behavior of the editor gizmo to align it with the improved ray-based logic now used by the translation gizmo. The goal is to provide a more consistent and predictable user experience when manipulating objects in 3D space.

Fix Implemented:
The ApplyScale method within OvEditor::Core::GizmoBehaviour has been refactored. Instead of calculating scale based on 2D screen-space mouse displacement, it now leverages the core ray-casting and 3D plane intersection logic from the ApplyTranslation method, as suggested in the original issue (#488).

Specifically:

  1. A 3D ray is cast from the current mouse position into the scene.
  2. An interaction plane is defined, aligned with the gizmo's scaling axis and oriented towards the camera (similar to how the translation plane is established).
  3. The intersection point of the mouse ray with this 3D plane is calculated.
  4. The displacement of this intersection point on the plane (since the drag operation began) is determined.
  5. This 3D displacement is then projected onto the world-space direction of the gizmo's scaling axis. The resulting scalar value represents the desired change in scale along that axis.
  6. This scalar change is then applied to the object's original scale, with snapping logic and checks to prevent negative scales remaining in place.
  7. The ApplyScale method is no longer const as it now modifies internal state (m_firstPick, m_initialOffset) consistent with ApplyTranslation.

By adopting this approach, the scaling operation now directly responds to how the mouse moves in the 3D world relative to the gizmo axis, rather than interpreting 2D screen movements. This should result in a scaling gizmo that feels more direct, accurate, and harmonious with the translation gizmo.

Screenshots:

before the changes in this PR:

before

After:

after

Copy link
Member

@adriengivry adriengivry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! Tested locally and it works as expected.
Some nits:

  • use tabs instead of spaces for indentations (I know we are not 100% consistent across the board, but we are trying to move away from spaces)
  • use const qualifiers whenever possible (i.e. const auto ray, const planePoint, etc...). Same as above, not 100% consistent, but we're now trying to enforce it more.

P.S. thank you for taking the time to document your PR, it's much appreciated and makes reviewing so much easier!

@adriengivry adriengivry added QoL Quality of Life : Something that can improve users productivity Editor Something relative with the editor labels May 6, 2025
@Rishabh-Sukhwani Rishabh-Sukhwani force-pushed the fix/update-gizmo-scaling branch from 0a85e2b to 351a9fa Compare May 7, 2025 03:31
@Rishabh-Sukhwani
Copy link
Contributor Author

Thanks for the review and the positive feedback! The * 2.0f multiplier definitely feels more natural.

I've implemented the suggestions and amended my previous commit:

  • The indentation throughout GizmoBehaviour.cpp has been updated to use tabs instead of spaces.
  • const qualifiers have been added to local variables where appropriate.
  • You'll notice that to make planeNormal const, I had to ensure tempPlaneNormal was fully determined before planeNormal's initialization (which was already the case, but it clarifies the flow for const).
  • The changes have been tested, and the scaling behavior works as expected with these refinements.

And thank you for the kind words about the PR documentation! I always try to make the review process as smooth as possible.

Copy link
Member

@adriengivry adriengivry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like something went wrong with the last commit 😅

OvEditor_cGVaixdWQR.mp4

On a side note, you don't necessarily need to squash/amend commits, as they'll automatically be squashed into a single commit when merged to main! Sometimes better to keep the full history on the PR so it's easier to see what has changed between each review/revert if something went wrong

Copy link
Member

@adriengivry adriengivry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Approved ✅

@adriengivry adriengivry merged commit 8ecae34 into Overload-Technologies:main May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Editor Something relative with the editor QoL Quality of Life : Something that can improve users productivity

Development

Successfully merging this pull request may close these issues.

Update gizmo scaling behaviour to match translation (ray cast)

2 participants