servo / euclid

Geometry primitives (basic linear algebra) for Rust
Other
462 stars 102 forks source link

Add Intersection_or_zero #462

Closed nical closed 4 years ago

nical commented 4 years ago

Implemented for Rect, Box2D and Box3D, it returns the intersection of the two rects or boxes, or Rect/Box::zero() if they don't intersect.

This is useful in WebRender where we do a lot of r1.intersection(r2).unwrap_or_else(Rect::zero) which is inconvenient to write now that intersection returns Option<NonEmpty<Self>>.

nical commented 4 years ago

While it sounds similar, it is different from Box2D::intersection_unchecked that can be chained and returns a negative box if the two don't intersect.

nical commented 4 years ago

FWIW both names intersection_or_empty and intersection_or_zero work for me, though we have Rect::zero rather than Rect::empty I don't mind adding Rect::empty in the process since it's always the first name I reach for for whatever reason.

nical commented 4 years ago

Note that b.is_empty() doesn't imply b == Box::zero(), because is_empty() will also return true for negative areas and if there is a NaN, but I agree that it would be nice to be able to use Rect::empty as the name is a bit more "semantic" than zero.

I added a commit that introduces Rect::empty() and Box2D/3D::empty() in addition to ::zero(), and change the new method to intersection_or_empty. Let me know if you prefer with or without this extra change.

nical commented 4 years ago

The more I work through the webrender update the more I realize that returning Option<NonEmpty<Self>> is creating unanticipated friction. I'm proposing to panic-roll-back to returning Option<Self> as we did in 0.20 before we've committed too much to 0.21 (see #463).

kvark commented 4 years ago

@nical that's sad to hear! I guess we'll need to try updating WR before releasing euclid, or even merging important changes, in the future.

nical commented 4 years ago

Yeah I suspect it will be a pain to do because of the vendoring and having to point multiple crates (some vendored, some in tree) to unpublished euclid somehow, but indeed that would avoid spinning multiple versions quickly. All the more reason to make breaking changes as rarely as we can.