Fix T70585: Walk Navigation keyframe tweaks (still frame)
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Oct 8 2019, 1:55 PM.

Details

Reviewers

Dalai Felinto (dfelinto)

Maniphest Tasks

T70585: 'View Navigation (Walk/Fly)' inserts Location only keyframes instead of the selected keying set.

Commits

rB7a1847f0e9fd: Fix T70585: Walk Navigation keyframe tweaks (still frame)

Summary

Walk Navigation was missing rotation keyframes on confirm (when animation
wasnt playing) after rB22bdd08dfd0.

Because for walk [in contrast to fly], the cursor is constantly warped
(WM_cursor_warp in walkEvent), we cant do something reasonable with
comparing mouse positions (to detect rotation) when mouse comes to rest.

Now remember if rotation changed at any time.

Also tweaked the insertion for location keyframes (which was always
happening even if the camera did not move), now checking the real dvec
(instead on relying on speed)

Diff Detail

Repository: rB Blender

Event Timeline

Philipp Oeser (lichtwerk) created this revision.Oct 8 2019, 1:55 PM

Harbormaster completed remote builds in B5282: Diff 18864.Oct 8 2019, 1:55 PM

Tested here. Nitpicking aside the patch is good and working well.

That aside (as a separate patch in the far future?) I think it should keyframe ALL channels if anything changed. This is how Blender does by default.
If you open:

camera bug.blend740 KBDownload

And just translate the camera around it will keyframe location, rotation AND scale. But 2.80 is working like this patch, and not how I suggest, so we can treat as a separate issue.

source/blender/editors/space_view3d/view3d_walk.c
207	FYI I personally would use bitwise flags for that, makes easy to expand in the future. That aside let me nitpick ;) whatabout: - bool needs_rotation_keyframe; - bool needs_translation_keyframe; + bool need_keyframe_rotation; + bool need_keyframe_translation;

This revision is now accepted and ready to land.Oct 8 2019, 2:22 PM

In D6018#138447, @Dalai Felinto (dfelinto) wrote:

Tested here. Nitpicking aside the patch is good and working well.

That aside (as a separate patch in the far future?) I think it should keyframe ALL channels if anything changed. This is how Blender does by default.
If you open:

camera bug.blend740 KBDownload

And just translate the camera around it will keyframe location, rotation AND scale. But 2.80 is working like this patch, and not how I suggest, so we can treat as a separate issue.

Yep, for that, would need to really take selected keyingsets into account. atm. it just does ANIM_KS_ROTATION_ID and ANIM_KS_LOCATION_ID which I think for fly/walk is fine though?
(also need to make myself familiar with the Only Needed option for keyingsets -- and why this option apparently isnt avaiable for builting keymaps? anyhow, thats for later...)

+1 for need_ vs. needs_ (will change that and commit this for now).

Thx for review!

Closed by commit rB7a1847f0e9fd: Fix T70585: Walk Navigation keyframe tweaks (still frame) (authored by Philipp Oeser (lichtwerk)). · Explain WhyOct 8 2019, 4:28 PM

This revision was automatically updated to reflect the committed changes.

Philipp Oeser (lichtwerk) added a commit: rB7a1847f0e9fd: Fix T70585: Walk Navigation keyframe tweaks (still frame).