WIP: Viewer refactor #16
Open
Conversation
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
Update and reimport legacy code to fit in new structure Implemented HashId Fixed Rust2024 match errors There's still a bug when zooming in/out on partitions that are not the origin, hence excess logging in graph_controller.rs. I think it's the order of operations when you change the anchor partition that's to blame, but I wanted to get this on the repo before Monday.
- Add update_camera() for viewport-preserving camera positioning - Add update_cursor_from_viewport() using existing coordinate adapters - Add preserve_cursor_during_layout_change() helper for layout changes - Create new simplified rebuild_viewport_graph() - pure graph builder - Keep legacy update_viewport_graph() for backward compatibility - Refactor disperse(), contract(), set_detail_level() to use new helpers - Reduce code duplication and improve separation of concerns 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add test_hash_id() helper using SHA256 to convert integers to HashId - Fix partition_table.rs tests to use HashId for node_id field - Fix edge_label_validation_test.rs to use HashId - Fix Rust 2024 pattern matching in partition_controller.rs tests - Add sha2 dev dependency for test helper function Tests now compile successfully with 181/200 passing. The 2 failing layout tests (test_skip_layer_terminal_stitch_edge_bundles and viewport_multi_partition_boundary_handling) appear to be pre-existing issues unrelated to the graph_controller refactor. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Replace custom SHA256-based test_hash_id() with HashId::pad_str() in test files. Remove unnecessary sha2 dependency. This aligns with existing codebase patterns and simplifies test fixture creation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Replace all calls to update_viewport_graph with rebuild_viewport_graph - Update production code in graph_widget.rs and graph_algorithms.rs - Update all test files and example programs - Remove unused cmp import from graph_controller.rs - Tests that were failing before these changes continue to fail (not introduced by this change) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The legacy update_viewport_graph had cursor restoration logic that ran after rebuilding the viewport graph. When it was removed, cursor navigation broke because the cursor was not being restored to its tracked node in the newly rebuilt viewport graph. Added cursor restoration logic to graph_widget.rs that: - Runs after rebuild_viewport_graph when rebuild_needed is true - Calls restore_cursor_after_viewport_update to find tracked node - Falls back to initialize_cursor if restoration fails This ensures cursor maintains its position as it navigates and triggers viewport rebuilds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Migrate from world-coordinate-based cursor to viewport-coordinate-based cursor system, eliminating complex position restoration logic. ## Key Changes ### Architecture - Remove CursorState from ViewportState - Add ViewportCursor as sibling field in GraphController - Create cursor_adapter.rs with bridge methods ### ViewportCursor (cursor_v2.rs) - Store viewport coordinates as primary (screen-space) - Derive world coordinates from viewport + node tracking - Store fractional position [0.0-1.0] within nodes - Support smooth animations in world space - Navigation via move_horizontal/move_vertical ### Camera Delta Adjustment Replace "restore cursor position" with automatic camera compensation: 1. Before layout change: w1 = cursor.to_world_pos() 2. Rebuild layout with new parameters 3. After layout change: w2 = cursor.to_world_pos() 4. Adjust camera: camera += (w2 - w1) Result: Cursor stays at same viewport position, camera compensates for world coordinate changes. ### Animation System Integration - Add Animation field to ViewportCursor - Update ViewportState::update() signature to accept cursor + viewport_graph - Multizone camera following uses cursor.get_viewport_pos() directly - Tests updated for new API ### Updated Files - src/cursor_v2.rs: New cursor implementation (877 lines, 13 tests) - src/cursor_adapter.rs: GraphController bridge methods - src/animation.rs: Updated for ViewportCursor integration - src/viewport_state.rs: Removed CursorState - src/graph_controller.rs: Camera delta in set_detail_level/disperse/contract - src/graph_widget.rs: Updated cursor rendering - src/lib.rs: Disabled legacy cursor module 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Added GraphController::update_animations() to provide a clean API for advancing camera and cursor animations without borrowing conflicts. Updated parent crate views (block_group.rs, inline_gen_graph_widget.rs) to use the new API instead of directly calling viewport_state.update(). Cleaned up unused imports in cursor_adapter, graph_controller, and graph_widget. Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The camera following logic was causing infinite movement because it used the stored cursor viewport position, which doesn't update when the camera moves. This created a feedback loop where camera movements triggered more movements. Changed animation.rs:142 to calculate cursor viewport position dynamically from its world position and current camera state, rather than using the stale stored value. The cursor's world position remains stable, so the viewport position now correctly reflects where the cursor appears on screen as the camera moves. This eliminates the feedback loop - when camera moves to follow cursor, the cursor's recalculated viewport position accounts for the camera movement, preventing spurious additional camera adjustments. Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
dkhofer
approved these changes
Oct 8, 2025
Member
dkhofer
left a comment
dkhofer
left a comment
There was a problem hiding this comment.
I'm good with these changes. Summary of stuff I'd like to see addressed in a followup PR:
- Rename edge_router_rs to edge_router
- Please move the sugiyama stuff into a separate subcrate, gen-sugiyama I guess. The initial subcrate is big enough already, and makes it a little clearer that we'd prefer to use the public sugiyama crate if it ever gains the functionality we need.
Thanks!
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.
Update and reimport legacy code to fit in new structure Implemented HashId
Fixed Rust2024 match errors
There's still a bug when zooming in/out on partitions that are not the origin, hence excess logging in graph_controller.rs. I think it's the order of operations when you change the anchor partition that's to blame, but I wanted to get this on the repo before Monday.