servo / euclid

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

Box inclusive contains #503

Closed ua-kxie closed 3 months ago

ua-kxie commented 11 months ago

How do the maintainers feel about inclusive contains functions for the Box structs?

https://docs.rs/euclid/latest/euclid/struct.Box2D.html#method.contains

nical commented 11 months ago

Why not. We haven't needed something like this so far but it's not a big burden to add and maintain. Is it something you need in your projects?

ua-kxie commented 11 months ago

yes, I'm using an inclusive contains. Basically to detect if the cursor is on top of a region, but the cursor snaps to a grid.

image

I've since impl'd this in a trait extension and it works really well, but perhaps others may find this useful as well

kyp44 commented 4 months ago

I have written something similar, except that instead of just creating an inclusive version of contains I have associated functions:

pub trait BoxInclusive {
  type Point;

  fn new_inclusive(min: Self::Point, max: Self::Point) -> Self;

  fn from_points_inclusive<I>(points: I) -> Self
  where
      I: IntoIterator,
      I::Item: Borrow<Self::Point>;
}

These simply create a normal box, then extend the max point by one in every dimension so that the max point itself is contained.

EDIT: I just submitted a PR for something unrelated, so I already have the project forked. I'd be happy to issue another PR for these inclusive box methods if there is interest from the maintainers in adding them to the box types directly.

nical commented 4 months ago

I'm happy to take a PR for this if you want to make one.

ua-kxie commented 4 months ago

These simply create a normal box, then extend the max point by one in every dimension so that the max point itself is contained.

This sounds very much like what I did, but wouldn't it be better to just have contains_inclusive functions with the > replaced with >= from performance and aesthetics considerations?

kyp44 commented 4 months ago

I have a need to create and use inclusive boxes without even necessarily ever using the contains method. Whatever approach the maintainers prefer is what we should go with.

nical commented 4 months ago

Adding contains_inclusive sounds good to me.