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

Use stack_z_offsets in all the cases we create a TransparentUi #16197

Merged
merged 2 commits into from
Nov 4, 2024

Conversation

doup
Copy link
Contributor

@doup doup commented Oct 31, 2024

Objective

Use same pattern when creating TransparentUi items where the sort_key is the UiNode stack index + some offset.

Solution

Refactored to follow same pattern.

Testing

Ran few UI examples.

Doubts

Maybe stack_z_offsets::BACKGROUND_COLOR should be renamed. This is used for ExtractedUiNode, which is not only used for "background color" it's also used to render borders, images and text (I think).

@@ -75,14 +75,16 @@ pub mod graph {
/// When this is _not_ possible, pick a suitably unique index unlikely to clash with other things (ex: `0.1826823` not `0.1`).
///
/// Offsets should be unique for a given node entity to avoid z fighting.
/// These should pretty much _always_ be larger than -1.0 and smaller than 1.0 to avoid clipping into nodes
/// These should pretty much _always_ be larger than -0.5 and smaller than 0.5 to avoid clipping into nodes
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically not true (could be -0.3, 0.7), but more safe than the suggested -1.0 to 1.0 range since with 0.5 there wouldn't be overlap between two consecutive nodes.

@BenjaminBrienen BenjaminBrienen added A-Rendering Drawing game state to the screen D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 1, 2024
@alice-i-cecile 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 and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 4, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Nov 4, 2024
Merged via the queue into bevyengine:main with commit 718688e Nov 4, 2024
30 checks passed
@doup doup deleted the use-ui-stack-z-offsets branch November 5, 2024 10:18
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Dec 2, 2024
…evyengine#16197)

# Objective

Use same pattern when creating `TransparentUi` items where the
`sort_key` is the `UiNode` stack index + some offset.

## Solution

Refactored to follow same pattern.

## Testing

Ran few UI examples.

## Doubts

Maybe `stack_z_offsets::BACKGROUND_COLOR` should be renamed. This is
used for `ExtractedUiNode`, which is not only used for "background
color" it's also used to render borders, images and text (I think).
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
…evyengine#16197)

# Objective

Use same pattern when creating `TransparentUi` items where the
`sort_key` is the `UiNode` stack index + some offset.

## Solution

Refactored to follow same pattern.

## Testing

Ran few UI examples.

## Doubts

Maybe `stack_z_offsets::BACKGROUND_COLOR` should be renamed. This is
used for `ExtractedUiNode`, which is not only used for "background
color" it's also used to render borders, images and text (I think).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants