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

Add a gizmo-based overlay to show UI node outlines (Adopted) #11237

Merged
merged 71 commits into from
Mar 18, 2024

Conversation

pablo-lua
Copy link
Contributor

@pablo-lua pablo-lua commented Jan 6, 2024

Objective

Solution


Changelog

Added

  • Added debug_overlay mod to bevy_dev_tools
  • Added bevy_ui_debug feature to bevy_dev_tools

How to use

  • The user must use bevy_dev_tools feature in TOML
  • The user must use the plugin UiDebugPlugin, that can be found on bevy::dev_tools::debug_overlay
  • Finally, to enable the function, the user must set UiDebugOptions::enabled to true
    Someone can easily toggle the function with something like:
fn toggle_overlay(input: Res<ButtonInput<KeyCode>>, options: ResMut<UiDebugOptions>) {
   if input.just_pressed(KeyCode::Space) {
      // The toggle method will enable if disabled and disable if enabled
      options.toggle();
   }
}

Note that this feature can be disabled from dev_tools, as its in fact behind a default feature there, being the feature bevy_ui_debug.

Limitations

Currently, due to limitations with gizmos itself, it's not possible to support this feature to more the one window, so this tool is limited to the primary window only.

Showcase

image
Ui example with debug_overlay enabled

image
And disabled

@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets A-Gizmos Visual editor and debug gizmos C-Feature A new feature, making something new possible labels Jan 6, 2024
@alice-i-cecile
Copy link
Member

Do we keep the keybinding config?

IMO no, this doesn't justify the complexity here (for now). Just make sure there's a good way for users to toggle it from their own code.

Is it a good idea to move this to bevy_gizmos instead of bevy_ui?

Definitely belongs in bevy_ui for now. If the plans regarding a bevy_debug_tools first-party crate come to fruition, this is another prime candidate for inclusion and should be moved there.

@pablo-lua
Copy link
Contributor Author

IMO no, this doesn't justify the complexity here (for now)

Agreed, for now I'll remove the key binding, the user can easily enough enable or disable by setting DebugOptions::enabled = true / false.

@nicopap nicopap self-requested a review January 6, 2024 21:22
@66OJ66
Copy link
Contributor

66OJ66 commented Jan 8, 2024

It might be worth adding a toggle so that only Nodes which are children of a Visible root node have outlines applied.
(not too sure what the best approach would be though)

Reason for this: some Apps will create all their UI elements upfront under separate root nodes, then just show/hide these elements by changing the root node Visibility settings.

In this scenario, users could potentially see 100s of overlaid outlines which would make this new feature harder to use

@pablo-lua
Copy link
Contributor Author

Added this, for now, It will hide the outlines of anything that is not visible judging by ViewVisibility component, let me know if you guys think that there is another better approach in this case

@pablo-lua
Copy link
Contributor Author

Remembered about UiScale, I'll have a little look at it another hour, but this is a error so I'll throw this in draft until I fix it

@pablo-lua pablo-lua marked this pull request as draft January 9, 2024 01:33
@pablo-lua
Copy link
Contributor Author

With this fixed, only thing that matters for now is adding some docs, I'm not very good at this, will try tomorrow 👍

@pablo-lua pablo-lua marked this pull request as ready for review January 10, 2024 00:20
Copy link
Contributor

You added a new feature but didn't update the readme. Please run cargo run -p build-templated-pages -- update features to update it, and commit the file change.

@pablo-lua
Copy link
Contributor Author

I removed the feature from top level features. Added bevy_ui_debug as a feature of bevy_dev_tools, and now you have to add the plugin yourself, like in the example, but I'm not sure if thats alright, so I could use of another opinion on what we have now. I'll fix the CI now btw.

@alice-i-cecile
Copy link
Member

Hmm, if we add a dev-dependency on bevy in the bevy Cargo.toml with bevy_debug_tools enabled does that work?

This may still be better, as it prompts users that the flag exists though.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 11, 2024
@pablo-lua
Copy link
Contributor Author

