Skip to content

feat: [Remote rendering 3.2a] server-tracked camera: record, load-path write, and re-serialization - #110

Merged
LKasianAnsys merged 10 commits into
mainfrom
feat/3.2a-server-tracked-camera
Sep 22, 2026
Merged

LKasianAnsys merged 10 commits into
mainfrom
feat/3.2a-server-tracked-camera

Conversation

@LKasianAnsys

@LKasianAnsys LKasianAnsys commented Sep 14, 2026 •

Copy link
Copy Markdown
Collaborator

Issue

Addresses #20

Context

This is the first of 4 PRs for user story 3.2 (#20) of the phased implementation plan (ADR here) for adding remote rendering in VISOR.

Until now the server never applied camera state to its copy of the VTK pipeline: the browser owned it, and when save_state was called, the server had to fetch it via a round trip to the client, as its own pipeline was stale.

This PR adds a camera record to each renderer, populates it from the loaded state in apply_state, and applies it to the pipeline camera via sync_camera. It also adds serialize_camera_state, called immediately after sync_camera inside the lock. That call re-serializes the render window's object graph so that the client receives the camera that was just written rather than the copy before the load. Without it, the loaded camera is correct on the server and then replaced on the canvas by the stale copy. See the note below; this is not specific to the camera and impacts the following stories in the remote rendering Phase 3 work.

To verify: load a saved state with an off-axis camera and refresh. The framing should persist. To see the failure mode, comment out the serialize_camera_state() call in apply_state and load again.

Nothing reads the record yet. reset_camera writes it, and the save path still uses the client round trip; that changes in PR 3.2b (#311). Gesture attribution on the client is in PR 3.2c, and the report back to the server is added in PR 3.2d.

Regression test xfail

The test_load_state_into_empty_scene regression test was marked xfail(run=False) on Linux. It started failing on this branch, but investigation showed that load_state into an empty scene has never rendered on Linux (#122, reproduced on main and the old tree on the internal repo before the remote rendering work). The failure was not visible before this branch. The test is not run on Linux, rather than expected to fail, because the faulty load leaves the shared server unusable for the tests that follow it. This will be revisited in #24, when the full save/load state workflow is verified on top of the server-authoritative viewer state.


Copilot summary

This pull request significantly refactors and documents the camera state management contract for renderers, clarifying the roles of the pipeline camera and the camera record, and ensuring consistent, testable behavior across both production and test implementations. It also introduces comprehensive tests for these behaviors.

Camera state management improvements:

  • The IRenderer interface now clearly distinguishes between the pipeline camera (the VTK camera object) and the camera record (the authoritative state), with detailed docstrings explaining the contract for reset_camera, get_camera_state, sync_camera, and the new serialize_camera_state method.
  • The NullRenderer now maintains a camera record, implementing the record half of the contract even though it has no pipeline camera, and provides detailed documentation for each method. [1] [2] [3]

VisorLocalRenderer implementation updates:

  • Implements the clarified contract: reset_camera writes the pipeline camera and then updates the record from it; sync_camera stores the given object and projects it onto the pipeline camera; serialize_camera_state refreshes the serialization cache for the camera without notifying the client. Private helpers _read_pipeline_camera and _apply_to_pipeline_camera are added for clarity and testability.

Scene loading and camera state:

  • When loading a persisted scene, the loaded camera is now written to the record and projected onto the pipeline camera before rendering, and the serialization cache is refreshed to ensure the client receives the correct state.

Testing improvements:

  • Introduces a hand-written camera double (_CameraDouble) for unit tests, ensuring that camera state read/write logic is exercised with controlled, non-default values, and that setter calls can be asserted for both order and value.
  • Updates the renderer fixture to use the camera double, ensuring tests are meaningful and not vacuously passing due to default values.

Imports and type hints:

  • Moves the VisorCameraState import out of TYPE_CHECKING blocks to allow runtime use in both production and test code. [1] [2] [3]

These changes together ensure that camera state is handled consistently, robustly, and transparently across the codebase, and that both production and test code reflect the same contract for camera management.

@github-actions github-actions Bot added test Work associated with testing added enhancement New feature or request labels Sep 14, 2026
@LKasianAnsys
LKasianAnsys force-pushed the feat/3.2a-server-tracked-camera branch 5 times, most recently from 60c8952 to f3da08b Compare September 15, 2026 03:56
@github-actions github-actions Bot added the maintenance Operation not directly changing the production code - e.g., updating a devops pipeline label Sep 15, 2026
@LKasianAnsys
LKasianAnsys force-pushed the feat/3.2a-server-tracked-camera branch from 0e76bfb to 88d9ee9 Compare September 15, 2026 16:20
@github-actions github-actions Bot removed the maintenance Operation not directly changing the production code - e.g., updating a devops pipeline label Sep 15, 2026
@LKasianAnsys
LKasianAnsys force-pushed the feat/3.2a-server-tracked-camera branch 2 times, most recently from d7298e1 to 500a079 Compare September 16, 2026 17:20
@LKasianAnsys
LKasianAnsys force-pushed the feat/3.2a-server-tracked-camera branch from 500a079 to d5b9247 Compare September 16, 2026 17:49
@LKasianAnsys
LKasianAnsys force-pushed the feat/3.2a-server-tracked-camera branch from 7a76263 to a4b1c6b Compare September 16, 2026 18:13

@ansBAkula ansBAkula left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LKasianAnsys Cool stuff! I tested it, and it seems to be working as described.

Just one comment regarding the apply_state method naming. The method does more than simply apply/load a state; it also restores and synchronizes the server camera from the previously saved camera state, making the server authoritative for camera settings. Given that broader responsibility, the current name may not fully reflect its behavior.

A more descriptive name might make sense here. What do you think?

@LKasianAnsys LKasianAnsys left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ansBAkula Thanks for testing it! And yes I see what you mean on apply_state, it was structured in a way that was a bit confusing. Added a reply inline and made a small change to the method structure - let me know what you think!

Comment thread src/ansys/visor/viewer/vtk/scene/base.py

@ansBAkula ansBAkula left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

@LKasianAnsys
LKasianAnsys merged commit ebd27ff into main Sep 22, 2026
19 of 20 checks passed
@LKasianAnsys
LKasianAnsys deleted the feat/3.2a-server-tracked-camera branch September 22, 2026 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

added enhancement New feature or request test Work associated with testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants