Geometry Nodes: Do cache invalidation after writing attributes.
ClosedPublic
Actions

Authored by Jacques Lucke (JacquesLucke) on Oct 27 2021, 11:37 AM.

Details

Reviewers

Hans Goudey (HooglyBoogly)

Maniphest Tasks

T92509: Do cache invalidation OutputAttribute.save() method

Commits

rBcf771807b799: Geometry Nodes: do cache invalidation after writing attributes

Summary

This is a better and more general fix for T92511 and T92508 than the ones that I committed before.

Previously, we tagged caches dirty when first accessing attributes. This led to incorrect caches when under some circumstances. Now cache invalidation is part of OutputAttribute.save().

A nice side benefit of this change is that it may make things more efficient in some cases, because we don't invalidate caches when they don't have to be invalidated.

Diff Detail

Repository

rB Blender

Branch

do-cache-invalidation-after-write (branched from master)

Build Status

Buildable 18255
Build 18255: arc lint + arc unit

Event Timeline

Jacques Lucke (JacquesLucke) requested review of this revision.Oct 27 2021, 11:37 AM

Jacques Lucke (JacquesLucke) created this revision.

Harbormaster completed remote builds in B18255: Diff 44011.Oct 27 2021, 11:37 AM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Oct 27 2021, 11:39 AM

Harbormaster completed remote builds in B18257: Diff 44013.Oct 27 2021, 11:55 AM

Jacques Lucke (JacquesLucke) added a reviewer: Hans Goudey (HooglyBoogly).Oct 27 2021, 11:56 AM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Oct 27 2021, 11:58 AM

Jacques Lucke (JacquesLucke) added a task: T92509: Do cache invalidation OutputAttribute.save() method.Oct 27 2021, 1:50 PM

Merge branch 'master' into do-cache-invalidation-after-write

Harbormaster completed remote builds in B18285: Diff 44062.Oct 28 2021, 10:52 AM

This looks reasonable to me. It's too bad that we have to return WriteAttributeLookup instead of GVArray, but I don't see a better way currently, unless we added a similar callback to GVMutableArray, which seems too specific.

source/blender/blenkernel/BKE_spline.hh
573	I didn't add something like this already because I'd like to make the cache invalidation more specific in the future rather than more generic. I guess we could add an `IndexMask` input here if that becomes an issue.

This revision is now accepted and ready to land.Oct 28 2021, 3:53 PM

It's too bad that we have to return WriteAttributeLookup instead of GVArray.

I don't think that's too bad tbh. Feels like a fairly natural extension to me.

source/blender/blenkernel/BKE_spline.hh
573	In that case, I recommend you add methods like `positions_changed()`, `tilt_changed()`, ... to the splines and curve. Internally they can still call `mark_cache_invalid` for now.