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

fix shadow rendering for tiny-skia backend #2715

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

T-256
Copy link
Contributor

@T-256 T-256 commented Jan 2, 2025

Fixes #2712

After commits bisecting, I found the issued came out from #2382 (commit 1e802e7).
This PR adds bounds_with_shadow to Quad.

I verified it by running cargo r -p custom_quad.

TODO

@B0ney
Copy link
Contributor

B0ney commented Jan 2, 2025

I don't think this is an optimal fix. Since now the software renderer would redraw everything every update, leading to much higher CPU usage.

@T-256
Copy link
Contributor Author

T-256 commented Jan 2, 2025

I don't think this is an optimal fix

Agree.
Then I think there should be problem with layers determining. because I still have shadows shown beyond scrollable area even after with this PR.

@B0ney
Copy link
Contributor

B0ney commented Jan 2, 2025

Looking at the issues you linked, I think it might be that the damage regions aren't taking the shadow's blur radius + offset into consideration.

This means that as the damaged areas are redrawn, because the regions aren't including the shadows, they won't get cleared properly.

Consequently, the renderer will draw over them (additively) after every update. This repeated stacking might explain why the shadows get darker.

@dtzxporter
Copy link
Contributor

Looking at this, it may make sense to abstract the bounds calculation to "layout bounds" and "render bounds" where one deals with collision/layout, and the other is the bounds that will be modified when rendering, for dirty tracking, etc.

@T-256
Copy link
Contributor Author

T-256 commented Jan 2, 2025

Looking at this, it may make sense to abstract the bounds calculation to "layout bounds" and "render bounds" where one deals with collision/layout, and the other is the bounds that will be modified when rendering, for dirty tracking, etc.

This approach makes sense to me, but I'm unsure how to implement it, could be great to have at least a PR to demonstrate how it effects on performance.

@T-256
Copy link
Contributor Author

T-256 commented Jan 3, 2025

To reproduce scrollable bug with buttons with shadow:

Click to expand
use iced::widget::{button, column, container, scrollable, text};
use iced::{
    color,
    widget::button::{Status, Style},
    window::Settings,
    Shadow,
};
use iced::{Length, Theme};

pub trait StyleUtils {
    fn with_shadow(self, shadow: impl Into<Shadow>) -> Self;
}

impl StyleUtils for Style {
    fn with_shadow(self, shadow: impl Into<Shadow>) -> Self {
        Self {
            shadow: shadow.into(),
            ..self
        }
    }
}

#[derive(Default, Debug, Clone)]
enum Message {
    #[default]
    Test,
}

fn main() -> iced::Result {
    iced::application("shadow", |_: &mut i32, _| {}, view)
        .window(Settings {
            size: (300.0, 250.0).into(),
            resizable: false,
            ..Default::default()
        })
        .run()
}

fn view(_state: &i32) -> iced::Element<Message> {
    let mut c = column![];
    for _ in 0..10 {
        let shadow_button = button(text("foo").size(20)).style(|theme: &Theme, status: Status| {
            button::primary(theme, status).with_shadow(Shadow {
                color: color!(0x000000),
                offset: [2.0, 5.0].into(),
                blur_radius: 5.0,
            })
        });
        c = c.push(shadow_button.on_press(Message::Test));
    }
    container(scrollable(c.spacing(20).padding(20)))
        .center(Length::Fill)
        .padding(50)
        .into()
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[tiny-skia] Changes affected to shadows needs to have full redraw
3 participants