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

Scene view component #2947

Merged
merged 19 commits into from
May 6, 2024
Merged

Conversation

fabian0702
Copy link
Contributor

This PR contains the newly created ui.scene_view element as discussed in PR #2608. This would allow to have multiple cameras for the same scene, but from multiple perspectives at the same time.

Copy link
Contributor

@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

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

Hi @fabian0702, thanks again for pushing this new feature forward!

There are still some issue though:

  1. Text renderers don't seem to work. Adding something like scene.text('Hello, world!') or scene.text3d('Hello, world!') to a ui.scene causes rapid flickering when a ui.scene_view is shown on the same page.
  2. Both pytests are currently failing.
  3. I added camera parameters to ui.scene on main, which we should add to ui.scene_view before merging. But this shouldn't be too hard to integrate.

Can you, please, have another look into it? Thanks!

@falkoschindler falkoschindler added the enhancement New feature or request label Apr 25, 2024
@fabian0702
Copy link
Contributor Author

@falkoschindler I fixed the problems you mentioned and added the necessary camera parameters. Unfortunately I had to remove the text renderer from the scene_view component due to the way text is currently rendered.

Copy link
Contributor

@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

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

Great! I think this feature is ready to merge. Thanks again, @fabian0702!

Regarding the text renders: It looks like they can't handle rendering a scene into another div. So I removed them altogether and added a note about this limitation to the documentation of ui.scene_view.

And I think I found a simpler way to wait for the parent scene. Awaiting this.$nextTick() seems to work well in all automated and manual tests. What do you think?

@falkoschindler falkoschindler added this to the 1.4.24 milestone May 3, 2024
@falkoschindler falkoschindler merged commit 48b6f76 into zauberzeug:main May 6, 2024
1 check passed
@fabian0702
Copy link
Contributor Author

@falkoschindler I'm all for simplifying the code. The rest of the changes look good to me, thank you for the improvements.

@fabian0702 fabian0702 deleted the SceneView-Component branch May 6, 2024 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants