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

Some Rect changes #357

Merged
merged 3 commits into from Jul 4, 2019
Merged

Some Rect changes #357

merged 3 commits into from Jul 4, 2019

Conversation

@nical
Copy link
Collaborator

nical commented Jul 3, 2019

  • Introduce NonEmpty<T> as discussed in #339.
  • Remove the default unit parameter in Rect as the compiler rarely seem to make use of that information.
  • Treat negative rects as empty rects in contains_rect (just like we do in Box2D and Box3D).

This change is Reviewable

@nical
Copy link
Collaborator Author

nical commented Jul 3, 2019

})
}

/// Returns true if this box contains the interior of the other box. Always

This comment has been minimized.

@kvark

kvark Jul 3, 2019

Member

what's the "Always" for?

This comment has been minimized.

@nical

nical Jul 3, 2019

Author Collaborator

A copy-paste accident from the equivalent comment in the non-non-empty type. Fixed.

pub fn union(&self, other: &NonEmpty<Box3D<T, U>>) -> NonEmpty<Box3D<T, U>> {
NonEmpty(Box3D {
min: point3(
max(self.min.x, other.min.x),

This comment has been minimized.

@kvark

kvark Jul 3, 2019

Member

this is computing the intersection, not union... do we have any test coverage for this?

This comment has been minimized.

@nical

nical Jul 3, 2019

Author Collaborator

Oops! Now there is.

use core::cmp::{PartialEq};

#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]

This comment has been minimized.

@kvark

kvark Jul 3, 2019

Member

could be useful to serialize it transparently, i.e. #[serde(transparent)]

This comment has been minimized.

@nical

nical Jul 3, 2019

Author Collaborator

Good idea, done.

@nical nical force-pushed the nical:breaking-7 branch 2 times, most recently from c4f55d9 to 0e4deb4 Jul 3, 2019
@kvark
kvark approved these changes Jul 4, 2019
@nical nical force-pushed the nical:breaking-7 branch from 0e4deb4 to be7d686 Jul 4, 2019
@nical nical force-pushed the nical:breaking-7 branch from be7d686 to 9d498b2 Jul 4, 2019
@nical
Copy link
Collaborator Author

nical commented Jul 4, 2019

@bors-servo r=kvark

@bors-servo
Copy link
Contributor

bors-servo commented Jul 4, 2019

📌 Commit 9d498b2 has been approved by kvark

@bors-servo
Copy link
Contributor

bors-servo commented Jul 4, 2019

Testing commit 9d498b2 with merge 690693a...

bors-servo added a commit that referenced this pull request Jul 4, 2019
Some Rect changes

 - Introduce `NonEmpty<T>` as discussed in #339.
 - Remove the default unit parameter in Rect as the compiler rarely seem to make use of that information.
 - Treat negative rects as empty rects in contains_rect (just like we do in Box2D and Box3D).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/euclid/357)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 4, 2019

☀️ Test successful - checks-travis
Approved by: kvark
Pushing 690693a to master...

@bors-servo bors-servo merged commit 9d498b2 into servo:master Jul 4, 2019
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.