Page MenuHome

Fix T66713: Walk Teleport and other movements do not take into account the unit scale of the Scene
ClosedPublic

Authored by Marco van den Oever (crashlog) on Oct 18 2022, 4:17 AM.

Details

Summary

Fix for T66713.

When changing a scene's unit scale from 1 to something else, 0.1 for e.g. walk navigation no longer worked properly.
Whenever gravity is enabled, and the user starts to fall or jump, the view-port glitched out into low earth orbit.


I've found that this is due to the walk navigation incorrectly taking the unit scale into account, in the part where gravity, falling and jumping are handled. In fact, the unit scale is taken into account twice, resulting in the weird behavior.

The falling part works as follows:

  • When we start to fall, the current viewport position (walk->rv3d->viewinv) is stored (walk->teleport.origin)
  • During every frame after this, the origin is taken, and a distance is subtracted based on the time since falling start and gravity, to simulate acceleration (z_new)
  • Because we need a delta and not an absolute value, the delta between the newly calculated z-value and the current z-value is taken (z_cur - z_new)
  • The delta is then set on the dvec vector
  • Later on, the dvec is scaled according to the unit scale
  • This overshoots the fall distance, which is compensated by the next frame (resulting in a negative fall distance). But that is then also overshot, so we're being moved from too far down, to too far up, and so forth.

As I understand the code now, the viewport matrix (walk->rv3d->viewinv) already operates in unit scale-adjusted space, so our delta (z_cur - z_new) is already the scaled value we need, so shouldn't be scaled again.
I've first disabled the dvec scale, which also solves the problem, but makes other parts (walking around with wasd) super slow, since those are scaled too.
So it seems the most correct to me to have the falling distance be done in non-scaled space, same as wasd and jump.

On a side note, I've been wondering whether it'd be nicer to pull this feature out of the core entirely, and move it to an addon. That way, it would be way easier to debug and make changes, or disable entirely. I'm assuming that by enabling such an addon by default and have it use the same keymap, it shouldn't be a breaking change.

I'm still testing this locally, and there may be other issues that come to light now, since walking around with gravity wasn't really possible before.

Diff Detail

Repository
rB Blender

Event Timeline

Marco van den Oever (crashlog) requested review of this revision.Oct 18 2022, 4:17 AM
Marco van den Oever (crashlog) created this revision.
Marco van den Oever (crashlog) edited the summary of this revision. (Show Details)
Pratik Borhade (PratikPB2123) retitled this revision from Walk Teleport and other movements do not take into account the unit scale of the Scene to Fix T66713: Walk Teleport and other movements do not take into account the unit scale of the Scene.Oct 20 2022, 12:10 PM
Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)
This revision is now accepted and ready to land.Oct 21 2022, 7:17 AM

Committed fix a0bbd65d57f38344fb71565c3c34396363a4be23
Thanks for the fix, regarding making this into an add-on.

In principle it could work but would mean having go make an API that exposes internal functionality so the Python script can take control of the view, where motion is properly key-framed and features are exposed via key-maps.

Just splitting out the functionality is a reasonably big task, so I don't think it has much advantage, for the work involved.

Committed fix a0bbd65d57f38344fb71565c3c34396363a4be23
Thanks for the fix, regarding making this into an add-on.

In principle it could work but would mean having go make an API that exposes internal functionality so the Python script can take control of the view, where motion is properly key-framed and features are exposed via key-maps.

Just splitting out the functionality is a reasonably big task, so I don't think it has much advantage, for the work involved.

Fair enough!

Thank you for the review, hope this'll make other people happy too :)