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

rounded rectangle graphics #2491

Merged
merged 1 commit into from
Sep 12, 2024
Merged

rounded rectangle graphics #2491

merged 1 commit into from
Sep 12, 2024

Conversation

gintsgints
Copy link
Contributor

@gintsgints gintsgints commented Jul 2, 2024

This is my attempt to add functionality for additional canvas shape, rounded rectangle.

@vladh
Copy link
Contributor

vladh commented Jul 4, 2024

I don't think this approach is flexible enough. For example, I would expect drawing a square with a very high border radius to result in a circle. This is what happens in iced::Border::rounded and also in CSS. However, in your implementation, a buggy shape is drawn.

For example, here's the result of drawing examples/graphics with a radius of 80:

2024-07-04-12-16-51

@gintsgints
Copy link
Contributor Author

Hmm, so what should flexible solution do instead?
I can make radius relative to the width of the rectangle or disallow too big radius.

@vladh
Copy link
Contributor

vladh commented Jul 4, 2024

Hmm, so what should flexible solution do instead?

It looks like, at the very least, you want something like:

radius = min(radius, min(height / 2, width / 2))

See:

It would also be nice to be able to specify horizontal and vertical radius, and to specify individual radii for each of the 4 corners, by using or extending the existing iced::Border::Radius.

@gintsgints
Copy link
Contributor Author

One commit implement just min check, other use Radius as argument, so it is possible to specify different radius for each corner.

@@ -54,7 +54,7 @@ impl Border {
/// The border radii for the corners of a graphics primitive in the order:
/// top-left, top-right, bottom-right, bottom-left.
#[derive(Debug, Clone, Copy, PartialEq, Default)]
pub struct Radius([f32; 4]);
pub struct Radius(pub [f32; 4]);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this needs to be pub — the from() methods can be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, you are right. Fixed.

@@ -47,6 +47,12 @@ impl Path {
Self::new(|p| p.rectangle(top_left, size))
}

/// Creates a new [`Path`] representing a rounded rectangle given its top-left
/// corner coordinate, its `Size` and radius.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe:

its [`Size`] and border [`Radius`]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -160,6 +161,51 @@ impl Builder {
self.close();
}

/// Adds a rounded rectangle to the [`Path`] given its top-left
/// corner coordinate its `Size` and curve radius.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe:

its [`Size`] and border [`Radius`]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@vladh
Copy link
Contributor

vladh commented Jul 5, 2024

This looks okay to me right now, but I'm not a maintainer so you'll have to wait for one to look at this.

@hecrj hecrj added this to the 0.13 milestone Sep 12, 2024
@hecrj hecrj added feature New feature or request widget canvas addition labels Sep 12, 2024
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!

I have removed the example since we already have too many.

@hecrj hecrj enabled auto-merge September 12, 2024 22:59
@hecrj hecrj merged commit f392f4a into iced-rs:master Sep 12, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants