Introduce versioned EntityPath & refactor mesh/tensor caching
#3230
Conversation
teh-cmc
changed the title
EntityPath & refactor mesh/tensor caching (sunset MeshId/TensorId)EntityPath & refactor mesh/tensor caching
teh-cmc
changed the title
EntityPath & refactor mesh/tensor cachingEntityPath & refactor mesh/tensor caching
teh-cmc
added
🐍 Python API
| /// | ||
| /// This assumes that the row we get from the store only contains a single instance for this | ||
| /// component; it will log a warning otherwise. | ||
| /// | ||
| /// This should only be used for "mono-components" such as `Transform` and `Tensor`. | ||
| /// | ||
| /// This is a best-effort helper, it will merely log errors on failure. | ||
| pub fn query_latest_component<C: Component>( | ||
| pub fn query_latest_component_and_row_id<C: Component>( |
There was a problem hiding this comment.
Rather than "component_and_row_id" what if this the result-type was just a "VersionedComponent"?
pub fn query_latest_component<C: Component>(
&self,
entity_path: &EntityPath,
query: &LatestAtQuery,
) -> Option<VersionedComponent<C>>
There was a problem hiding this comment.
Interesting. I'll try and see how this looks.
There was a problem hiding this comment.
I've implemented it and I think it's a nice enough improvement as it at least prevents the combinatorial explosion of query_latest_* APIs.
Overall the whole thing is definitely not great, there is so much more we could do if the notion of versioning was more tightly integrated (or rather: integrated at all) all over the codebase overall but... gotta 🚢 !
| @@ -348,7 +348,7 @@ impl<A: Archetype> ArchetypeView<A> { | |||
| components: impl IntoIterator<Item = ComponentWithInstances>, | |||
| ) -> Self { | |||
| Self { | |||
| row_id, | |||
| primary_row_id: row_id, | |||
There was a problem hiding this comment.
The correct caching behavior of Tensor depends on this id being the id of the TensorData component. This will currently be the case for our definitions of Image / Tensor but may not always be the case if we introduce an archetype with an optional tensor.
I suspect this means we should do the book-keeping through so that every ComponentWithInstances knows its own RowId.
Since it's not a blocker today we can probably get away with just an issue / comment.
There was a problem hiding this comment.
Yes! I actually meant to add a comment about that and forgot. Will do.
| @@ -61,19 +61,22 @@ pub struct PerTensorState { | |||
|
|
|||
| /// Last viewed tensor, copied each frame. | |||
| /// Used for the selection view. | |||
| tensor: Option<DecodedTensor>, | |||
| tensor: Option<(VersionedInstancePathHash, DecodedTensor)>, | |||
There was a problem hiding this comment.
I think we can simplify this by having DecodedTensor track the RowId itself.
There was a problem hiding this comment.
The issue is that DecodedTensor is completely unaware of Rerun and can be constructed from e.g. a DynamicImage and nothing else.
Which I think is a good thing really, even if that wasn't the case I wouldn't feel too great about coupling this to specific query patterns.
Commit by commit
What the title says.
The most important contribution of this PR is the new
VersionedInstancePath[Hash]types, which further specializes anInstancePath[Hash](itself a specialization ofEntityPath) with extra versioning information in the form of aRowIdfrom the store.I.e. a
VersionedInstancePathgives you a specific instance of an entity at a specific path at a specific time and row in the store, which is as specific as it gets.This allows us to completely remove
MeshId&TensorId, and paves the way for generic frame-based caching.What
Checklist