[Merged by Bors] - Added multi windows check for bevy_ui `Interaction`. by fadhliazhari · Pull Request #5225 · bevyengine/bevy

fadhliazhari · 2022-07-06T10:26:52Z

Objective

Currently bevy_ui only checks for primary window cursor position to determine Interaction behavior.
Added checks for focused window where cursor position is available.
Fixes Allow bevy_ui to handle multiple window #5224.

Solution

Added checks for focused windows in Interaction focus system.

Follow Up

All windows with camera will be rendering the UI elements right now.
We will need some way to tell which camera to render which UI.

`Interaction` focus system will get first available window's cursor position instead of just primary window.

nicopap

There is a situation that this handles poorly:

You have two windows:

Window 1 with UI elements
Window 2 without the UI elements

When the mouse hovers over Window 2 I believe, this patch will activate the button under the corresponding position in Window 1.

I suggest this alternative: get the Window for windows.get(id) where id is the target: RenderTarget::Window(WindowId) field of the Camera component of the entity with the CameraUi marker component. Probably requires adding a query to ui_focus_system or store the ID in an additional resource.

This is more difficult, because you can have multiple ui cameras. I think currently, it would be fine to take the windows id of the first ui camera and keep the issue open. A design that accounts for all ui cameras might be way harder to handle properly.

crates/bevy_ui/src/focus.rs

Now the focus system will consider the currently focused window instead of all window

nicopap

I think we should limit the query to stuff with a CameraUi component. Otherwise, we might pick up other types of camera.

crates/bevy_ui/src/focus.rs

nicopap · 2022-07-06T12:59:29Z

crates/bevy_ui/src/focus.rs

+            if let Some(&CameraUi {
+                is_enabled: false, ..
+            }) = camera_ui
+            {
+                return None;
+            }


With the change suggested on the query parameter, you can replace this with if !camera_ui.is_enabled.

The user are able to add in any camera without this component.

crates/bevy_ui/src/focus.rs

nicopap · 2022-07-06T14:45:59Z

Give me a minute and I'll give you a third review.

nicopap

LGTM I'd like this merged

crates/bevy_ui/src/focus.rs

This enables having different UI per camera. With bevyengine#5225, this enables having different interactive UIs per window. Although, to properly complete this, the focus system will need to account for RenderLayer of UI nodes.

alice-i-cecile · 2022-07-11T14:04:48Z

bors r+

# Objective - Currently bevy_ui only checks for primary window cursor position to determine `Interaction` behavior. - Added checks for focused window where cursor position is available. - Fixes #5224. ## Solution - Added checks for focused windows in `Interaction` focus system. ## Follow Up - All windows with camera will be rendering the UI elements right now. - We will need some way to tell which camera to render which UI. --- Co-authored-by: fadhliazhari <44402264+fadhliazhari@users.noreply.github.com>

bors · 2022-07-11T14:35:54Z

Pull request successfully merged into main.

Build succeeded:

This enables having different UI per camera. With bevyengine#5225, this enables having different interactive UIs per window. Although, to properly complete this, the focus system will need to account for RenderLayer of UI nodes.

# Objective - Currently bevy_ui only checks for primary window cursor position to determine `Interaction` behavior. - Added checks for focused window where cursor position is available. - Fixes bevyengine#5224. ## Solution - Added checks for focused windows in `Interaction` focus system. ## Follow Up - All windows with camera will be rendering the UI elements right now. - We will need some way to tell which camera to render which UI. --- Co-authored-by: fadhliazhari <44402264+fadhliazhari@users.noreply.github.com>

Added multi windows check for bevy_ui Interaction.

2a6aac0

`Interaction` focus system will get first available window's cursor position instead of just primary window.

Nilirad added A-Windowing Platform-agnostic interface layer to run your app in A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jul 6, 2022

nicopap suggested changes Jul 6, 2022

View reviewed changes

crates/bevy_ui/src/focus.rs Outdated Show resolved Hide resolved

bevy_ui Interaction focus system for focused window

6bfa8f8

Now the focus system will consider the currently focused window instead of all window

nicopap suggested changes Jul 6, 2022

View reviewed changes

Small refactor to bevy_ui focus system

5b92198

nicopap approved these changes Jul 6, 2022

View reviewed changes

crates/bevy_ui/src/focus.rs Show resolved Hide resolved

bevy_ui refactor focus system

2a13465

NiklasEi approved these changes Jul 6, 2022

View reviewed changes

alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Jul 7, 2022

ManevilleF approved these changes Jul 7, 2022

View reviewed changes

fadhliazhari and others added 3 commits July 8, 2022 11:36

Merge branch 'bevyengine:main' into multiple-window-ui-interaction

393eae3

Rename CameraUi to UiCameraConfig

c435089

Merge branch 'bevyengine:main' into multiple-window-ui-interaction

cc7482b

nicopap mentioned this pull request Jul 8, 2022

User UI camera control #5252

Closed

alice-i-cecile added C-Bug An unexpected or incorrect behavior and removed C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Jul 11, 2022

bors bot changed the title ~~Added multi windows check for bevy_ui Interaction.~~ [Merged by Bors] - Added multi windows check for bevy_ui Interaction. Jul 11, 2022

bors bot closed this Jul 11, 2022

fadhliazhari deleted the multiple-window-ui-interaction branch July 12, 2022 08:31

nicopap mentioned this pull request Jul 21, 2022

UI render layers #5414

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Merged by Bors] - Added multi windows check for bevy_ui `Interaction`.#5225

[Merged by Bors] - Added multi windows check for bevy_ui `Interaction`.#5225
fadhliazhari wants to merge 7 commits intobevyengine:mainfrom
fadhliazhari:multiple-window-ui-interaction

fadhliazhari commented Jul 6, 2022 •

edited

Loading

Uh oh!

nicopap left a comment •

edited

Loading

Uh oh!

Uh oh!

nicopap left a comment

Uh oh!

Uh oh!

nicopap Jul 6, 2022 •

edited

Loading

Uh oh!

fadhliazhari Jul 6, 2022

Uh oh!

Uh oh!

nicopap commented Jul 6, 2022

Uh oh!

nicopap left a comment

Uh oh!

Uh oh!

alice-i-cecile commented Jul 11, 2022

Uh oh!

bors bot commented Jul 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

fadhliazhari commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Solution

Follow Up

Uh oh!

nicopap left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nicopap left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nicopap Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fadhliazhari Jul 6, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nicopap commented Jul 6, 2022

Uh oh!

nicopap left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alice-i-cecile commented Jul 11, 2022

Uh oh!

bors bot commented Jul 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

fadhliazhari commented Jul 6, 2022 •

edited

Loading

nicopap left a comment •

edited

Loading

nicopap Jul 6, 2022 •

edited

Loading