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

[Bug] IFullScreenRenderWindowInitialValues is missing rootContainer property #3116

Open
josealvarez97 opened this issue Aug 26, 2024 · 4 comments
Labels
type: bug 🐞 Errors in functionality

Comments

@josealvarez97
Copy link

Bug description

The example Using vtk.js with React has rootContainer as the property, and indeed, if one doesn't specify rootContainer (but something else like container), the example will not work well.

Steps to reproduce

But it was confusing because TypeScript complains that the IFullScreenRenderWindowInitialValues doesn't have the rootContainer property. It should have it, since the library source code actually expects it.

There's no rootContainer on next-app/node_modules/@kitware/vtk.js/Rendering/Misc/FullScreenRenderWindow.d.ts

image

Detailed Behavior

No response

Expected Behavior

But rootContainer is referenced multiple times in next-app/node_modules/@kitware/vtk.js/Rendering/Misc/FullScreenRenderWindow.js

image

Therefore, this is just a small TypeScript issue.

Environment

  • vtk.js version: "@kitware/vtk.js": "^32.1.0",
  • Browsers: Safari
  • OS: Mac
@josealvarez97 josealvarez97 added the type: bug 🐞 Errors in functionality label Aug 26, 2024
@Kitware Kitware deleted a comment Aug 26, 2024
@jourdain
Copy link
Collaborator

The fullScreenRenderWindow was made to shorten examples code so we could focus on what that example was for and not setting the rendering stack.

The fullScreenRenderWindow aim to be full screen and therefore don't have a container by default. Of course we broke that rule to allow us to inject a container at construction. But either way, that fullScreenRenderWindow should not be used outside of examples.

@finetjul
Copy link
Member

@jourdain should we modify the documentation https://kitware.github.io/vtk-js/docs/vtk_react.html to use a GenericRenderWindow instead of a FullScreenRenderWindow ?

@josealvarez97
Copy link
Author

Thank you for the insight @jourdain

Actually @finetjul , yesterday it was very hard to adapt this example to React https://kitware.github.io/vtk-js/examples/ManyRenderers.html

@jourdain
Copy link
Collaborator

Indeed the react example should not use FullScreenRenderWindow. Using the core VTK classes should be the way to go. For real react showcase, you can look at https://github.com/Kitware/react-vtk-js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐞 Errors in functionality
Projects
None yet
Development

No branches or pull requests

5 participants
@finetjul @jourdain @josealvarez97 and others