-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Add support for world space UI #21812
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
base: main
Are you sure you want to change the base?
Conversation
…obalTransfrom;Correct the coordinate system of UiContain Children Node
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
…the Contain Node; Add some tests
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
|
The example needs to be run with the "bevy_ui_container" feature enabled: Otherwise it doesn't work correctly. |
example required features bevy_ui_container Co-authored-by: ickshonpe <david.curthoys@googlemail.com>
fix word error Co-authored-by: ickshonpe <david.curthoys@googlemail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the feature gate should be removed, world UI is fairly fundamental and probably not something that should be feature gated.
And also just for this PR, it adds a lot of extra noise that makes the changes more difficult to review.
fix word error Co-authored-by: ickshonpe <david.curthoys@googlemail.com>
Yes, perhaps we need to open a new create or plugin? |
| }); | ||
| #[cfg(feature = "bevy_ui_container")] | ||
| { | ||
| let Ok(view) = camera_views.get(default_camera_view.ui_container) else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look right, if you run:
cargo run --example testbed_ui --features bevy/bevy_ui_container
there are visual artifacts which seem to result from drawing some of the phase items twice?
There are similar problems with the other queue_* functions.
|
@ickshonpe |
crates/bevy_ui/src/container.rs
Outdated
| let Ok((scale, size)) = query_ui_scale.get(target.0) else { | ||
| return; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it "return"? Shouldn't it be "continue"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rendering code for the UiContainer still has some issues. This is related to the UiCameraView, which necessitates the existence of both UiCamera and UiContainer. I plan to decouple them to reduce dependency and improve performance.
|
I don't want this feature-gated either. I think it adds complexity, and makes it harder to reason about how layout works. |
Since the majority of the members agree, I plan to remove the feature gate. |
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
1 similar comment
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
|
Judging from your modifications to UiSurface and UiScale, it appears that your PR lacks a systematic approach to bevy_ui. Instead, it seems you made changes based on a targeted look at certain parts of the code, resulting in several adjustments that do not align with Bevy’s current architecture and design principles. Therefore, I suggest you first explain your design approach, discuss it with the team, and then proceed with the changes before submitting the PR. |
Yes, there is indeed this issue. My design solution is based on an idea: since the UI is laid out relative to the camera—where the camera acts as a "container for the UI"—and this container always follows the camera, why not define a container in world space to hold the UI and make the UI follow this container? For this purpose, I designed UiContainerSize, which corresponds to the layout size of the camera. |
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
1 similar comment
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
Preface
Added feature 'bevy_ui_container' to ensure normal functioning of other functions
Objective
Fixes #21278Try #5476
Existing problems:
performance optimization...
Expectations and difficulties:
Non root nodes take effect (May affect the entire Node)UiContain supports Visibility (Restricted by the relationship of visibility)Solution
1.Ui follows the world from a point (Transfrom) to move
2.Modify Ui Layout Size Source (UiContainerSize)
3.Place Node in UiContainer for rendering(Use UiContainerTarget to specify Container)
Testing
Did you test these changes? If so, how?
I changed the UiSurface Resource that Node relies on in the test to Component, and its result is consistent with the Camera Node.
Are there any parts that need more testing?
Showcase