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

Refactor WebXR Rendering to vtkWebXRRenderWindowHelper #2924

Merged
merged 6 commits into from
Oct 26, 2023

Conversation

tbirdso
Copy link
Contributor

@tbirdso tbirdso commented Sep 22, 2023

Context

Under previous experimentation WebXR rendering support was fully contained within the vtkOpenGLRenderWindow. This refactoring encapsulates that support in the new class vtkWebXRRenderWindowHelper. That class accepts a reference to an underlying render window and is responsible for driving XR operations.

Refactoring allows for better separation of concepts and could enable XR rendering with alternate rendering backends other than vtkOpenGLRenderWindow in the future.

Results

No feature changes. XR methods are removed from the vtkOpenGLRenderWindow interface. An application aiming to use WebXR with vtk.js must now instantiate a vtkWebXRRenderWindowHelper to drive XR behavior.

Changes

  • Documentation and TypeScript definitions were updated to match those changes
  • Added vtkWebXRRenderWindowHelper
  • Removed XR interface from vtkOpenGLRenderWindow

PR and Code Checklist

  • semantic-release commit messages
  • Run npm run reformat to have correctly formatted code

Testing

  • Tested environment:
    • vtk.js: master
    • OS: Windows 10
    • Browser: Chrome 117.0.0.0

Additional Notes

Additional effort may be required for refactoring WebXR interaction handling.

@tbirdso
Copy link
Contributor Author

tbirdso commented Oct 5, 2023

@floryst ping for review

EDIT: Also rebased on master

Refactors WebXR rendering support. Under previous behavior WebXR rendering was embedded in
vtkOpenGLRenderWindow. This change removes the dependency of vtkOpenGLRenderWindow on WebXR and
moves XR components into vtkWebXRRenderWindowHelper. The helper class holds a reference to an
underlying render window and is responsible to start/run/stop XR rendering.

BREAKING CHANGE: Removes WebXR API from `vtkOpenGLRenderWindow`. Applications requiring XR support
must instantiate `vtkWebXRRenderWindowHelper` instead.
Updates WebXR examples for refactoring to `vtkWebXRRenderWindowHelper`.
Replaces `global` definition of XRGLLayer with imported definition from
`window`.

Addresses observed issue in VolView development where the `global`
definition is not yet available at call time.
…pplication logic

Updates `vtkWebXRRenderWindowHelper` interface to expose the underlying XR session and to emit the
VTK "modified" event when state changes. Motivated by subsequent VolView application development.
…nd public interface

Improves TypeScript definitions and removes previous publicAPI methods for setting the XR session
and rendering an XR frame. Motivated by subsequent application development in VolView.
Add WebXR TypeScript definitions to vtkWebXRRenderWindowHelper to resolve CI failures.
Copy link
Collaborator

@floryst floryst left a comment

Choose a reason for hiding this comment

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

LGTM!

@floryst
Copy link
Collaborator

floryst commented Oct 12, 2023

Since this is a breaking change, we will merge it along with the rest of v29 PRs.

@floryst floryst added this to the v29 milestone Oct 12, 2023
@tbirdso
Copy link
Contributor Author

tbirdso commented Oct 26, 2023

@floryst There is interest in making this available in a VTK.js release sooner rather than later. @aylward made the point this morning that breaking changes are limited to XR users, anyone who was previously using vtkOpenGLRenderWindow without XR will not notice behavioral changes.

Would you be open to merging this today? Or, when do you expect v29 to release?

@floryst
Copy link
Collaborator

floryst commented Oct 26, 2023

I'll start work to cut a v29 release today.

@floryst floryst changed the base branch from master to v29-merge October 26, 2023 15:56
@floryst floryst merged commit a0d5625 into Kitware:v29-merge Oct 26, 2023
4 checks passed
@tbirdso tbirdso mentioned this pull request Nov 15, 2023
46 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants