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

box-shadow clipping fix #16790

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Dec 12, 2024

Objective

Instead of clipping the non-visable sections of box-shadows, the shadow is scaled to fit into the remaining area after clipping because the normalized coordinates that are meant to border the unclipped subsection of the shadow are always set to [Vec2::ZERO, Vec2::X, Vec2::ONE, Vec2::Y],

Solution

Calculate the coordinates for the corners of the visible area.

Testing

Test app:

use bevy::color::palettes::css::RED;
use bevy::color::palettes::css::WHITE;
use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .run();
}

fn setup(mut commands: Commands) {
    commands.spawn(Camera2d);
    commands
        .spawn(Node {
            ..Default::default()
        })
        .with_children(|commands| {
            commands
                .spawn((
                    Node {
                        width: Val::Px(100.),
                        height: Val::Px(100.),
                        margin: UiRect {
                            left: Val::Px(100.),
                            top: Val::Px(300.),
                            ..Default::default()
                        },
                        overflow: Overflow::clip(),
                        ..Default::default()
                    },
                    BackgroundColor(WHITE.into()),
                ))
                .with_children(|commands| {
                    commands.spawn((
                        Node {
                            position_type: PositionType::Absolute,
                            left: Val::Px(50.),
                            top: Val::Px(50.),
                            width: Val::Px(100.),
                            height: Val::Px(100.),
                            ..Default::default()
                        },
                        BackgroundColor(RED.into()),
                        BoxShadow::from(ShadowStyle {
                            x_offset: Val::ZERO,
                            y_offset: Val::ZERO,
                            spread_radius: Val::Px(50.),
                            blur_radius: Val::Px(6.),
                            ..Default::default()
                        }),
                    ));
                });
        });
}

Main:
bad_shadow

This PR:
clipped_shadow

@ickshonpe ickshonpe requested a review from UkoeHB December 12, 2024 19:57
@ickshonpe ickshonpe added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 12, 2024
@ickshonpe ickshonpe added this to the 0.15.1 milestone Dec 12, 2024
Copy link
Contributor

@UkoeHB UkoeHB left a comment

Choose a reason for hiding this comment

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

Btw possibly related to this PR, this line looks odd to me:

let positions = QUAD_VERTEX_POSITIONS
    .map(|pos| (box_shadow.transform * (pos * rect_size).extend(1.)).xyz());

Shouldn't it be transform + (pos * rect_size)?

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Dec 12, 2024

Btw possibly related to this PR, this line looks odd to me:

let positions = QUAD_VERTEX_POSITIONS
    .map(|pos| (box_shadow.transform * (pos * rect_size).extend(1.)).xyz());

Shouldn't it be transform + (pos * rect_size)?

transform is a 4x4 matrix so addition doesn't work here, to apply the matrix transformation we have to extend the scaled position to a Vec4 and multiply. The matrix here normally represents a translation though in which case it does all cancel out to just an addition between two Vec3s.

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 A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants