(breaking change) Some Rect and Box adjustments

servo / euclid

Geometry primitives (basic linear algebra) for Rust

Other

462 stars 102 forks source link

(breaking change) Some Rect and Box adjustments #459

Closed nical closed 4 years ago

nical commented 4 years ago

A few rect/box related adjustments:

Change Rect::intersection -> Option<Self> into Rect::try_intersection -> Option<NonEmpty<Self> to match Box2D's API and get the extra typeness of NonEmpty. That's main ly for consistency but I would understand if we feel like it's too annoying a change becase webrender (and perhaps servo layout?) code is riddled with intersection checks. Let me know.
Make Box2D::try_intersection check that the box is empty instead of just negative. That was an oversight, we'd previously get Some(intersection) with intersection being an empty rect (or a bad rect with NaNs) which I believe isn't what we want.
Express Box3D::try_intersection the same way as Box2D::try_intersection: That shouldn't affect the behavior but it removes a bit of redundant computation.

nical commented 4 years ago

Let me know if the Rect::try_intersection -> Option<NonEmpty<Self>> change is too annoying to deal with downstream. I can live with the inconsistency.

For WebRender I want to eventually replace most uses of Rect with Box2D anyway because it has less precision loss and is faster for the majority of what we do in WebRender (anything except translating boxes).

nical commented 4 years ago

As discussed, changed the PR so that we have

fn intersection(&self, other: &Self) -> Option<NonEmpty<Self>>
fn intersection_unchecked(&self, other: &Self) -> Self

I don't feel like rebasing the other two commits over the last change, but if having the back-and-forth in the commit log bothers you I can squash it into one.

With this Rect::intersection keeps the same name so it won't cause as much breakage downstream.

nical commented 4 years ago

@bors-servo r+

bors-servo commented 4 years ago

:pushpin: Commit a7b648a has been approved by nical

bors-servo commented 4 years ago

:hourglass: Testing commit a7b648a3e167cba3f445865960f91174d57e9466 with merge de9e4cdc175c780d8ddf661b8b185fc6d0663d3c...

nical commented 4 years ago

Oh noes I meant r=kvark. It's one of these days.

bors-servo commented 4 years ago

:sunny: Test successful - checks-travis Approved by: nical Pushing de9e4cdc175c780d8ddf661b8b185fc6d0663d3c to master...