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

Add and improve Rect methods #117

Merged
merged 1 commit into from
Mar 12, 2024
Merged

Conversation

danieldg
Copy link
Contributor

@danieldg danieldg commented Mar 9, 2024

This adds Rect::join and is_empty (matching skia's) and improves the speed of from_points and transform.

@danieldg
Copy link
Contributor Author

danieldg commented Mar 9, 2024

I was considering adding another Rect struct, possibly named RawRect that had public fields (and therefore did not maintain validity guarantees like Rect). This is useful to postpone and combine validity checks if you are doing several operations (for example: create a rect from a list of points, transform it, outset it, intersect it with another one, and then draw using it). If this seems like something that would be useful to add (it does match skia's Rect, and might be useful internally too), then I'll put it on this branch too. Otherwise I'll just mark this ready.

path/src/rect.rs Outdated
Some(*self)
} else if ts.has_skew() {
// we need to transform all 4 corners
let tl = Point {
Copy link
Owner

Choose a reason for hiding this comment

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

Point::from_xy would be formatted more nicely.

@RazrFalcon
Copy link
Owner

Thanks. Looks good to me.

As for RawRect, I thought about it before, but I don't see it being useful in tiny-skia. The current Rect overhead would never become a bottleneck.

This adds Rect::join and is_empty (matching skia's) and improves the
speed of from_points and transform.
@danieldg
Copy link
Contributor Author

danieldg commented Mar 9, 2024

The overhead is primarily the excessive unwrap calls, not just execution speed. I use a RawRect-like struct in my uses of tiny_skia; having it in the library directly would just be simpler. My main desire for RawRect is the ability to set the edges directly, instead of needing to extract the three borders you don't want to change and calling from_ltrb (and also unwrapping everywhere when you know your changes do not cause overflows). In addition, I find it useful to make a rect with some edges set to f32::INFINITY and intersect it with other rects to represent one- or two-sided cropping (or optional cropping, actually). I can keep it out of tiny_skia if you don't want it, though.

@RazrFalcon
Copy link
Owner

I understand the use case, but I don't think it should be in tiny-skia.

@danieldg danieldg marked this pull request as ready for review March 11, 2024 00:24
@danieldg
Copy link
Contributor Author

whoops, I meant to mark it as ready last comment

@RazrFalcon RazrFalcon merged commit 9b673e6 into RazrFalcon:master Mar 12, 2024
3 checks passed
@danieldg danieldg deleted the better-rects branch March 12, 2024 01:54
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.

2 participants