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

Eliminate sync_viewport_blueprint #4440

Closed
jleibs opened this issue Dec 5, 2023 · 0 comments
Closed

Eliminate sync_viewport_blueprint #4440

jleibs opened this issue Dec 5, 2023 · 0 comments
Assignees
Labels
🟦 blueprint The data that defines our UI 🚜 refactor Change the code, not the functionality

Comments

@jleibs
Copy link
Member

jleibs commented Dec 5, 2023

The current blueprint design allows the UI to edit a mutable copy of the blueprint and then syncs all the changes back to the store at the end of each frame.

The new design uses SystemCommand::UpdateBlueprint to send changes back to the store.

To finish this migration, we need to refactor blueprint lifecycle, construction and component patterns for the remaining types:

  • Each type: ViewportBlueprint, ContainerBlueprint, SpaceViewBlueprint, DataQueryBlueprint should all follow the same design. They are generally considered to be read-only views of the blueprint on the current frame. They should always store their own BlueprintId, which is mappable to an EntityPath used to store any edits. These edits are always deferred through the CommandSender and won't be visible until the next frame. The basic pattern should look like:
    /// Create a new ephemeral blueprint entity.
    ContainerBlueprint::new() -> ContainerBlueprint;
    
    /// Send the initial state of an ephemeral blueprint entity to the store
    /// Should never be used on a non-ephemeral blueprint entity such as created via `from_store`
    ContainerBlueprint::save_initial(deferred: &CommandSender);
    
    /// Remove the entity and all of its children
    /// Note: because blueprint entities aren't stored hierarchically this will generate multiple clear events.
    ContainerBlueprint::clear(deferred: &CommandSender);
    
    /// Load an actual blueprint component from the store.
    /// Recursively loads all the referenced types.
    ContainerBlueprint::from_store(store: &StoreDb, id: ContainerId) -> ContainerBlueprint {...}
    
    /// Modify some field.
    /// Used by UI events to modify the blueprint component. Encapsulates all 
    ContainerBlueprint::edit_foo(foo: Foo, deferred: &CommandSender)
    

When this is complete, sync_viewport_blueprint should no longer be necessary.

@jleibs jleibs added 🚜 refactor Change the code, not the functionality 🟦 blueprint The data that defines our UI labels Dec 5, 2023
@jleibs jleibs self-assigned this Dec 12, 2023
jleibs added a commit that referenced this issue Dec 15, 2023
### What

Resolves:
 - #4155
 - #4440

Lots of refactoring and untangling here:
- `ViewportBlueprint` is a thin wrapper around the Archetype -- it is
now always read-only.
- All the runtime-mutable stuff (specifically the tree /
deferred-tree-actions) are moved out of ViewportBlueprint and into
Viewport
- Subsequently a couple of the UI eliminate now take a `&mut Viewport`
instead of a &mut ViewportBlueprint
- Almost all modifications are now made by calling `set_<prop>` APIs
which emit a deferred component-write directly to the blueprint.
- The one remaining complex type is the ViewportLayout which is still
stored as a single component. The deferred update logic for the tree is
now done at the end of the frame, followed by a single component-update.
   
### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
  * Full build: [app.rerun.io](https://app.rerun.io/pr/4524/index.html)
* Partial build:
[app.rerun.io](https://app.rerun.io/pr/4524/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
- Useful for quick testing when changes do not affect examples in any
way
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4524)
- [Docs
preview](https://rerun.io/preview/db702308b1658fe691878c7976b445ba5623b72a/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/db702308b1658fe691878c7976b445ba5623b72a/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
@jleibs jleibs closed this as completed Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🟦 blueprint The data that defines our UI 🚜 refactor Change the code, not the functionality
Projects
None yet
Development

No branches or pull requests

1 participant