Fix T87929: Cycles 'Indirect Only' collection property missing update
AbandonedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Apr 30 2021, 12:48 PM.

Details

Reviewers

Kévin Dietrich (kevindietrich)
Brecht Van Lommel (brecht)

Maniphest Tasks

T87929: Toggling "Indirect Only" for the view layers properties of a collection does not result in a change in the viewport (2.93, 3.0 regression)

Summary

Caused by rBbbe6d4492823: Cycles: optimize device updates.

Regular object ray visibility updates will get detected through
dependency graph (is_updated_shading / is_updated_transform), that is
why these update correctly (objects are in recalc map).

Objects in collections that had their 'Indirect Only' property toggled
however arent. Now tag for update as well if their visibility changed
(through the 'Indirect Only' property of a collection they are in).

Not exactly sure if this should be handled on the dependency graph side
instead?

Diff Detail

Repository

rB Blender

Branch

T87929 (branched from master)

Build Status

Buildable 14324
Build 14324: arc lint + arc unit

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Apr 30 2021, 12:48 PM

Philipp Oeser (lichtwerk) created this revision.

Harbormaster completed remote builds in B14324: Diff 36678.Apr 30 2021, 12:48 PM

Philipp Oeser (lichtwerk) added a project: Render & Cycles.Apr 30 2021, 12:48 PM

This does not seem to be the right fix for the issue, see inline comments. I might look into fixing this myself, as it appears to be a bit more involved, unless you want to have a go at it based on my comments.

intern/cycles/blender/blender_object.cpp
251	This seems redundant, the only place after that where it is used is the condition: if (object->is_modified() \|\| object_updated ...) `object->is_modified()` should already be true if `visibility_is_modified()` is also true, or we have a bug when updating the bit field.
252	Why is it using `TRANSFORM_MODIFIED`? If we fail to detect a change in visibility then I would expect to have a dedicated flag for this or a comment explaining the relationship. Off the top of my head, I think the issue is actually a missing BVH data update when visibility changes, and thus we will need to change the logic in GeometryManager to also repack data when a change in visibility is detected. Also, tagging the object manager for an update should be done via `Objet::tag_update` which will be called anyway at the end of the function (if the object was modified, which it is).

This revision now requires changes to proceed.May 2 2021, 2:11 AM

In D11127#284569, @Kévin Dietrich (kevindietrich) wrote:

This does not seem to be the right fix for the issue, see inline comments. I might look into fixing this myself, as it appears to be a bit more involved, unless you want to have a go at it based on my comments.

Please do, that frees up time to tackle more reports...

Revision Contents
Changeset List

Path

Size

intern/

cycles/

blender/

blender_object.cpp

5 lines

Diff 36678

View Options

intern/cycles/blender/blender_object.cpp

Fix T87929: Cycles 'Indirect Only' collection property missing updateAbandonedPublicActions