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.

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

Optionally include static chunks in the chunk store range query for al… #9044

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zehiko
Copy link
Contributor

@zehiko zehiko commented Feb 14, 2025

^.

@zehiko zehiko self-assigned this Feb 14, 2025
Copy link

github-actions bot commented Feb 14, 2025

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

  • I have tested the web viewer
Result Commit Link Manifest
20d634d https://rerun.io/viewer/pr/9044 +nightly +main

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

Comment on lines +790 to +806
if include_static {
let static_chunks = self
.static_chunk_ids_per_entity
.get(entity_path)
.map(|static_chunks_per_component| {
static_chunks_per_component
.values()
.filter_map(|chunk_id| self.chunks_per_chunk_id.get(chunk_id).cloned())
.collect_vec()
})
.unwrap_or_default();

debug_assert!(static_chunks.iter().map(|chunk| chunk.id()).all_unique());
static_chunks.into_iter().chain(chunks).collect()
} else {
chunks
}
Copy link
Member

Choose a reason for hiding this comment

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

You need to do this first, and then for every component that didn't get any static chunks (and only those), extend the results with the existing temporal logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that makes sense, tnx

@teh-cmc
Copy link
Member

teh-cmc commented Feb 14, 2025

(Reminder to do the same for latest_at_relevant_chunks_for_all_components)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants