I'm uploading this as a patch for sanity checking since I don't really know anything about depsgraph stuff. Fixes the issue, though.
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- fix-T99151 (branched from master)
- Build Status
Buildable 24329 Build 24329: arc lint + arc unit
Event Timeline
Comment Actions
I am not aware of any part of the code in the dependency relations builder which will rely on the viewport settings. So from this point of view tagging for relations update seems wrong.
Also, Eevee and the workbench engines respond correctly to the changes in the viewport, so to me it does not seem to be the dependency graph problem.
To me it seems the root of the issue is somewhere else. Also, what is exactly you expect from the reviewers at this state of the patch?
Comment Actions
Updating this visibility does not require a depsgraph relations update. We only need to notify the render engines to refresh, not actually do this relatively expensive update.
Maybe DEG_id_type_tag(bmain, ID_OB) works? It's used in VIEW3D_OT_localview which is a similar type of 3D viewport visibility setting.
Comment Actions
From what I can tell, this is because they query BKE_base_is_visible/BKE_object_is_visible_in_viewport in every iteration, while Cycles only does when the object gets tagged for updating. By default, however, b_depsgraph.updates in BlenderSync::sync_recalc is empty after toggling the visibility option, so Cycles doesn't update (until you change something else).
Also, what is exactly you expect from the reviewers at this state of the patch?
To get feedback on whether my approach makes sense, which is what I got, so thanks ;)
Yep, that one also works. I'll update the patch. I originally got the DEG_relations_tag_update idea from rna_Object_hide_update.
Comment Actions
Sorry, I must have forgotten to rebuild or something, I just checked again before merging and DEG_id_type_tag doesn't work after all.
Another call from localview *does* work, though: DEG_id_tag_update(&scene->id, ID_RECALC_BASE_FLAGS).
Comment Actions
@Lukas Stockner (lukasstockner97) Please stop committing patches which are in the Needs Review state.
You can not ignore "Requested Changes" from any of the reviewers, even if some other reviewers did accept the change.
Comment Actions
I am not sure why one would consider Phabricator weird when we literally use it daily, and would surely resolve its weirdness if that was the case.
As for the change itself, it is semantically wrong to fake updates like that just to implicitly trigger a specific code path in Cycles. The current state of the code violates the intended use of depsgraph API. There needs to be more proper way of explicitly communicating what's changed, because otherwise making such workarounds work will just get in a way of development.