euclid icon indicating copy to clipboard operation
euclid copied to clipboard

Box inclusive contains

Open ua-kxie opened this issue 2 years ago • 7 comments

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

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

ua-kxie avatar Aug 10 '23 03:08 ua-kxie

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?

nical avatar Aug 14 '23 08:08 nical

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

ua-kxie avatar Aug 16 '23 18:08 ua-kxie

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.

kyp44 avatar Mar 06 '24 18:03 kyp44

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

nical avatar Mar 14 '24 10:03 nical

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?

ua-kxie avatar Mar 15 '24 01:03 ua-kxie

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.

kyp44 avatar Mar 15 '24 16:03 kyp44

Adding contains_inclusive sounds good to me.

nical avatar Mar 16 '24 07:03 nical