Point a cached ID at the node that actually wrote it - #85
Merged
Conversation
0.27.0 gave cached bodies fresh originalIDs so two identical shapes stopped being one thing to select, but each fresh ID inherited the node of whichever call site was generated first. The viewport then separated the two cylinders while the editor highlighted one line for either. Which node a fresh ID should carry depends on what the old one stood for. An ID standing for the reused node itself belongs to the node reusing it -- two pieces of source wrote the same shape. An ID standing for something deeper inside, the cube and sphere within a module called twice, keeps its own node: both calls really do come from the one body that spells those parts out, and re-pointing them at the call site collapses every part of a call onto a single line -- the same defect, one level down. Telling the two apart needs the node whose generation filled the entry, so record it per cache key for the render's duration. Also fixes a second case 0.27.0 had backwards: across renders the old IDs predate this render's idToNode, so nothing was written for them at all and geometry served from an earlier evaluate carried no source node. A re-render of an unchanged file is all cache hits, which left nothing in the viewport pickable. Attribution now matches an uncached render exactly for identical primitives at different call sites, reused modules, and reused differences. CacheHitSkipsRegeneratingTheSubtree was asserting the empty idToNode the second bug produced; it now reads the skip through the child's own bodies, which is what it meant all along. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #84. Selecting either of two identical cylinders highlighted the same line in the editor.
#84 gave cached bodies fresh originalIDs, but each fresh ID inherited the node of whichever call site was generated first — so the viewport separated the two cylinders while the editor pointed both at line 1.
Which node a fresh ID carries now depends on what the old one stood for:
cubeandspherein a module called twice keep their own nodes, since both calls really do come from that one module body. Re-pointing those at the call site was my first attempt and collapses every part of a call onto one line: the same defect, one level down.Telling those apart needs the node whose generation filled the entry, recorded per cache key for the render's duration (cleared with
idToNode, whose pointers share its lifetime).Second bug, also from #84
Across renders the old IDs predate the current
idToNode, so nothing was written for them — geometry served from an earlier evaluate carried no source node at all. A re-render of an unchanged file is all cache hits, which left nothing in the viewport pickable.Verified
Attribution now matches an uncached render exactly:
[1, 2][1, 2][2, 3, 2, 3][2, 3, 2, 3][1, 2, 3][1, 2, 3]difference()[1, 1, 1, 1][1, 1, 1, 1]805/805 C++ tests pass. Each new assertion was negative-controlled — always-call-site, always-old-node, never-serve-from-cache, and #84's exact code each fail their own test and no others.
Known ceiling
Two subtrees identical down to their transforms still share one attribution: only the inner node is a cache hit, and the producer check keeps the first copy's descendants. They occupy the same space, so there is nothing to select apart.
🤖 Generated with Claude Code