Railroad diagram generator produces non-connected lines in generated diagrams
From #63 (comment 9797):
I noticed several diagrams from the documentation have non-connected lines (see example below)
From #63 (comment 9819):
The gaps in the lines are likely the Arc routine in the 2D graphics. Specified sizes in it don't match with output at pixel level. You ask for a 2x2 rectangle and you get a 3x3 rectangle, at least in my version of the library. Code there has magic offsets to correct for this, but maybe the offsets aren't quite right, or they 'fixed' the library.
From #63 (comment 9821):
[...] maybe the offsets aren't quite right, or they 'fixed' the library.
Note that I'm using Eclipse 2021 and Java 11, as that was recently merged to
develop
. So maybe indeed there are differences in Java 7 vs Java 11 that play a role here.
From !46 (comment 10796):
I tried fixing the arc painting yesterday, and while that worked, I have now more gaps than before, it seems (or maybe they already existed, I didn't try building the diagrams from my unmodified version).
The gaps shouldn't exist, the solver is supposed to adjust sizes and positions to compensate. So this needs a bigger plan to resolve I think. There are three different possible causes for gaps:
- The solver may be broken and not compute the right coordinates and sizes of the elements.
- The constraints given to the solver may be wrong or incomplete.
- The painter may paint elements at a different position than stated.
Current debug output is mostly aimed at small cases to check the solver internals, it's not helpful in finding trouble in anything larger than the simplest example. In particular, there should be some way to understand the graph being solved and what constraints exist in that graph.
For painter checking, it may be sufficient to paint each element of a result at a fresh image, and check painted positions afterwards. Stuff is a bit tricky here though, as the
awt
package doesn't seem to always do what I say. A vertical or horizontal line is always centered on a row of pixels. This works fine for odd width lines. For even width lines, it doesn't use 2 rows of pixels. Instead it centers at one row, and the neighbouring rows then get 0.5 line as well (with antialiasing).Not sure if the painter problems are the real trouble. Paint may not always end up at the right spot, but it shouldn't introduce gaps, so something more seems to be happening.
From !46 (comment 21972):
The arc positioning was off when comparing computed coordinates with actual plotted dots, so I fixed that by replacing the fuzzy quarter circle draw by a clipped full circle, losing much of the magic offset corrections there. It mostly works, AWT just fails to understand centering on a pixel row doesn't work for lines with an even width.
However the result was worse than before, in the tutorial diagrams there are now more gaps. So this lead me to the following points
- Add the capability to automatically check that stuff is painted where it is supposed to be, at least for the end-points. Simplest approach here would be a clear image buffer, paint one element, check that the endpoints are where they should be. Repeat for all elements. Done.
- Extend the solver debug output for a more clear larger scope view. Currently it's all about details of each painted blob. This works fine for simple figures, but at more realistic figure scale you get lost in the huge number of details. Finding the point of interest takes too much time. Not sure how to achieve that currently. EDIT: Perhaps by exploiting the proxy hierarchy? There are proxies, essentially a box with content and connection points nested in the figure.
- I am not sure AWT is the way to go, it doesn't always paint where I want it.
If you want to dive into the coordinate stuff, all blobs have inclusive borders, a blob at
(1,1) -- (3,2)
covers pixels(1,1), (2,1), (3,1) (1,2), (2,2), (2,3)
. Arcs are square, and their origins are at the corner opposite of the center-point of the arc (bit weird, but useful for aligning lines connecting with the arc).For reference this is the patch I currently have
commit 6c6b5302a56e4f85329af74b0124b30c3ac8b45d Author: Albert Hofkamp <a.t.hofkamp@tue.nl> Date: Fri May 21 08:58:13 2021 +0200 63 Change arcs, add random debug stuff. diff --git a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/RailRoadDiagramApplication.java b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/RailRoadDiagramApplication.java index 8cfac53d..b301aff7 100644 --- a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/RailRoadDiagramApplication.java +++ b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/RailRoadDiagramApplication.java @@ -142,6 +142,7 @@ public class RailRoadDiagramApplication extends Application<IOutputComponent> { // Paint graphics to the image. double top = 0; + System.out.printf("\n====\n"); for (RailRule rule: rules) { rule.paint(0, top, gd); Size2D size = rule.getSize(); diff --git a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/BottomLeftArc.java b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/BottomLeftArc.java index f447a448..79cedf7d 100644 --- a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/BottomLeftArc.java +++ b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/BottomLeftArc.java @@ -55,6 +55,7 @@ public class BottomLeftArc extends Arc { double right = solver.getVarValue(this.right) + baseLeft - 1; double top = solver.getVarValue(this.top) + baseTop; double bottom = solver.getVarValue(this.bottom) + baseTop - 1; + System.out.printf("BLarc: left=%03.1f, right=%03.1f, top=%03.1f, bottom=%03.1f\n", left, right, top, bottom); double size = bottom - top + 1; Assert.check(Math.abs(size - (right - left + 1)) < Solver.EPSILON); // Must be a square area. diff --git a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/BottomRightArc.java b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/BottomRightArc.java index 01557ab4..156784d4 100644 --- a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/BottomRightArc.java +++ b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/BottomRightArc.java @@ -55,6 +55,7 @@ public class BottomRightArc extends Arc { double right = solver.getVarValue(this.right) + baseLeft - 1; double top = solver.getVarValue(this.top) + baseTop; double bottom = solver.getVarValue(this.bottom) + baseTop - 1; + System.out.printf("BRarc: left=%03.1f, right=%03.1f, top=%03.1f, bottom=%03.1f\n", left, right, top, bottom); double size = bottom - top + 1; Assert.check(Math.abs(size - (right - left + 1)) < Solver.EPSILON); // Must be a square area. diff --git a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/HorLine.java b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/HorLine.java index 2259a7d2..c2c81f53 100644 --- a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/HorLine.java +++ b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/HorLine.java @@ -13,11 +13,13 @@ package org.eclipse.escet.common.raildiagrams.graphics; +import static org.eclipse.escet.common.java.Strings.fmt; import static org.eclipse.escet.common.raildiagrams.graphics.PaintSupport.setLineWidth; import java.awt.Color; import java.awt.Graphics2D; +import org.eclipse.escet.common.java.Assert; import org.eclipse.escet.common.raildiagrams.solver.Solver; /** Horizontal line. */ @@ -52,6 +54,8 @@ public class HorLine extends Area { return; } + //Assert.check((int)right != 108 || (int)top != 53, fmt("right=%s.", this.right)); + //System.out.printf("Hline: left=%03.1f, right=%03.1f, top=%03.1f, bottom=%03.1f\n", left, right, top, bottom); gd.setColor(railColor); setLineWidth(gd, (int)width); gd.drawLine((int)left, (int)center, (int)right, (int)center); diff --git a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/PaintSupport.java b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/PaintSupport.java index 7781d8eb..a8e2f1c1 100644 --- a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/PaintSupport.java +++ b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/PaintSupport.java @@ -16,6 +16,7 @@ package org.eclipse.escet.common.raildiagrams.graphics; import java.awt.BasicStroke; import java.awt.Color; import java.awt.Graphics2D; +import java.awt.geom.Ellipse2D; /** Functions to simplify writing graphics paint code. */ public class PaintSupport { @@ -51,13 +52,42 @@ public class PaintSupport { public static void drawArc(Graphics2D gd, double xpos, double ypos, ArcType arcType, double size, double lineWidth, Color color) { - gd.setColor(color); + size = size + 1; + double length = 2 * size - 3; + +// System.out.printf("%s: x=%.1f, y=%.1f, size=%.1f, lineWidth=%.1f\n", arcType.toString(), +// xpos, +// ypos, +// size, +// lineWidth); + //gd.setColor(color); + gd.setColor(new Color(20, 20, 200)); setLineWidth(gd, (int)lineWidth); - int arcX = (int)(xpos + arcType.xSizeMul * size + arcType.xLwidthMul * lineWidth); - int arcY = (int)(ypos + arcType.ySizeMul * size + arcType.yLwidthMul * lineWidth); - gd.drawArc(arcX, arcY, (int)(2 * size - 2 * lineWidth), (int)(2 * size - 2 * lineWidth), arcType.startAngle, - 90); + switch (arcType) { + case BL_ARC: + ypos = ypos - size + 2; + gd.setClip((int)xpos - 1, (int)(ypos + size - 2), (int)size, (int)size); + break; + case BR_ARC: + xpos = xpos - size + 2; + ypos = ypos - size + 2; + gd.setClip((int)(xpos + size - 2), (int)(ypos + size - 2), (int)size, (int)size); + break; + case TL_ARC: + gd.setClip((int)xpos - 1, (int)ypos - 1, (int)size, (int)size); + break; + case TR_ARC: + xpos = xpos - size + 2; + gd.setClip((int)(xpos + size - 2), (int)ypos - 1, (int)size, (int)size); + break; + default: + throw new AssertionError("Unexpected arc-type found."); + } + double widthCorrection = lineWidth / 2.0 - 0.5; + double circleLength = length - lineWidth + 0.5; + gd.draw(new Ellipse2D.Double(xpos + widthCorrection, ypos + widthCorrection, circleLength, circleLength)); + gd.setClip(null); } /** Arcs that can be painted. */ diff --git a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/TopLeftArc.java b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/TopLeftArc.java index d7e7dcee..2a5480ac 100644 --- a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/TopLeftArc.java +++ b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/TopLeftArc.java @@ -55,6 +55,7 @@ public class TopLeftArc extends Arc { double right = solver.getVarValue(this.right) + baseLeft - 1; double top = solver.getVarValue(this.top) + baseTop; double bottom = solver.getVarValue(this.bottom) + baseTop - 1; + System.out.printf("TLarc: left=%03.1f, right=%03.1f, top=%03.1f, bottom=%03.1f\n", left, right, top, bottom); double size = bottom - top + 1; Assert.check(Math.abs(size - (right - left + 1)) < Solver.EPSILON); // Must be a square area. diff --git a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/TopRightArc.java b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/TopRightArc.java index 79b41f4b..7cf5a8c5 100644 --- a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/TopRightArc.java +++ b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/TopRightArc.java @@ -55,6 +55,7 @@ public class TopRightArc extends Arc { double right = solver.getVarValue(this.right) + baseLeft - 1; double top = solver.getVarValue(this.top) + baseTop; double bottom = solver.getVarValue(this.bottom) + baseTop - 1; + System.out.printf("TRarc: left=%03.1f, right=%03.1f, top=%03.1f, bottom=%03.1f\n", left, right, top, bottom); double size = bottom - top + 1; Assert.check(Math.abs(size - (right - left + 1)) < Solver.EPSILON); // Must be a square area. diff --git a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/VertLine.java b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/VertLine.java index 52983047..70a2da63 100644 --- a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/VertLine.java +++ b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/graphics/VertLine.java @@ -53,6 +53,7 @@ public class VertLine extends Area { return; } + System.out.printf("Vline: left=%03.1f, right=%03.1f, top=%03.1f, bottom=%03.1f\n", left, right, top, bottom); gd.setColor(railColor); setLineWidth(gd, (int)width); gd.drawLine((int)center, (int)top, (int)center, (int)bottom); diff --git a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/railroad/DiagramElement.java b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/railroad/DiagramElement.java index 0081adbe..37be3354 100644 --- a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/railroad/DiagramElement.java +++ b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/railroad/DiagramElement.java @@ -115,6 +115,7 @@ public abstract class DiagramElement { for (ProxyDiagramElement childElement: childDiagramElements) { double childLeft = left + solver.getVarValue(childElement.left); double childTop = top + solver.getVarValue(childElement.top); + System.out.printf("left=%s, top=%s\n", childElement.left.name, childElement.top.name); childElement.paint(childLeft, childTop, gd); } } diff --git a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/railroad/NamedNode.java b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/railroad/NamedNode.java index c86a11df..e9ff7216 100644 --- a/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/railroad/NamedNode.java +++ b/common/org.eclipse.escet.common.raildiagrams/src/org/eclipse/escet/common/raildiagrams/railroad/NamedNode.java @@ -153,7 +153,7 @@ public class NamedNode extends DiagramElement { solver.addEq(leftLine.left, 0, entryLine.right); addGraphic(entryLine); - HorLine exitLine = new HorLine(solver, "name-entry", railColor, railwidth); + HorLine exitLine = new HorLine(solver, "name-exit", railColor, railwidth); solver.addEq(exitLine.left, exitLength, exitLine.right); solver.addEq(rightLine.right, 0, exitLine.left); addGraphics(exitLine);