feat(view_state): JSON save/restore for camera + render-look state#6
Merged
Conversation
…h, private buffers Applies findings from /simplify code review: - Replace `mode: String` with typed `GroundPlaneMode` / `TransparencyMode` enums in RenderState DTOs. Adds `#[serde(rename_all = "snake_case")]` to both enum derives. Removes parse_*/_name helper pairs and KNOWN_GP/KNOWN_TR validation lists (~50 lines). - `#[derive(Default)]` on the four Partial* DTOs collapses the merge() function's None-literal placeholders (~25 lines). - Single-lock per-frame publish: extract `App::view_state_from_options` taking borrowed Options, called inside one with_context_mut closure. Removes the double lock acquisition from the per-frame path. - `apply_view_state` now only sets `camera_fitted = true` when an engine exists — prevents silent disabling of future auto-fit before engine init. - Context buffer fields are now fully private; access via `set_*`/`take_*`/ reader accessors + test-only `clear_view_buffers`/`pending_view_apply`. - Removed stale `#[allow(dead_code)]` annotations from App methods. - Removed redundant `App::current_view_state` (public callers use the free function in core; internal callers use `view_state_from_options` directly).
There was a problem hiding this comment.
Pull request overview
Adds a versioned JSON save/restore mechanism for the current “view” (camera pose + render-look settings) with both programmatic APIs and UI buttons, including support for deterministic headless rendering.
Changes:
- Introduces
ViewState/RenderState/ViewTransitionDTOs plussave_view_*/load_view_*APIs backed by Context snapshot + pending-apply buffers. - Wires App frame loop + headless rendering to drain queued view applies and publish per-frame view snapshots.
- Adds UI “Save View…” / “Load View…” actions (native dialogs via
rfd), integration tests, and an example demo.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/view_state_demo.rs | New example demonstrating save/load workflow via UI buttons. |
| docs/plans/2026-05-19-view-save-restore-implementation.md | Detailed implementation plan for the feature. |
| docs/plans/2026-05-19-view-save-restore-design.md | Design spec documenting ownership, JSON shape, and APIs. |
| docs/feature-status.md | Marks view save/restore as completed and removes from planned work. |
| crates/polyscope/tests/view_state_headless.rs | Headless integration test ensuring queued view state is honored (auto-fit skipped). |
| crates/polyscope/src/headless.rs | Drains pending view apply in headless renders and publishes snapshot post-render. |
| crates/polyscope/src/app/view_state.rs | App-side gather/apply of distributed view state (camera + render/look + options). |
| crates/polyscope/src/app/render.rs | Drains pending view apply at frame start; publishes snapshot at frame end. |
| crates/polyscope/src/app/render_ui.rs | Handles new ViewAction intents and opens rfd dialogs after egui passes. |
| crates/polyscope/src/app/mod.rs | Registers the new view_state submodule. |
| crates/polyscope/Cargo.toml | Adds rfd/serde_json deps and registers the new example. |
| crates/polyscope-ui/src/panels.rs | Adds new ViewAction variants and Save/Load buttons in controls section. |
| crates/polyscope-render/Cargo.toml | Adds serde (and serde_json) to support camera state serialization tests/DTO. |
| crates/polyscope-render/src/camera.rs | Adds CameraState DTO + apply logic (Instant/FlyTo) + conversions to core DTO. |
| crates/polyscope-render/src/lib.rs | Re-exports CameraState. |
| crates/polyscope-core/src/view_state.rs | Core view-state DTOs, JSON/file APIs, partial merge, and validation. |
| crates/polyscope-core/src/state.rs | Adds Context buffers + accessors for snapshot/pending apply. |
| crates/polyscope-core/src/ssao.rs | Adds PartialEq to SsaoConfig for DTO/test comparisons. |
| crates/polyscope-core/src/options.rs | Adds snake_case serde rename for TransparencyMode. |
| crates/polyscope-core/src/ground_plane.rs | Adds snake_case serde rename for GroundPlaneMode. |
| crates/polyscope-core/src/error.rs | Adds InvalidViewState and NoActiveView error variants + tests. |
| crates/polyscope-core/src/lib.rs | Exposes view_state module and re-exports its public API. |
| CHANGELOG.md | Documents the new view save/restore feature and APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| state.render.background_color[2], | ||
| ); | ||
|
|
||
| self.ground_plane.mode = state.render.ground_plane.mode; |
Comment on lines
+297
to
+306
| let finite_arr = |a: &[f32; 3]| a.iter().all(|x| x.is_finite()); | ||
| if !finite_arr(&s.camera.position) { | ||
| return invalid("camera.position has non-finite values"); | ||
| } | ||
| if !finite_arr(&s.camera.target) { | ||
| return invalid("camera.target has non-finite values"); | ||
| } | ||
| if !finite_arr(&s.camera.up) { | ||
| return invalid("camera.up has non-finite values"); | ||
| } |
Comment on lines
21
to
25
| thiserror.workspace = true | ||
| bytemuck.workspace = true | ||
| serde = { workspace = true } | ||
| serde_json = { workspace = true } | ||
| rand = "0.8" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds JSON save/restore of the camera pose + render-look state (background, ground plane, transparency, SSAO, SSAA). Ports upstream C++ Polyscope's `getViewAsJson`/`setViewFromJson` (commit `7570a40` + FOV fix in PR #389) — but with a Rust-native idiomatic API.
Design
Full design spec lives at `docs/plans/2026-05-19-view-save-restore-design.md` (V2 after codex review caught state-ownership mistakes in V1).
Core mechanism: two new private buffers on `Context` — `view_state_snapshot` (App-published, frame-by-frame) and `pending_view_apply` (queued by callers, App-consumed). This lets free functions reach Camera (which lives in `RenderEngine`, not `Context`) without breaking the layering.
JSON shape:
```json
{
"version": 1,
"camera": { "position": [..], "target": [..], "up": [..], "fov": .., "near": .., "far": ..,
"projection_mode": "perspective", "ortho_scale": ..,
"navigation_style": "turntable", "up_direction": "pos_y", "front_direction": "neg_z" },
"render": { "background_color": [..], "ground_plane": { "mode": "tile_reflection", .. },
"transparency": { "mode": "simple", .. }, "ssao": {..}, "ssaa_factor": 1 }
}
```
Commits (14, TDD throughout)
Test Plan
Notes
🤖 Generated with Claude Code