Skip to content
This repository has been archived by the owner on Oct 4, 2022. It is now read-only.

Bug fix for extra movement of vegetation instances #521

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yuriy0
Copy link

@yuriy0 yuriy0 commented Sep 16, 2020

Issue #, if available:

Description of changes:

This fixes a bug where moving a vegetation instance with the movement gizmo while in Vegetation Tool moves the instance much more than the gizmo moves, i.e. the gizmo position and the instance position do not correspond.

The cause is as follows:
AxisGizmo is calling CEditTool::OnManipulatorDrag with a change in position relative to the manipulator position when the manipulator was first started dragging, e.g.:

Vec3 p1 = view->SnapToGrid(view->ViewToWorld(m_cMouseDownPos));
Vec3 p2 = view->SnapToGrid(view->ViewToWorld(point));
vDragValue = p2 - p1;

(AxisGizmo.cpp:417) The actual method of computing this drag value differs based on the edit mode settings, but vDragValue is always relative to the position of the manipulator at drag start.

Then CVegetationTool::MoveSelected takes that offset exactly as is and applies it to the current position of the vegetation instance:

void CVegetationTool::MoveSelected(CViewport* view, const Vec3& offset, bool bFollowTerrain)
{
    ....
    for (int i = 0; i < m_selectedThings.size(); i++)
    {
        oldPos = m_selectedThings[i].pInstance->pos;

        ...

        newPos = oldPos + offset;
        ...

The result of this logic is, for example:

  1. Start dragging the translation gizmo at (500,500,32), which is the position of the vegetation instance.
  2. On the 1st mouse move callback you drag it to (500, 501, 32). The offset from start position is (0,1,0).
  3. CVegetationTool::MoveSelected moves the instance to (500, 500, 32) + (0, 1, 0) = (500, 501, 32) (correct so far)
  4. On the 2nd mouse move callback you drag the gizmo to (500, 502, 32). The offset from the start position is (0, 2, 0).
    CVegetationTool::MoveSelected moves the instance to (500, 501, 32) (it’s current position) + (0, 2, 0) = (500, 503, 32) (NOT the position of the gizmo!)

Generally this results in moving the instance way too far compared to the gizmo position (indeed, the difference between actual and intended end positions is exponential).

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@AMZN-alexpete
Copy link

Thanks for submitting this fix @yuriy0, we're working on getting it merged for an upcoming release.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants