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

Remove deprecation from getDistanceOrthogonal #21 #673

Merged
merged 2 commits into from
Jan 19, 2025

Conversation

azoitl
Copy link
Contributor

@azoitl azoitl commented Jan 18, 2025

getDistanceOrthogonal is used in several places in GEF. There is no straight forward replacement. To reason for deprecation is precision of subclasses. As there is only one subclass with higher precision an improved version of that method is added there. Furthermore the impact on precision will anyhow only happen if both sides are PrecisionPoints.

#21

@azoitl azoitl requested a review from ptziegler January 18, 2025 16:19
@azoitl azoitl force-pushed the getDistanceOrthogonal branch from 2773fd1 to 28acce4 Compare January 18, 2025 16:20
getDistanceOrthogonal is used in several places in GEF. There is no
straight forward replacement. To reason for deprecation is precision of
subclasses. As there is only one subclass with higher precision an
improved version of that method is added there. Furthermore the impact
on precision will anyhow only happen if both sides are PrecisionPoints.

eclipse-gef#21
@azoitl azoitl force-pushed the getDistanceOrthogonal branch from 28acce4 to 3a971a5 Compare January 18, 2025 16:24
@ptziegler
Copy link
Contributor

I agree that it doesn't make sense to deprecate getDistanceOrthogonal(), but not e.g. getDistanceSquared(), which suffers from the same problem.

That said: I'm not sure if it makes sense to override the method in PrecisePoint, given that you still have the imprecision due to the cast to int. Perhaps it's better to create a getPreciseDistanceOrthogonal(Point) / getDistanceOrthorgonal(PrecisionPoint) method?

@azoitl
Copy link
Contributor Author

azoitl commented Jan 18, 2025

That said: I'm not sure if it makes sense to override the method in PrecisePoint, given that you still have the imprecision due to the cast to int. Perhaps it's better to create a getPreciseDistanceOrthogonal(Point) / getDistanceOrthorgonal(PrecisionPoint) method?

I don't think this makes much sense. The reason I overwrote it is to reduce rounding errors for the deltas and sums. returning a double would mostly only be needed of you sum-up several of the distances. But what could make sense is to round the result instead of casting it to int. What do you think?

PrecisionPoint provides now its own getDistanceSquared which uses the
precise coordinates to reduce rounding errors in the distance
calculations.
@ptziegler ptziegler added this to the 3.23.0 milestone Jan 19, 2025
@ptziegler ptziegler merged commit 8d89994 into eclipse-gef:master Jan 19, 2025
14 checks passed
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