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: quad rendering including border only inside of the bounds #1843

Merged
merged 5 commits into from
Jun 27, 2023

Conversation

wash2
Copy link
Contributor

@wash2 wash2 commented May 10, 2023

It seems that border should not be drawn outside the quad bounds. For example, a slider handle with a large border radius will leave a trail of the border color when it is dragged, because it goes un-tracked in the damage.

This change also makes tiny-skia rendering of quads more consistent with the wgpu backend, which didn't seem to draw outside the bounds as far as i could tell with the same slider test.

@hecrj hecrj added bug Something isn't working rendering labels May 11, 2023
@hecrj hecrj added this to the 0.10.0 milestone May 11, 2023
@hecrj hecrj deleted the branch iced-rs:master May 11, 2023 14:45
@hecrj hecrj closed this May 11, 2023
@hecrj hecrj reopened this May 11, 2023
@hecrj hecrj changed the base branch from advanced-text to master May 11, 2023 15:04
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Thanks. Good catch! Just a small detail.

Comment on lines 176 to 195
// Make sure the border radius is not larger than the bounds
let border_width = border_width
.min(bounds.width / 2.0)
.min(bounds.height / 2.0);

// Offset the fill by the border width
let path_bounds = Rectangle {
x: bounds.x + border_width,
y: bounds.y + border_width,
width: bounds.width - 2.0 * border_width,
height: bounds.height - 2.0 * border_width,
};
// fill border radius is the border radius minus the border width
let mut fill_border_radius = *border_radius;
for radius in &mut fill_border_radius {
*radius = (*radius - border_width / 2.0)
.min(path_bounds.width / 2.0)
.min(path_bounds.height / 2.0);
}
let path = rounded_rectangle(path_bounds, fill_border_radius);
Copy link
Member

Choose a reason for hiding this comment

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

I'd expect the border to be rendered on top of the background like a border-box in CSS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, so I guess that would mean not offsetting the fill rectangle?

@wash2
Copy link
Contributor Author

wash2 commented May 11, 2023

Hmm this might need some more testing. I'm going to mark it as a draft until i've tested it some more 😅. Specifically, if the border radius is smaller than half the border width, the path doesn't seem to work well.

@wash2 wash2 marked this pull request as draft May 11, 2023 16:33
@wash2 wash2 marked this pull request as ready for review May 11, 2023 23:16
@wash2
Copy link
Contributor Author

wash2 commented May 16, 2023

This seems to be working pretty well now.

@wash2 wash2 requested a review from hecrj May 16, 2023 14:18
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Thanks!

@hecrj hecrj enabled auto-merge June 27, 2023 20:07
@hecrj hecrj merged commit f63a9d1 into iced-rs:master Jun 27, 2023
@wash2 wash2 deleted the fix-tiny-skia-quad branch June 27, 2023 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rendering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants