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

Get main into a releasable state #2529

Closed
emilk opened this issue Jun 27, 2023 · 2 comments
Closed

Get main into a releasable state #2529

emilk opened this issue Jun 27, 2023 · 2 comments
Assignees
@jleibs
Labels
🟦 blueprint The data that defines our UI 🪳 bug Something isn't working 🧑‍💻 dev experience developer experience (excluding CI)

Comments

@emilk
Copy link
Member

emilk commented Jun 27, 2023 •
edited
Loading

Currently blocked by the blueprint not being serialized properly when closing and restarting the viewer, and maybe by other things too.

We don't have to be perfect, but get the big things in place.
@emilk emilk added 🪳 bug Something isn't working 🧑‍💻 dev experience developer experience (excluding CI) 🟦 blueprint The data that defines our UI labels Jun 27, 2023
@jleibs jleibs mentioned this issue Jun 30, 2023
Merged
3 tasks
jleibs added a commit that referenced this issue Jul 4, 2023
@jleibs
Basic persistence for blueprints (#2578)
52f51c1 
### What
When moving over to the new blueprint store, we temporarily lost the
ability to persist blueprints.

This is one of the major regressions currently preventing us from
releasing main (See: #2529)

This PR adds new methods to the `store_hub`:
 - `persist_app_blueprints`
 - `try_to_load_persisted_blueprint`
 
The blueprints are stored using the standard rrd file format. They go
into a sub-directory of our app-state storage:
- Linux: /home/UserName/.local/share/rerun/blueprints/
- macOS: /Users/UserName/Library/Application Support/rerun/blueprints/
- Windows: C:\Users\UserName\AppData\Roaming\rerun\blueprints\

We only store a single blueprint in this folder per app-id. 

Any time we change the app_id via the `store_hub` (such as when loading
an rrd) we will search the directory for a persisted blueprint and then
load it. We track the `insert_id` on the active blueprint to determine
if we need to save back out to disk again. I've tested to confirm that
in most cases when we restore from blueprint we don't do anything weird
like re-modify the blueprint and save it again.

### Additional Notes:
The way that DataBlueprint/AutoValues work required a workaround that I
don't particularly love, but does seems to be workable until we refactor
to get rid of more of the serde business.
- After implementing blueprint-loading, I noticed that every time we
start the app fresh, we see a cycle where the AutoValue for
`pinhole_image_plane_distance` goes from Auto(100.06751) -> Auto(1) ->
Auto(100.03121). Even ignoring the transient jump due to view bounds
being a frame delayed to populate, the Auto-value itself is unstable
from run to run. This meant that every time we started the app, we would
do a new insertion which would cause the blueprint to immediately get
saved again.
- The work-around is to replace the usage of `!=` (PartialEq) with a
conceptually equivalent method `has_edits` for the DataBlueprintTree. We
now consider all values of Auto to be considered a match. This seems to
work out fine since we (should?) always recompute Auto values at the
beginning of each frame.
- Longer term, I think the right solution is to move the Auto values out
of the blueprint and into the AppState so they persist frame-to-frame
and don't even show up in the blueprint comparison logic. However,
DataBlueprintTree is a complicated piece of work, and plumbing through
that state is proving to be challenging.
   
### Future work
This currently only implements file-backed storage, so doesn't work with
web-view.
 - #2579

### Testing:
* Run colmap fiat:
```
python examples/python/structure_from_motion/main.py --dataset colmap_fiat
```
* Adjust views
* Exit application
* Check the md5sum of the blueprint:
```
$ md5sum /home/jleibs/.local/share/rerun/blueprints/structure_from_motion.blueprint 
03fab330d9c23ac3da1163c854aa8518  /home/jleibs/.local/share/rerun/blueprints/structure_from_motion.blueprint
```
* Run colmap fiat a *second* time.
* Confirm the layout persists:

![image](https://github.com/rerun-io/rerun/assets/3312232/036d6ae9-6d3e-4abf-a981-cc46a7b6fe71)
* Close rerun and verify the blueprint hasn't been changed:
```
$ md5sum /home/jleibs/.local/share/rerun/blueprints/structure_from_motion.blueprint 
03fab330d9c23ac3da1163c854aa8518  /home/jleibs/.local/share/rerun/blueprints/structure_from_motion.blueprint
```


### 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 [demo.rerun.io](https://demo.rerun.io/pr/2578) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2578)
- [Docs
preview](https://rerun.io/preview/pr%3Ajleibs%2Fsave_blueprints/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Ajleibs%2Fsave_blueprints/examples)
@emilk
Copy link
Member Author

emilk commented Jul 10, 2023 •
edited
Loading

Mostly needs testing at this point, and documenting the minor regressions (no blueprint persistance on web)

@Wumpf Wumpf closed this as completed Jul 18, 2023
@Wumpf
Copy link
Member

Wumpf commented Jul 18, 2023 •
edited
Loading

TODO: 0.8 release task document regressions there. (@jleibs )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jleibs jleibs

Labels
🟦 blueprint The data that defines our UI 🪳 bug Something isn't working 🧑‍💻 dev experience developer experience (excluding CI)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@emilk @Wumpf @jleibs