Fix: GPencil animated layer transforms evaluate wrong when identity
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Sep 20 2022, 12:14 PM.

Details

Reviewers

Antonio Vazquez (antoniov)
Sybren A. Stüvel (sybren)
Falk David (filedescriptor)

Maniphest Tasks

T101164: GPencil: framechanging does not evaluate keyframed layer transform correctly (if this would result in identity matrix)

Commits

rBdfcc8086fb29: Fix: GPencil animated layer transforms evaluate wrong when identity
rB24814a03b734: Fix: GPencil animated layer transforms evaluate wrong when identity
rB1cbfe0463879: Fix: GPencil animated layer transforms evaluate wrong when identity

Summary

Due to (optimization) checks in in BKE_gpencil_prepare_eval_data &
BKE_gpencil_update_layer_transforms, updates were skipped if animation
reached exact identity transforms.

Now check if the matrix has changed additionally to gain proper updates.
Unsure if this is the cheapest way to check for the animated state of
layer transforms tbh, but I see similar checks elsewhere.

Fixes T101164.

Diff Detail

Repository: rB Blender

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Sep 20 2022, 12:14 PM

Philipp Oeser (lichtwerk) created this revision.

Harbormaster completed remote builds in B23844: Diff 55903.Sep 20 2022, 12:14 PM

Philipp Oeser (lichtwerk) added projects: Grease Pencil, Animation & Rigging.Sep 20 2022, 12:15 PM

LGTM

This revision is now accepted and ready to land.Sep 20 2022, 3:28 PM

@Falk David (filedescriptor) : any concerns performance-wise?

Apart from the comment, this looks good.

(It's unfortunate that we have to keep layer_mat and the components in sync. It means that there are probably more hidden issues like this. E.g. BKE_gpencil_prepare_eval_data returns early if there is a parent on the layer. Does that mean the update is wrong if the layer transforms are used in combination with parenting? We'll have to keep an eye on this)

source/blender/blenkernel/intern/gpencil.c
2743	The check for `is_zero_v3(gpl->scale)` should be removed. It's a very small optimization that probably leads to similar issues like the ones in the original report.