Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix handling clears in transform cache #8872

Merged
merged 5 commits into from
Jan 31, 2025
Merged

Fix handling clears in transform cache #8872

merged 5 commits into from
Jan 31, 2025

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Jan 30, 2025 •
edited by teh-cmc
Loading

Related

What

Testing:

as before, most of this is added test code. Some clutter in the diff because I split add_chunk into add_non_static_chunk and add_static_chunk to make it easier to reason. Also this unfortunately rippled into minor interface change since I had to go with EntityPath in one spot where I used EntityPathHash before

@Wumpf Wumpf added 🦟 regression A thing that used to work in an earlier release exclude from changelog PRs with this won't show up in CHANGELOG.md labels Jan 30, 2025
@Wumpf Wumpf changed the title Andreas/clears in transform cache Fix handling clears in transform cache Jan 30, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 30, 2025 •
edited
Loading

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link Manifest
✅ 52ca859 https://rerun.io/viewer/pr/8872 +nightly +main

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

@teh-cmc teh-cmc self-requested a review January 31, 2025 08:06
@teh-cmc teh-cmc force-pushed the andreas/clears-in-transform-cache branch from 9fc9f2b to 024a23e Compare January 31, 2025 08:42
teh-cmc
teh-cmc approved these changes Jan 31, 2025
View reviewed changes
Copy link
Member

@teh-cmc teh-cmc left a comment

Choose a reason for hiding this comment

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

If there's a nasty edge-case lurking somewhere in there, I certainly won't catch it just by reading.
The general idea looks pretty legit to me.

crates/viewer/re_view_spatial/src/transform_cache.rs Outdated Show resolved Hide resolved
crates/viewer/re_view_spatial/src/transform_cache.rs Outdated Show resolved Hide resolved
crates/viewer/re_view_spatial/src/transform_cache.rs Outdated Show resolved Hide resolved
crates/viewer/re_view_spatial/src/transform_cache.rs Show resolved Hide resolved
@Wumpf Wumpf merged commit 7250c48 into main Jan 31, 2025
31 checks passed
@Wumpf Wumpf deleted the andreas/clears-in-transform-cache branch January 31, 2025 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teh-cmc teh-cmc teh-cmc approved these changes

Assignees
No one assigned
Labels
exclude from changelog PRs with this won't show up in CHANGELOG.md 🦟 regression A thing that used to work in an earlier release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TransformCache doesn't handle clears
2 participants
@Wumpf @teh-cmc