Skip to content

Comments

Fix transforms & videos breaking when receiving new data in the background#12452

Merged
Wumpf merged 3 commits intomainfrom
andreas/fix-outdated-caches
Jan 15, 2026
Merged

Fix transforms & videos breaking when receiving new data in the background#12452
Wumpf merged 3 commits intomainfrom
andreas/fix-outdated-caches

Conversation

@Wumpf
Copy link
Member

@Wumpf Wumpf commented Jan 14, 2026

Related

What

This keeps all caches up to date.
We first tried discarding them which also worked fine, but decided to go with keeping things up to date since most likely those recordings are still in used. If it gets too much memory used, GC will do.

@Wumpf Wumpf requested review from IsseW and emilk January 14, 2026 18:59
@Wumpf Wumpf added 🪳 bug Something isn't working 📺 re_viewer affects re_viewer itself include in changelog consider-patch PRs & issues that should be considered to be cherry-picked to a patch release branch. and removed consider-patch PRs & issues that should be considered to be cherry-picked to a patch release branch. labels Jan 14, 2026
@github-actions
Copy link

github-actions bot commented Jan 14, 2026

Web viewer built successfully.

Result Commit Link Manifest
7203596 https://rerun.io/viewer/pr/12452 +nightly +main

View image diff on kitdiff.

Note: This comment is updated whenever you push a commit.

Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original problem is that not all store events reach all the cashes.

For instance, it seems like the transform cache relies on knowing about all store additions:

if event.kind == re_chunk_store::ChunkStoreDiffKind::Addition {
// Since entity paths lead to implicit frames, we have to prime our lookup table with them even if this chunk doesn't have transform data.
self.frame_id_registry
.register_all_frames_in_chunk(&event.chunk);
}

It seems like Cache::on_store_events were added in other to be able to selectively invalidate on store deletions, but some caches abuse it to also look as store additions, effectively becoming store subscribers rather than caches.

Unless we change the interface of trait Cache to explicitly only pass deletions, then we must face the possibility that some "caches" will need to get all store events, and not just some of them.

@Wumpf
Copy link
Member Author

Wumpf commented Jan 14, 2026

The original problem is that not all store events reach all the cashes.

so but if we don't have a cache to begin with it should work fine since upon initial creation of any of these caches we restore all the data from the store. So I do think this is a viable approach!

Relevant "examples":

I meant to draft this PR though since I didn't have a repro before/after yet as noted on Slack, that's an oversight on my part!

@Wumpf Wumpf marked this pull request as draft January 14, 2026 21:52
Wumpf added 2 commits January 14, 2026 22:54
…round

This discards unused caches if their stores receives new data, rationale in the comment
@Wumpf Wumpf force-pushed the andreas/fix-outdated-caches branch from a0b6dbc to a65732f Compare January 14, 2026 21:54
@emilk
Copy link
Member

emilk commented Jan 15, 2026

upon initial creation of any of these caches we restore all the data from the store

Oh I missed that! Then this should work.

I will try to repro and test

@emilk
Copy link
Member

emilk commented Jan 15, 2026

I created this issue, which may be related: https://linear.app/rerun/issue/RR-3342/debug-crash-video-santity-check

Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me and @Wumpf reproed the problem like so:

diff --git a/docs/snippets/all/archetypes/transform3d_hierarchy_frames.py b/docs/snippets/all/archetypes/transform3d_hierarchy_frames.py
index 9aa3ab4acf..f2b03fd1f4 100644
--- a/docs/snippets/all/archetypes/transform3d_hierarchy_frames.py
+++ b/docs/snippets/all/archetypes/transform3d_hierarchy_frames.py
@@ -1,5 +1,6 @@
 """Logs a transform hierarchy using named transform frame relationships."""
 
+import time
 import numpy as np
 import rerun as rr
 
@@ -22,6 +23,10 @@ rr.log(
     rr.CoordinateFrame("sun_frame"),
 )
 
+print("Sleeping…")
+time.sleep(3.0)
+print("Slept")
+
 rr.log(
     "planet",
     rr.Ellipsoids3D(
(base) 

(switch away from the recording during the sleep).

And this PR fixes the problem!

@Wumpf Wumpf requested a review from emilk January 15, 2026 15:37
@Wumpf
Copy link
Member Author

Wumpf commented Jan 15, 2026

as discussed: alternative approach of always updating the caches. Confirmed with above snippet

@Wumpf Wumpf marked this pull request as ready for review January 15, 2026 15:39
@Wumpf Wumpf merged commit e4ea965 into main Jan 15, 2026
45 checks passed
@Wumpf Wumpf deleted the andreas/fix-outdated-caches branch January 15, 2026 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🪳 bug Something isn't working include in changelog 📺 re_viewer affects re_viewer itself

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants