Skip to content

Commit 5cfe5b7

Browse files
ptzieglerazoitl
authored andcommitted
Avoid accumulative rounding errors when using nested scalable figures
When translating geometric shapes across multiple scalable layers, rounding errors may occur if each layer is using a fractional scaling factor. This is because the methods translateToRelative() and translateToAbsolute() work with integer precision. Given an integer-based shape, this causes rounding errors after every layer. To avoid this, an internal useDoublePrecision() method is added to the Figure, which indicates whether those methods should convert the integer-precision shape to (intermediate) double-precision shape. The fractional values are accumulated and only converted back to integer precision at the end. Relates to #829
1 parent 95c2feb commit 5cfe5b7

File tree

6 files changed

+187
-5
lines changed

6 files changed

+187
-5
lines changed

org.eclipse.draw2d.tests/src/org/eclipse/draw2d/test/Draw2dTestSuite.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@
6060
InsetsTest.class,
6161
DirectedGraphLayoutTest.class,
6262
ScrollPaneTests.class,
63-
LabelTest.class
63+
LabelTest.class,
64+
PrecisionTests.class
6465
})
6566
public class Draw2dTestSuite {
6667
}
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2025 Patrick Ziegler and others.
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License 2.0 which is available at
6+
* http://www.eclipse.org/legal/epl-2.0.
7+
*
8+
* SPDX-License-Identifier: EPL-2.0
9+
*
10+
* Contributors:
11+
* Patrick Ziegler - initial API and implementation
12+
*******************************************************************************/
13+
14+
package org.eclipse.draw2d.test;
15+
16+
import org.eclipse.draw2d.Figure;
17+
import org.eclipse.draw2d.FigureUtilities;
18+
import org.eclipse.draw2d.IFigure;
19+
import org.eclipse.draw2d.ScalableLayeredPane;
20+
import org.eclipse.draw2d.geometry.PrecisionRectangle;
21+
import org.eclipse.draw2d.geometry.Rectangle;
22+
23+
import org.junit.jupiter.api.BeforeEach;
24+
import org.junit.jupiter.api.Test;
25+
26+
/**
27+
* Test case to ensure rounding errors don't accumulate when translating
28+
* coordinates through multiple scalable layers.
29+
*/
30+
public class PrecisionTests extends BaseTestCase {
31+
private IFigure fig;
32+
33+
@BeforeEach
34+
public void setUp() {
35+
fig = new Figure();
36+
IFigure layers = createLayers();
37+
layers.add(fig);
38+
IFigure root = new Figure();
39+
root.add(FigureUtilities.getRoot(layers));
40+
}
41+
42+
private static IFigure createLayers() {
43+
ScalableLayeredPane f1 = createLayer(1.33);
44+
ScalableLayeredPane f2 = createLayer(5.79);
45+
ScalableLayeredPane f3 = createLayer(0.87);
46+
ScalableLayeredPane f4 = createLayer(3.88);
47+
ScalableLayeredPane f5 = createLayer(1.46);
48+
49+
f1.add(f2);
50+
f2.add(f3);
51+
f3.add(f4);
52+
f4.add(f5);
53+
54+
return f5;
55+
}
56+
57+
private static ScalableLayeredPane createLayer(double scale) {
58+
ScalableLayeredPane figure = new ScalableLayeredPane();
59+
figure.setScale(scale);
60+
figure.setOpaque(true);
61+
return figure;
62+
}
63+
64+
@Test
65+
public void testPreciseTranslateToAbsolute() {
66+
Rectangle r1 = new Rectangle(13, 37, 163, 377);
67+
Rectangle r2 = new PrecisionRectangle(13, 37, 163, 377);
68+
fig.translateToAbsolute(r1);
69+
fig.translateToAbsolute(r2);
70+
assertEquals(493, 1404, 6187, 14309, r1);
71+
assertEquals(r1.x, r2.x);
72+
assertEquals(r1.y, r2.y);
73+
assertEquals(r1.width, r2.width);
74+
assertEquals(r1.height, r2.height);
75+
}
76+
77+
@Test
78+
public void testPreciseTranslateToRelative() {
79+
Rectangle r1 = new Rectangle(753, 891, 353, 564);
80+
Rectangle r2 = new PrecisionRectangle(753, 891, 353, 564);
81+
fig.translateToRelative(r1);
82+
fig.translateToRelative(r2);
83+
assertEquals(19, 23, 11, 16, r1);
84+
assertEquals(r1.x, r2.x);
85+
assertEquals(r1.y, r2.y);
86+
assertEquals(r1.width, r2.width);
87+
assertEquals(r1.height, r2.height);
88+
}
89+
}

org.eclipse.draw2d/src/org/eclipse/draw2d/Figure.java

Lines changed: 74 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@
3333
import org.eclipse.draw2d.geometry.Dimension;
3434
import org.eclipse.draw2d.geometry.Insets;
3535
import org.eclipse.draw2d.geometry.Point;
36+
import org.eclipse.draw2d.geometry.PrecisionDimension;
37+
import org.eclipse.draw2d.geometry.PrecisionPoint;
38+
import org.eclipse.draw2d.geometry.PrecisionRectangle;
3639
import org.eclipse.draw2d.geometry.Rectangle;
3740
import org.eclipse.draw2d.geometry.Translatable;
3841

@@ -2046,8 +2049,10 @@ public void translateFromParent(Translatable t) {
20462049
@Override
20472050
public final void translateToAbsolute(Translatable t) {
20482051
if (getParent() != null) {
2049-
getParent().translateToParent(t);
2050-
getParent().translateToAbsolute(t);
2052+
Translatable tPrecise = toPreciseShape(t);
2053+
getParent().translateToParent(tPrecise);
2054+
getParent().translateToAbsolute(tPrecise);
2055+
fromPreciseShape(tPrecise, t);
20512056
}
20522057
}
20532058

@@ -2067,8 +2072,55 @@ public void translateToParent(Translatable t) {
20672072
@Override
20682073
public final void translateToRelative(Translatable t) {
20692074
if (getParent() != null) {
2070-
getParent().translateToRelative(t);
2071-
getParent().translateFromParent(t);
2075+
Translatable tPrecise = toPreciseShape(t);
2076+
getParent().translateToRelative(tPrecise);
2077+
getParent().translateFromParent(tPrecise);
2078+
fromPreciseShape(tPrecise, t);
2079+
}
2080+
}
2081+
2082+
/**
2083+
* Converts the given shape using integer-precision into a compatible shape
2084+
* using double-precision. Does nothing if the conversion to double-precision is
2085+
* disabled via {@link #useDoublePrecision()} or if the given
2086+
* {@link Translatable} is already using double-precision.
2087+
*
2088+
* @param source integer-precision shape
2089+
* @return double-precision geometry
2090+
*/
2091+
private Translatable toPreciseShape(Translatable source) {
2092+
if (getParent() instanceof Figure parentFigure && parentFigure.useDoublePrecision()) {
2093+
if (source instanceof Point p && !(source instanceof PrecisionPoint)) {
2094+
return new PrecisionPoint(p);
2095+
} else if (source instanceof Dimension d && !(source instanceof PrecisionDimension)) {
2096+
return new PrecisionDimension(d);
2097+
} else if (source instanceof Rectangle r && !(source instanceof PrecisionRectangle)) {
2098+
return new PrecisionRectangle(r);
2099+
}
2100+
}
2101+
// is already precise or doesn't have a precise variant
2102+
return source;
2103+
}
2104+
2105+
/**
2106+
* Converts the double-precision values of {@code source} into integer-precision
2107+
* values and stores them in {@code target}. Does nothing if the conversion to
2108+
* double-precision is disabled via {@link #useDoublePrecision()}, if
2109+
* {@code source == target} or if {@code source} is not a double-precision
2110+
* shape.
2111+
*
2112+
* @param source double-precision shape
2113+
* @param target integer-precision shape
2114+
*/
2115+
private void fromPreciseShape(Translatable source, Translatable target) {
2116+
if (source != target && getParent() instanceof Figure parentFigure && parentFigure.useDoublePrecision()) {
2117+
if (source instanceof PrecisionPoint p1 && target instanceof Point p2) {
2118+
p2.setLocation(p1.x, p1.y);
2119+
} else if (source instanceof PrecisionDimension d1 && target instanceof Dimension d2) {
2120+
d2.setSize(d1.width, d1.height);
2121+
} else if (source instanceof PrecisionRectangle r1 && target instanceof Rectangle r2) {
2122+
r2.setBounds(r1.x, r1.y, r1.width, r1.height);
2123+
}
20722124
}
20732125
}
20742126

@@ -2084,6 +2136,24 @@ protected boolean useLocalCoordinates() {
20842136
return false;
20852137
}
20862138

2139+
/**
2140+
* Returns {@code true} if this Figure should use double-precision arithmetic
2141+
* when translating a {@link Translatable}. This method should not be called or
2142+
* overridden by clients. This method is overridden by classes dealing with
2143+
* (fractional) scaling (implementing the {@link ScalableFigure}).
2144+
*
2145+
* @return {@code false}.
2146+
* @since 3.21
2147+
* @noreference This method is not intended to be referenced by clients.
2148+
* @nooverride This method is not intended to be re-implemented or extended by
2149+
* clients.
2150+
*/
2151+
@SuppressWarnings("static-method")
2152+
protected boolean useDoublePrecision() {
2153+
// This method should only be used internally and is not part of the interface
2154+
return false;
2155+
}
2156+
20872157
/**
20882158
* @see IFigure#validate()
20892159
*/

org.eclipse.draw2d/src/org/eclipse/draw2d/ScalableFreeformLayeredPane.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,4 +114,12 @@ public void translateFromParent(Translatable t) {
114114
IScalablePaneHelper.translateFromParent(this, t);
115115
}
116116

117+
/**
118+
* @since 3.21
119+
* @see org.eclipse.draw2d.Figure#useDoublePrecision()
120+
*/
121+
@Override
122+
protected boolean useDoublePrecision() {
123+
return true;
124+
}
117125
}

org.eclipse.draw2d/src/org/eclipse/draw2d/ScalableLayeredPane.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,4 +127,13 @@ public boolean isCoordinateSystem() {
127127
return true;
128128
}
129129

130+
/**
131+
* @since 3.21
132+
* @see org.eclipse.draw2d.Figure#useDoublePrecision()
133+
*/
134+
@Override
135+
protected boolean useDoublePrecision() {
136+
return true;
137+
}
138+
130139
}

org.eclipse.draw2d/src/org/eclipse/draw2d/ScalableLightweightSystem.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,5 +116,10 @@ public void translateToParent(Translatable t) {
116116
public void translateFromParent(Translatable t) {
117117
IScalablePaneHelper.translateFromParent(this, t);
118118
}
119+
120+
@Override
121+
protected boolean useDoublePrecision() {
122+
return true;
123+
}
119124
}
120125
}

0 commit comments

Comments
 (0)