Fixes for a few minor borders and outlines bugs (bevyengine#16071)

# Objective 1. Nodes with `Display::None` set are removed from the layout and have no position or size. Outlines should not be drawn for a node with `Display::None` set. 2. The outline and border colors are checked for transparency together. If only one of the two is transparent, both will get queued. 3. The `node.is_empty()` check is insufficient to check if a border is present since a non-zero sized node can have a zero width border. ## Solution 1. Add a check to `extract_uinode_borders` and ignore the node if `Display::None` is set. 2. Filter the border and outline optional components by `is_fully_transparent`. 3. Check if all the border widths are zero instead. ## Testing I added dark cyan outlines around the left and right sections in the `display_and_visibility` example. If you run the example and set the outermost node to `Display::None` on the right, then you'll see the that the outline on the left disappears.
grace125 · Oct 23, 2024 · c9a3f34 · c9a3f34
1 parent 7577895
commit c9a3f34
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 21 deletions.
diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs
@@ -5,8 +5,8 @@ mod ui_material_pipeline;
 pub mod ui_texture_slice_pipeline;
 
 use crate::{
-    BackgroundColor, BorderColor, CalculatedClip, ComputedNode, DefaultUiCamera, Outline,
-    ResolvedBorderRadius, TargetCamera, UiAntiAlias, UiBoxShadowSamples, UiImage, UiScale,
+    BackgroundColor, BorderColor, CalculatedClip, ComputedNode, DefaultUiCamera, Display, Node,
+    Outline, ResolvedBorderRadius, TargetCamera, UiAntiAlias, UiBoxShadowSamples, UiImage, UiScale,
 };
 use bevy_app::prelude::*;
 use bevy_asset::{load_internal_asset, AssetEvent, AssetId, Assets, Handle};
@@ -399,6 +399,7 @@ pub fn extract_uinode_borders(
     uinode_query: Extract<
         Query<(
             Entity,
+            &Node,
             &ComputedNode,
             &GlobalTransform,
             &ViewVisibility,
@@ -415,7 +416,8 @@ pub fn extract_uinode_borders(
 
     for (
         entity,
-        uinode,
+        node,
+        computed_node,
         global_transform,
         view_visibility,
         maybe_clip,
@@ -435,24 +437,22 @@ pub fn extract_uinode_borders(
             continue;
         };
 
-        // Skip invisible borders
-        if !view_visibility.get()
-            || maybe_border_color.is_some_and(|border_color| border_color.0.is_fully_transparent())
-                && maybe_outline.is_some_and(|outline| outline.color.is_fully_transparent())
-        {
+        // Skip invisible borders and removed nodes
+        if !view_visibility.get() || node.display == Display::None {
             continue;
         }
 
-        // don't extract border if no border or the node is zero-sized (a zero sized node can still have an outline).
-        if !uinode.is_empty() && uinode.border() != BorderRect::ZERO {
-            if let Some(border_color) = maybe_border_color {
+        // Don't extract borders with zero width along all edges
+        if computed_node.border() != BorderRect::ZERO {
+            if let Some(border_color) = maybe_border_color.filter(|bc| !bc.0.is_fully_transparent())
+            {
                 extracted_uinodes.uinodes.insert(
                     commands.spawn(TemporaryRenderEntity).id(),
                     ExtractedUiNode {
-                        stack_index: uinode.stack_index,
+                        stack_index: computed_node.stack_index,
                         color: border_color.0.into(),
                         rect: Rect {
-                            max: uinode.size(),
+                            max: computed_node.size(),
                             ..Default::default()
                         },
                         image,
@@ -463,8 +463,8 @@ pub fn extract_uinode_borders(
                             transform: global_transform.compute_matrix(),
                             flip_x: false,
                             flip_y: false,
-                            border: uinode.border(),
-                            border_radius: uinode.border_radius(),
+                            border: computed_node.border(),
+                            border_radius: computed_node.border_radius(),
                             node_type: NodeType::Border,
                         },
                         main_entity: entity.into(),
@@ -473,15 +473,20 @@ pub fn extract_uinode_borders(
             }
         }
 
-        if let Some(outline) = maybe_outline {
-            let outline_size = uinode.outlined_node_size();
+        if computed_node.outline_width() <= 0. {
+            continue;
+        }
+
+        if let Some(outline) = maybe_outline.filter(|outline| !outline.color.is_fully_transparent())
+        {
+            let outline_size = computed_node.outlined_node_size();
             let parent_clip =
                 maybe_parent.and_then(|parent| parent_clip_query.get(parent.get()).ok());
 
             extracted_uinodes.uinodes.insert(
                 commands.spawn(TemporaryRenderEntity).id(),
                 ExtractedUiNode {
-                    stack_index: uinode.stack_index,
+                    stack_index: computed_node.stack_index,
                     color: outline.color.into(),
                     rect: Rect {
                         max: outline_size,
@@ -495,8 +500,8 @@ pub fn extract_uinode_borders(
                         atlas_scaling: None,
                         flip_x: false,
                         flip_y: false,
-                        border: BorderRect::square(uinode.outline_width()),
-                        border_radius: uinode.outline_radius(),
+                        border: BorderRect::square(computed_node.outline_width()),
+                        border_radius: computed_node.outline_radius(),
                         node_type: NodeType::Border,
                     },
                     main_entity: entity.into(),

diff --git a/examples/ui/display_and_visibility.rs b/examples/ui/display_and_visibility.rs
@@ -1,7 +1,7 @@
 //! Demonstrates how Display and Visibility work in the UI.
 
 use bevy::{
-    color::palettes::css::{DARK_GRAY, YELLOW},
+    color::palettes::css::{DARK_CYAN, DARK_GRAY, YELLOW},
     prelude::*,
     winit::WinitSettings,
 };
@@ -187,6 +187,11 @@ fn spawn_left_panel(builder: &mut ChildBuilder, palette: &[Color; 4]) -> Vec<Ent
                                 ..default()
                             },
                             BackgroundColor(palette[0]),
+                            Outline {
+                                width: Val::Px(4.),
+                                color: DARK_CYAN.into(),
+                                offset: Val::Px(10.),
+                            },
                         ))
                         .with_children(|parent| {
                             parent.spawn(Node {
@@ -289,6 +294,11 @@ fn spawn_right_panel(
                         ..default()
                     },
                     BackgroundColor(palette[0]),
+                    Outline {
+                        width: Val::Px(4.),
+                        color: DARK_CYAN.into(),
+                        offset: Val::Px(10.),
+                    },
                 ))
                 .with_children(|parent| {
                     spawn_buttons(parent, target_ids.pop().unwrap());