Skip to content

Conversation

@elemoine
Copy link

@elemoine elemoine commented Feb 17, 2017

This is required if we want to take z into account when checking that patch bounds intersect.

At this point I am just creating this PR for discussion. @mbredif does that make sense to you?

@elemoine elemoine force-pushed the bounds3d branch 2 times, most recently from 2d0e783 to fb7446f Compare February 17, 2017 18:32
This is required if we want to take z into account when checking that bounds
intersect.
@mbredif
Copy link
Collaborator

mbredif commented Feb 23, 2017

Sure,
my concern is that changing the header will break existing data (how do we provide an upgrade strategy?). If we break it, we should break it for m values as well.

In #34 I addressed this similarly, but not exactly the same way. In particular, my boxes are always 4D, it is just that when m or z dimensions are missing, the min and max are not discriminative ([-DBLMAX, DBLMAX]). This complexifies a little bit the box construction (which has to know which are the valid dimensions) but simplifies the box usage (eg : point query...). What do you think ?

@elemoine
Copy link
Author

Sure,
my concern is that changing the header will break existing data (how do we provide an upgrade strategy?). If we break it, we should break it for m values as well.

Yep, that's a major issue. Do we agree that #34 suffers the same problem?

In #34 I addressed this similarly, but not exactly the same way. In particular, my boxes are always 4D, it is just that when m or z dimensions are missing, the min and max are not discriminative ([-DBLMAX, DBLMAX]). This complexifies a little bit the box construction (which has to know which are the valid dimensions) but simplifies the box usage (eg : point query...). What do you think ?

Agree that it's better to simplify the box usage.

@elemoine
Copy link
Author

I am closing this, as this is not meant to be merged.

@elemoine elemoine closed this Feb 24, 2017
@mbredif
Copy link
Collaborator

mbredif commented Feb 24, 2017

Do we agree that #34 suffers the same problem?

yes of course... I guess that means it's fine for within LI3DS but that pushing it upstream without a proper upgrade strategy will be impossible.

@elemoine
Copy link
Author

yes of course... I guess that means it's fine for within LI3DS but that pushing it upstream without a proper upgrade strategy will be impossible.

In that case, I don't think it's a good idea for LI3DS either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants