Anim: Motion Paths in camera space #117593

Merged
Christoph Lendenfeld merged 19 commits from ChrisLend/blender:motion_path_camera_space into main 2024-02-06 23:14:26 +01:00

This has been suggested by @P2design

Issue

Animators (especially for film and TV) often need to track the movement of things in screenspace. At the end of the day, the pixel motion is what counts. But motion paths were always in world space, which made it hard to use when the camera is also animated (during action scenes e.g.)

Solution

This PR introduces the feature of projecting a motion path into the screen space of the active scene camera.

Caveat
This makes the motion path only useful when looking through the active scene camera.

Build
https://builder.blender.org/download/patch/PR117593/

Technical implementation

This is achieved by baking the motion path points into the camera space on creation. For every point calculated, the camera is evaluated through the depsgraph and the resulting world matrix is used.
Then I pass in the current frame's world matrix of the camera into the shader to make sure the points follow it.
As can be seen in the video, it looks quite odd when viewed at another angle but this is expected. I mentioned that in the tooltip, so it shouldn't be an issue


For the review:
While technically possible I've not added the feature of choosing an arbitrary camera. My reasoning was that choosing an arbitrary camera doesn't add any value. You are either tracking arcs to the camera that renders the frame, or you'd track in world space.
But I am open to suggestions on that point.

This has been suggested by @P2design ### Issue Animators (especially for film and TV) often need to track the movement of things in screenspace. At the end of the day, the pixel motion is what counts. But motion paths were always in world space, which made it hard to use when the camera is also animated (during action scenes e.g.) ### Solution This PR introduces the feature of projecting a motion path into the screen space of the active scene camera. **Caveat** This makes the motion path only useful when looking through the active scene camera. **Build** https://builder.blender.org/download/patch/PR117593/ <video src=/attachments/5c39d83e-76aa-42ec-8cf0-66a01a209eca controls></video> ### Technical implementation This is achieved by baking the motion path points into the camera space on creation. For every point calculated, the camera is evaluated through the depsgraph and the resulting world matrix is used. Then I pass in the current frame's world matrix of the camera into the shader to make sure the points follow it. As can be seen in the video, it looks quite odd when viewed at another angle but this is expected. I mentioned that in the tooltip, so it shouldn't be an issue ----- For the review: While technically possible I've not added the feature of choosing an arbitrary camera. My reasoning was that choosing an arbitrary camera doesn't add any value. You are either tracking arcs to the camera that renders the frame, or you'd track in world space. But I am open to suggestions on that point.
Christoph Lendenfeld added the
Module
Animation & Rigging
label 2024-01-28 15:23:39 +01:00
Christoph Lendenfeld added 1 commit 2024-01-28 15:23:51 +01:00
Christoph Lendenfeld added 1 commit 2024-01-28 15:44:24 +01:00
Christoph Lendenfeld added 1 commit 2024-01-28 16:14:37 +01:00
buildbot/vexp-code-patch-lint Build done. Details
buildbot/vexp-code-patch-darwin-arm64 Build done. Details
buildbot/vexp-code-patch-darwin-x86_64 Build done. Details
buildbot/vexp-code-patch-linux-x86_64 Build done. Details
buildbot/vexp-code-patch-windows-amd64 Build done. Details
buildbot/vexp-code-patch-coordinator Build done. Details
999dbfd31f
make work for bones
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR117593) when ready.
Christoph Lendenfeld added 1 commit 2024-01-30 09:30:55 +01:00
Christoph Lendenfeld added 2 commits 2024-01-30 09:57:42 +01:00
buildbot/vexp-code-patch-lint Build done. Details
buildbot/vexp-code-patch-darwin-x86_64 Build done. Details
buildbot/vexp-code-patch-darwin-arm64 Build done. Details
buildbot/vexp-code-patch-linux-x86_64 Build done. Details
buildbot/vexp-code-patch-windows-amd64 Build done. Details
buildbot/vexp-code-patch-coordinator Build done. Details
43c362dfa5
move flag assignments
Author
Member

@blender-bot package
fixed a bug where objects would not move when autokeying enabled and pressing G

@blender-bot package fixed a bug where objects would not move when autokeying enabled and pressing `G`
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR117593) when ready.
Christoph Lendenfeld changed title from WIP: Anim: Motion Paths in camera space to Anim: Motion Paths in camera space 2024-01-30 10:00:29 +01:00
Christoph Lendenfeld added 1 commit 2024-01-30 10:12:49 +01:00
Falk David reviewed 2024-01-30 10:25:15 +01:00
@ -177,0 +194,4 @@
copy_v3_v3(vert_coordinate, mpv->co);
if (points_in_camera_space && cam_eval) {
/* Projecting the point into world space from the cameras pov. */
mul_m4_v3(cam_eval->object_to_world, vert_coordinate);
Member

I'd expect this to use math::transform_point(). E.g.

const float3 vert_coordinate = math::transform_point(float4x4_view(cam_eval->object_to_world), float3(mpv->co));
I'd expect this to use `math::transform_point()`. E.g. ``` const float3 vert_coordinate = math::transform_point(float4x4_view(cam_eval->object_to_world), float3(mpv->co)); ```
Author
Member

I assume that's just the new C++ syntax that does the same thing?

I assume that's just the new C++ syntax that does the same thing?
Member

Right, we should use that when possible.

Right, we should use that when possible.
Falk David reviewed 2024-01-30 10:25:48 +01:00
@ -173,0 +173,4 @@
if (mpath->flag & MOTIONPATH_FLAG_BAKE_CAMERA && mpath->camera) {
Object *cam_eval = DEG_get_evaluated_object(depsgraph, mpath->camera);
/* Convert point to camera space. */
mul_m4_v3(cam_eval->world_to_object, mpv->co);
Member

Same as above.

Same as above.
Author
Member

not sure if that improves things. since I can't assign to mpv->co I've got this code now
Also I can't use float4x4_view with transform_point

float3 co_camera_space = math::transform_point(float4x4(cam_eval->world_to_object),
                                                     float3(mpv->co));
copy_v3_v3(mpv->co, co_camera_space);
not sure if that improves things. since I can't assign to `mpv->co` I've got this code now Also I can't use `float4x4_view` with `transform_point` ``` float3 co_camera_space = math::transform_point(float4x4(cam_eval->world_to_object), float3(mpv->co)); copy_v3_v3(mpv->co, co_camera_space); ```
Member

Right. I'm a bit surprised that the motion paths are saved to the file. Seems like that would only be runtime data. In that case it would have been easy to make mpv->co a float3.

Right. I'm a bit surprised that the motion paths are saved to the file. Seems like that would only be runtime data. In that case it would have been easy to make `mpv->co` a `float3`.
Christoph Lendenfeld added 1 commit 2024-01-30 11:05:50 +01:00
Sybren A. Stüvel requested changes 2024-01-30 11:09:22 +01:00
Sybren A. Stüvel left a comment
Member

Using the camera matrix in the shader is a nice approach, I like it!

I think overall the code is in good shape, just one little note. And of course it'll need to be adjusted for my PR-breaking commit (4d3bb6a38a).

Using the camera matrix in the shader is a nice approach, I like it! I think overall the code is in good shape, just one little note. And of course it'll need to be adjusted for my PR-breaking commit (4d3bb6a38a225e74c6d037ebff94036d27b8ac08).
@ -123,6 +123,7 @@ static void motion_path_cache(OVERLAY_Data *vedata,
bool show_keyframes_no = (avs->path_viewflag & MOTIONPATH_VIEW_KFNOS) != 0;
bool show_frame_no = (avs->path_viewflag & MOTIONPATH_VIEW_FNUMS) != 0;
bool show_lines = (mpath->flag & MOTIONPATH_FLAG_LINES) != 0;
const bool points_in_camera_space = (avs->path_bakeflag & MOTIONPATH_BAKE_CAMERA_SPACE) != 0;

For futher clarification that this is a boolean, and not a collection of "the points in camera space", maybe is_in_camera_space is a better name.

For futher clarification that this is a boolean, and not a collection of "the points in camera space", maybe `is_in_camera_space` is a better name.
Author
Member

good point, changed that

good point, changed that
Christoph Lendenfeld requested review from Sybren A. Stüvel 2024-01-30 12:15:04 +01:00
Author
Member

@dr.sybren I added your changes, but due to the server issues the commit doesn't appear here. Will hopefully be resolved soon

@dr.sybren I added your changes, but due to the server issues the commit doesn't appear here. Will hopefully be resolved soon
Christoph Lendenfeld added 2 commits 2024-01-30 12:47:04 +01:00

but due to the server issues the commit doesn't appear here. Will hopefully be resolved soon

I made the commit in main, not in this PR, so I wouldn't expect it to appear here.

> but due to the server issues the commit doesn't appear here. Will hopefully be resolved soon I made the commit in main, not in this PR, so I wouldn't expect it to appear here.
Author
Member

@dr.sybren of course. Sorry should have been clearer. I merged main and implemented your review comment.
The code has updated by now, but you cannot see those commits in the history of the PR

@dr.sybren of course. Sorry should have been clearer. I merged main and implemented your review comment. The code has updated by now, but you cannot see those commits in the history of the PR
Christoph Lendenfeld added 1 commit 2024-02-02 09:21:14 +01:00
First-time contributor

why not make everything easier instead of a checkbox: when you look through a camera (any one, not necessarily active), the motion paths will be displayed in its space, and when you do not look through the camera(that is, look through the viewport itself), the paths are displayed in world space

why not make everything easier instead of a checkbox: when you look through a camera (any one, not necessarily active), the motion paths will be displayed in its space, and when you do not look through the camera(that is, look through the viewport itself), the paths are displayed in world space
Christoph Lendenfeld added 1 commit 2024-02-02 12:20:27 +01:00
Author
Member

@sanek122005 I had a look at that, while possible not quite feasible to do right now. Will revisit for 4.2

@dr.sybren I've checked what Jason mentioned, about having multiple cameras that are switched using markers. That doesn't work currently.
For that we'd need to evaluate the camera for every frame in the motion path range. This would be neat to do in the future because it would allow what @sanek122005 has suggested, but in the interest of landing this I'd wait with that improvement.
I did document that limitation in the description of the flag though

@sanek122005 I had a look at that, while possible not quite feasible to do right now. Will revisit for 4.2 @dr.sybren I've checked what Jason mentioned, about having multiple cameras that are switched using markers. That doesn't work currently. For that we'd need to evaluate the camera for every frame in the motion path range. This would be neat to do in the future because it would allow what @sanek122005 has suggested, but in the interest of landing this I'd wait with that improvement. I did document that limitation in the description of the flag though
Sybren A. Stüvel approved these changes 2024-02-02 12:37:08 +01:00
Sybren A. Stüvel left a comment
Member

For that we'd need to evaluate the camera for every frame in the motion path range.

Isn't that necessary anyway to get the camera motion (when it's in a camera rig with bones and constraints and whatnot)? I guess you mean that this would require evaluating the scene, to get the now-active scene camera.

This would be neat to do in the future because it would allow what @sanek122005 has suggested, but in the interest of landing this I'd wait with that improvement.
I did document that limitation in the description of the flag though

I think it's fine to have this as a known limitation now.
The code can be refactored a bit more, removing more duplications. Better to land this first, though, so that the new feature is added.

> For that we'd need to evaluate the camera for every frame in the motion path range. Isn't that necessary anyway to get the camera motion (when it's in a camera rig with bones and constraints and whatnot)? I guess you mean that this would require evaluating the scene, to get the now-active scene camera. > This would be neat to do in the future because it would allow what @sanek122005 has suggested, but in the interest of landing this I'd wait with that improvement. > I did document that limitation in the description of the flag though I think it's fine to have this as a known limitation now. The code can be refactored a bit more, removing more duplications. Better to land this first, though, so that the new feature is added.
Author
Member

For that we'd need to evaluate the camera for every frame in the motion path range.

Isn't that necessary anyway to get the camera motion (when it's in a camera rig with bones and constraints and whatnot)? I guess you mean that this would require evaluating the scene, to get the now-active scene camera.

Yes but I do it at bake time, so the cost is paid once. To get it to work with multiple cameras I'd need to do it at every frame (or have a "smart" caching system)

> > For that we'd need to evaluate the camera for every frame in the motion path range. > > Isn't that necessary anyway to get the camera motion (when it's in a camera rig with bones and constraints and whatnot)? I guess you mean that this would require evaluating the scene, to get the now-active scene camera. Yes but I do it at bake time, so the cost is paid once. To get it to work with multiple cameras I'd need to do it at every frame (or have a "smart" caching system)
Christoph Lendenfeld added 1 commit 2024-02-02 12:44:30 +01:00
Sybren A. Stüvel reviewed 2024-02-02 13:46:02 +01:00
@ -169,2 +182,4 @@
col[3] = col_kf[3] = 255;
Object *cam_eval = nullptr;
if (is_in_camera_space && draw_ctx->v3d->camera) {

There's now two places that decide which camera to use (this one and the similar line above).

It would be better to refactor this into a function, something like

static Object *camera_for_motion_path(const DRWContextState *draw_ctx, const eMotionPaths_BakeFlag bakeflag);

That function can then determine whether to use the scene's active camera or the view's active camera. I think for most animators the scene's active camera will be the desired one, as it's typically the on-screen path that matters most. If that's ever to change, there should be one place to modify.

There's now two places that decide which camera to use (this one and the similar line above). It would be better to refactor this into a function, something like ```cpp static Object *camera_for_motion_path(const DRWContextState *draw_ctx, const eMotionPaths_BakeFlag bakeflag); ``` That function can then determine whether to use the scene's active camera or the view's active camera. I think for most animators the scene's active camera will be the desired one, as it's typically the on-screen path that matters most. If that's ever to change, there should be one place to modify.
Author
Member

good idea. However I am not sure why I need the bakeflag.
Should the function return a nullptr when the MOTIONPATH_BAKE_CAMERA_SPACE is not set?

good idea. However I am not sure why I need the bakeflag. Should the function return a `nullptr` when the `MOTIONPATH_BAKE_CAMERA_SPACE` is not set?

good idea. However I am not sure why I need the bakeflag.
Should the function return a nullptr when the MOTIONPATH_BAKE_CAMERA_SPACE is not set?

That was my idea, yeah. Then there's one place that determines "is a camera needed, and if so, which one?"

The rest of the code can then just work with one check: is there a camera to work relative to? Seemed like a good idea to me, to avoid the need to check two things, and thus avoid spending brainpower understanding what has to happen when one check is positive and the other negative (should it fail? pretend the one positive check doesn't exist? those are all questions that would float in my mind when working with double conditions).

> good idea. However I am not sure why I need the bakeflag. > Should the function return a `nullptr` when the `MOTIONPATH_BAKE_CAMERA_SPACE` is not set? That was my idea, yeah. Then there's one place that determines "is a camera needed, and if so, which one?" The rest of the code can then just work with one check: is there a camera to work relative to? Seemed like a good idea to me, to avoid the need to check two things, and thus avoid spending brainpower understanding what has to happen when one check is positive and the other negative (should it fail? pretend the one positive check doesn't exist? those are all questions that would float in my mind when working with double conditions).
Author
Member

good point, changed it so the function takes the flag

good point, changed it so the function takes the flag
Christoph Lendenfeld added 1 commit 2024-02-02 14:46:33 +01:00
Christoph Lendenfeld added 2 commits 2024-02-06 10:34:20 +01:00
Christoph Lendenfeld requested review from Sybren A. Stüvel 2024-02-06 10:35:58 +01:00
Sybren A. Stüvel approved these changes 2024-02-06 16:51:34 +01:00
Sybren A. Stüvel left a comment
Member

LGTM! One thiny thing that can be adjusted when landing.

LGTM! One thiny thing that can be adjusted when landing.
@ -110,0 +111,4 @@
static Object *get_camera_for_motion_path(const DRWContextState *draw_context,
const eMotionPath_BakeFlag bake_flag)
{
if (bake_flag & MOTIONPATH_BAKE_CAMERA_SPACE) {

Flip the condition, return early. That way the code is clearly separated into two steps:

  1. See if the feature needs to be active at all.
  2. Determine which camera is used.
  if ((bake_flag & MOTIONPATH_BAKE_CAMERA_SPACE) == 0) {
    return nullptr;
  }
  return draw_context->v3d->camera;
Flip the condition, return early. That way the code is clearly separated into two steps: 1. See if the feature needs to be active at all. 2. Determine which camera is used. ```cpp if ((bake_flag & MOTIONPATH_BAKE_CAMERA_SPACE) == 0) { return nullptr; } return draw_context->v3d->camera; ```
Christoph Lendenfeld added 1 commit 2024-02-06 22:42:11 +01:00
Christoph Lendenfeld merged commit 79f84775f2 into main 2024-02-06 23:14:26 +01:00
Christoph Lendenfeld deleted branch motion_path_camera_space 2024-02-06 23:14:29 +01:00
Sign in to join this conversation.
No reviewers
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
5 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#117593
No description provided.