Bonsai: sync_render_visibility must tag the depsgraph - #8900
Conversation
Sorry, something went wrong.
…nderlay theoryshaw's root cause investigation (IfcOpenShell#8603): the DEFAULT underlay render path calls bpy.ops.render.render() once per drawing inside CreateDrawing's print_all loop. On EEVEE-Next, calling render.render() repeatedly within a single Python call stack, without Blender's main loop running a real iteration in between, leaves the render engine holding state from the previous call, so the 2nd-and-later drawing's underlay renders with the previous drawing's view. scene.camera, the depsgraph and the visible object set are all already correct at render time, confirmed by theoryshaw's own measurement, ruling out a scene/depsgraph sync issue. Printing each drawing as a separate operator invocation, or via the VIEWPORT render style (bpy.ops.render.opengl, driven off the 3D view space rather than the render engine), is unaffected. The existing tag_redraw()/wm.redraw_timer(DRAW_WIN_SWAP) mitigation before each print_all iteration does not fix this because it runs synchronously, nested inside the same call stack as the render.render() calls, so it never actually yields to Blender's main loop. This drives the print_all batch one drawing per modal timer tick instead of a plain Python for loop, so a genuine main loop iteration elapses between each drawing's render.render() call, matching the already-correct behaviour of printing each drawing separately. It stays a true modal operator (window_manager.event_timer_add + modal_handler_add) rather than a bare bpy.app.timers callback: a plain timer callback runs after execute() has already returned, and self.report() then raises ReferenceError because Blender has freed the operator's RNA, confirmed live. Modal keeps the operator alive across ticks, so self.report() still works. The single-drawing path (and print_all with one drawing) is untouched, since that's not where the bug happens. Modal timers don't fire in --background mode, so batches there keep the previous synchronous loop. Verified live in Blender 4.5 (EEVEE-Next): the new per-drawing state machine targets the correct camera at every step, in order, with a distinct main-loop tick elapsing between renders (confirmed via a heartbeat timer), and self.report() succeeds from the finishing modal() call. Could not reproduce the underlying EEVEE-Next render-content carry-over itself with a minimal synthetic scene in this environment, matching theoryshaw's own extensive list of things that don't reproduce or fix it in isolation. Fixes IfcOpenShell#8603 Generated with the assistance of an AI coding tool.
|
@BIMvoice heads up before you spend more time on this — I finally reproduced #8603 properly on the real model, and the diagnosis this PR is built on (mine) was wrong. Full write-up in #8603 (comment). Short version: it isn't render.render() carrying state across calls, so it isn't about whether the event loop runs between drawings. Blender.sync_render_visibility wrote hide_render via bpy.data.objects.foreach_set(...), which skips the RNA update and never tags the depsgraph, so the evaluated visibility stays a render behind and the renderer draws the previous drawing's objects with the current drawing's camera. The reason this restructure won't help: my repro is two separate bpy.ops.bim.create_drawing invocations with a full event loop in between, and it still fails. Rendering the same drawing twice in a row produces a pixel-identical wrong image. Your note that you couldn't reproduce the EEVEE-Next carry-over with synthetic scenes was the right instinct — there was no carry-over to reproduce. Sorry for the wrong lead. Closing or repurposing this is your call. |
Sorry, something went wrong.
|
@theoryshaw no wasted effort, and thank you for the write-up. Tracking it to foreach_set skipping the RNA update is a hard find, particularly when visible_get(), hide_render and visible_objects all read back the value you just wrote and tell you everything is fine. The pixel diff at 0.99% against 67% is about as unambiguous as evidence gets. Confirming from the source, and it supports you further than the write-up claims. Blender.sync_render_visibility at src/bonsai/bonsai/tool/blender.py:2365 does the foreach_set and then returns immediately, and the RNA assignment version, equality check included, is sitting commented out directly beneath it with the comment "For speed, check equality prior to change to prevent needless updates". So the code you are describing as the fix already existed and was deliberately bypassed for speed. Your measurements of 0.65s and then 0.06s suggest that tradeoff is no longer worth making. One detail to decide before it goes in: the commented out version opens with if not obj.data: continue, so objects with no data, empties among them, never get hide_render written at all. The foreach_set path writes every object unconditionally. Restoring the old block verbatim would quietly change behaviour for those. Do you want them included, or was the skip intentional? On this PR: I will repurpose it to the RNA fix rather than close it, since the branch and the issue link are already in place and it keeps one less PR in the queue. If you already have a working branch, say so and I will close this instead and review yours, since you have the real model and the repro. |
Sorry, something went wrong.
theoryshaw retracted the render.render()-carries-EEVEE-state diagnosis after reproducing on his real model. The actual cause is sync_render_visibility() writing hide_render via foreach_set, which skips the RNA update and never tags the depsgraph, not a missing main-loop iteration between renders. The modal/timer restructure this reverts addressed a cause that does not exist and added no independently useful behaviour (no cancellation, no per-drawing progress reporting beyond the pre-existing final count message). This reverts commit 28119d5.
foreach_set writes hide_render straight to DNA and skips the RNA update, so nothing tags the depsgraph. python readbacks (visible_get(), obj.hide_render, context.visible_objects) all read that same DNA field, so they report correct state even though the evaluated visibility used by the renderer is a render behind. On a batch or two sequential drawings, bpy.ops.render.render() then draws the previous drawing's object set with the current drawing's camera. Diagnosed and measured by theoryshaw on the real repro model (IfcOpenShell#8603): 0.65s for the first drawing's underlay (2982 of 3041 objects change), 0.06s for the next (248 changes). Assigns hide_render through RNA for every object in bpy.data.objects, not only ones with obj.data: an Empty's hide_render still affects the render when the Empty drives collection instancing, and skipping it would leave that Empty's hide_render never corrected once wrong. An equality check limits the RNA cost to objects whose visibility actually changed. Confirmed in Blender 5.2 (isolated profile): a two-plane EEVEE scene where flipping visibility between two same-call-stack renders via foreach_set produces a stale/wrong second render, while the same flip through RNA assignment renders correctly.
Backport of IfcOpenShell#8900 to v0.9.0.
Fixes #8603.
This PR has been rewritten. It previously carried a modal/timer restructure of the print_all batch loop, built on the original diagnosis that render.render() was carrying EEVEE state between calls. @theoryshaw retracted that diagnosis after reproducing on his real model, and found the actual cause. The restructure is fully reverted here; the net diff is one file.
Root cause
Blender.sync_render_visibility wrote hide_render with bpy.data.objects.foreach_set(...), which writes the DNA directly and skips the RNA update, so nothing tags the depsgraph. The evaluated visibility stays one render behind, and the renderer draws the previous drawing's object set with the current drawing's camera. That is why a ROOF plan came out as the 1ST FLOOR plan seen from the roof camera.
What made it so hard to see: visible_get(), obj.hide_render and context.visible_objects all read back the same DNA field that was just written, so every Python readback reported correct state. Only the evaluated, render-time visibility was stale.
Worth noting that the fix already existed in the file. The RNA assignment version, equality check included, was sitting commented out directly beneath the foreach_set with the note "For speed, check equality prior to change to prevent needless updates". It was bypassed deliberately for speed.
Fix
Assign hide_render through RNA so the depsgraph is tagged, with an equality check so only objects that actually change are written.
One deliberate difference from the commented-out block: it opened with if not obj.data: continue, which skipped objects with no data. That is not restored. An Empty's hide_render still matters when it drives collection instancing, and permanently skipping those objects means a wrong starting value is never corrected rather than merely corrected late. The loop now covers every object, matching the coverage the foreach_set path had.
Verification
The mechanism was confirmed directly rather than inferred. In an isolated Blender 5.2 profile, two overlapping planes, render, flip visibility in the same call stack, render again: via foreach_set the second render came out stale, via RNA assignment it rendered correctly. Notably there were zero event loop iterations between the two render.render() calls, which is independent confirmation that the previous modal restructure could never have fixed this.
Scope honesty, two points. The visual bug was not reproduced end to end on @theoryshaw's real model; what is demonstrated here is the mechanism in isolation. And the performance measurements taken here, on a synthetic 3041 object scene, came out two to three orders of magnitude smaller than his real model figures of 0.65s then 0.06s. The qualitative shape matches, with the equality check making repeat calls roughly six to seven times cheaper, but the absolute numbers do not, most likely because a flat synthetic scene lacks the nested collection hierarchy the hide_render update callback walks. His measurements on the real model are the ones to trust.
black and ruff clean, before and after.
Related, not changed here
src/bonsai/bonsai/bim/import_ifc.py:186 calls mesh.polygons.foreach_set("material_index", ...) with no mesh.update() in that method or its caller. Lower risk, since it runs on freshly created meshes, but it is the only other foreach_set site in Bonsai lacking the explicit update that the rest of them have. Left alone to keep this PR to one thing.
Produced with AI assistance.