pablo-lua commented Mar 11, 2024

Hmm, if we add a dev-dependency on bevy in the bevy Cargo.toml with bevy_debug_tools enabled does that work?

This may still be better, as it prompts users that the flag exists though.

That does work, but that can be strange. If for some reason you enable the feature, the dev_tools plugin will be auto Added by the feature in DefaultPlugins, so I thought it would be strange to have the dependencies "duplicated" in examples
If that's okay, I can change it though, but like you said, there is value in have it as a feature IMO

@alice-i-cecile
Copy link
Member

Yeah, I think this is better. Thanks for checking!

@alice-i-cecile alice-i-cecile added the M-Needs-Release-Note Work that should be called out in the blog due to impact label Mar 11, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Mar 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 18, 2024
github-merge-queue bot pushed a commit that referenced this pull request Mar 18, 2024
# Objective

- This is an adopted version of #10420
- The objective is to help debugging the Ui layout tree with helpful
outlines, that can be easily enabled/disabled

## Solution

- Like #10420, the solution is using the bevy_gizmos in outlining the
nodes

---

## Changelog

### Added
- Added debug_overlay mod to `bevy_dev_tools`
- Added bevy_ui_debug feature to `bevy_dev_tools`

## How to use
- The user must use `bevy_dev_tools` feature in TOML
- The user must use the plugin UiDebugPlugin, that can be found on
`bevy::dev_tools::debug_overlay`
- Finally, to enable the function, the user must set
`UiDebugOptions::enabled` to true
Someone can easily toggle the function with something like:

```rust
fn toggle_overlay(input: Res<ButtonInput<KeyCode>>, options: ResMut<UiDebugOptions>) {
   if input.just_pressed(KeyCode::Space) {
      // The toggle method will enable if disabled and disable if enabled
      options.toggle();
   }
}
```

Note that this feature can be disabled from dev_tools, as its in fact
behind a default feature there, being the feature bevy_ui_debug.

# Limitations
Currently, due to limitations with gizmos itself, it's not possible to
support this feature to more the one window, so this tool is limited to
the primary window only.

# Showcase


![image](https://github.com/bevyengine/bevy/assets/126117294/ce9d70e6-0a57-4fa9-9753-ff5a9d82c009)
Ui example with debug_overlay enabled


![image](https://github.com/bevyengine/bevy/assets/126117294/e945015c-5bab-4d7f-9273-472aabaf25a9)
And disabled

---------

Co-authored-by: Nicola Papale <[email protected]>
Co-authored-by: Pablo Reinhardt <[email protected]>
Co-authored-by: Alice Cecile <[email protected]>
Copy link
Contributor

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

@pablo-lua
Copy link
Contributor Author

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

Third time in this PR.

@pablo-lua
Copy link
Contributor Author

@alice-i-cecile Should be alright now, main Ci passed.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Mar 18, 2024
Merged via the queue into bevyengine:main with commit 1af9bc8 Mar 18, 2024
30 checks passed
@pablo-lua pablo-lua deleted the debug-ui-rects branch March 18, 2024 18:26
github-merge-queue bot pushed a commit that referenced this pull request Mar 26, 2024
# Objective

#11237 added some new text to the UI example. Unlike the other text
sharing the same container just above, this new text has no padding and
straddles the edge of the screen.

## Solution

Move the padding to the container, and add `row_gap` so nodes placed in
the container get some vertical separation as well.

Before / After
<img width="320" alt="12567-before (1)"
src="https://github.com/bevyengine/bevy/assets/200550/de0aa142-c715-4c57-b607-d1bdc5d20a01">
<img width="320" alt="12567-after"
src="https://github.com/bevyengine/bevy/assets/200550/70b5c9db-9cb2-4f92-88b0-83590ea838b0">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Dev-Tools Tools used to debug Bevy applications. A-Gizmos Visual editor and debug gizmos A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible M-Needs-Release-Note Work that should be called out in the blog due to impact S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants