RazrFalcon / tiny-skia

A tiny Skia subset ported to Rust
BSD 3-Clause "New" or "Revised" License
1.05k stars 67 forks source link

Add and improve Rect methods #117

Closed danieldg closed 3 months ago

danieldg commented 3 months ago

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

danieldg commented 3 months ago

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.

RazrFalcon commented 3 months ago

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.

danieldg commented 3 months ago

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 commented 3 months ago

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

danieldg commented 3 months ago

whoops, I meant to mark it as ready last comment