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

Change from #347 break the FanRouter functionality #383

Closed
wiresketch opened this issue Mar 14, 2024 · 4 comments · Fixed by #385
Closed

Change from #347 break the FanRouter functionality #383

wiresketch opened this issue Mar 14, 2024 · 4 comments · Fixed by #385

Comments

@wiresketch
Copy link
Contributor

wiresketch commented Mar 14, 2024

Change #347 that is seemingly only cosmetic, changes the code in FanRouter handleCollision method from:

		if (index % 2 == 0) {
			bendPoint = new Point(midPoint.x + (index / 2) * (-1 * ySeparation),
					midPoint.y + (index / 2) * xSeparation);
		} else {
			bendPoint = new Point(midPoint.x + (index / 2) * ySeparation,
					midPoint.y + (index / 2) * (-1 * xSeparation));
		}
		if (!bendPoint.equals(midPoint))
			points.insertPoint(bendPoint, 1);

to

		if (index % 2 == 0) {
			bendPoint = new Point(midPoint.x + (int) ((index / 2.0) * -1.0 * ySeparation),
					midPoint.y + (int) ((index / 2.0) * xSeparation));
		} else {
			bendPoint = new Point(midPoint.x + (int) ((index / 2.0) * ySeparation),
					midPoint.y + (int) ((index / 2.0) * -1.0 * xSeparation));
		}
		if (!bendPoint.equals(midPoint)) {
			points.insertPoint(bendPoint, 1);
		}

This breaks the connection drawing in the application that I'm developing. Here's a simple visual example.

Before:
before

After:
after

Clearly the new code change is not equivalent to the old one. The effect is that before the condition after the bendPoint calculation was false, but in the changed code it's calculated as true.

@ptziegler
Copy link
Contributor

I assume the problem is due to the fraction now being considered?

Before: ⌊(index / 2)⌋ * (-1 * ySeparation)
After: ⌊(index / 2) * (-1 * ySeparation)⌋

@wiresketch
Copy link
Contributor Author

Yes, this is what causes the problem.

@azoitl
Copy link
Contributor

azoitl commented Mar 16, 2024

As I broke it I'll provide a fix. Will do that right after the repo is ready for 3.20

ptziegler added a commit to ptziegler/gef-classic that referenced this issue Mar 16, 2024
This commit partially reverts fd75841
and uses PrecisionPoints instead of Points in the FanRouter class, to
store the bend point.

Note that because the mid point is a plain point, the equality check is
done using the integer coordinates.

Resolves eclipse-gef#383
ptziegler added a commit to ptziegler/gef-classic that referenced this issue Mar 16, 2024
This commit partially reverts fd75841
and uses PrecisionPoints instead of Points in the FanRouter class, to
store the bend point.

Note that because the mid point is a plain point, the equality check is
done using the integer coordinates.

Resolves eclipse-gef#383
@ptziegler
Copy link
Contributor

As I broke it I'll provide a fix. Will do that right after the repo is ready for 3.20

I already had a look at it yesterday and I think the easiest way is to simply revert the change and use a PrecisionPoint, to get rid of the deprecation warning. See #385

ptziegler added a commit to ptziegler/gef-classic that referenced this issue Mar 17, 2024
This commit partially reverts fd75841
and uses PrecisionPoints instead of Points in the FanRouter class, to
store the bend point.

Note that because the mid point is a plain point, the equality check is
done using the integer coordinates.

Resolves eclipse-gef#383
ptziegler added a commit to ptziegler/gef-classic that referenced this issue Mar 17, 2024
This commit partially reverts fd75841
and uses PrecisionPoints instead of Points in the FanRouter class, to
store the bend point.

Note that because the mid point is a plain point, the equality check is
done using the integer coordinates.

Resolves eclipse-gef#383
azoitl pushed a commit that referenced this issue Mar 17, 2024
This commit partially reverts fd75841
and uses PrecisionPoints instead of Points in the FanRouter class, to
store the bend point.

Note that because the mid point is a plain point, the equality check is
done using the integer coordinates.

Resolves #383
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 a pull request may close this issue.

3 participants