Skip to content

Commit bbb5abc

Browse files
laeubiamartya4256
laeubi
authored and
amartya4256
committed
When computing the size of a widget always return values rounded up
When converting pixels to point there can be depending on the zoom level be a fractional result. Currently in all cases these result is converted into an integer using `Math.round()` that will make values `+/-0.5` resulting in small values to be round towards a smaller value. While it is maybe valid for a _location_, when using points to express a _dimension_ this is not okay as it will result in the reported (integer) value to be to small leading to errors when the SWT API is then used after performing additional computations maybe. This now makes the following adjustments: 1. Introduce a rounding mode that allows different ways of rounding and adds as a first step ROUND (the previous default) and UP (for always round towards the largest integer) 2. Adjust the `Control` class to decide what mode is best in what situation. See - #2381 - #2166
1 parent 5d2665d commit bbb5abc

File tree

24 files changed

+97
-49
lines changed

24 files changed

+97
-49
lines changed

‎bundles/org.eclipse.swt/Eclipse SWT Drag and Drop/win32/org/eclipse/swt/dnd/TableDropTargetEffect.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ public void dragOver(DropTargetEvent event) {
151151
int effect = checkEffect(event.feedback);
152152
long handle = table.handle;
153153
Point coordinates = new Point(event.x, event.y);
154-
coordinates = Win32DPIUtils.pointToPixel(table.toControl(coordinates), DPIUtil.getZoomForAutoscaleProperty(table.nativeZoom)); // To Pixels
154+
coordinates = Win32DPIUtils.pointToPixelAsLocation(table.toControl(coordinates), DPIUtil.getZoomForAutoscaleProperty(table.nativeZoom)); // To Pixels
155155
LVHITTESTINFO pinfo = new LVHITTESTINFO();
156156
pinfo.x = coordinates.x;
157157
pinfo.y = coordinates.y;

‎bundles/org.eclipse.swt/Eclipse SWT Drag and Drop/win32/org/eclipse/swt/dnd/TreeDropTargetEffect.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ public void dragOver(DropTargetEvent event) {
165165
int effect = checkEffect(event.feedback);
166166
long handle = tree.handle;
167167
Point coordinates = new Point(event.x, event.y);
168-
coordinates = Win32DPIUtils.pointToPixel(tree.toControl(coordinates), DPIUtil.getZoomForAutoscaleProperty(tree.nativeZoom)); // To Pixels
168+
coordinates = Win32DPIUtils.pointToPixelAsLocation(tree.toControl(coordinates), DPIUtil.getZoomForAutoscaleProperty(tree.nativeZoom)); // To Pixels
169169
TVHITTESTINFO lpht = new TVHITTESTINFO ();
170170
lpht.x = coordinates.x;
171171
lpht.y = coordinates.y;

‎bundles/org.eclipse.swt/Eclipse SWT OLE Win32/win32/org/eclipse/swt/ole/win32/OleClientSite.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -987,7 +987,7 @@ private int OnInPlaceDeactivate() {
987987
return COM.S_OK;
988988
}
989989
private int OnPosRectChange(long lprcPosRect) {
990-
Point size = Win32DPIUtils.pointToPixel(getSize(), DPIUtil.getZoomForAutoscaleProperty(nativeZoom)); // To Pixels
990+
Point size = Win32DPIUtils.pointToPixelAsSize(getSize(), DPIUtil.getZoomForAutoscaleProperty(nativeZoom)); // To Pixels
991991
setExtent(size.x, size.y);
992992
return COM.S_OK;
993993
}

‎bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/Point.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import java.io.*;
1818

19+
import org.eclipse.swt.internal.*;
1920
import org.eclipse.swt.widgets.*;
2021

2122
/**
@@ -138,14 +139,20 @@ public static sealed class OfFloat extends Point permits Point.WithMonitor {
138139

139140
private static final long serialVersionUID = -1862062276431597053L;
140141

141-
public float residualX, residualY;
142+
private float residualX, residualY;
143+
private RoundingMode roundingMode;
142144

143145
public OfFloat(int x, int y) {
144146
super(x, y);
145147
}
146148

147149
public OfFloat(float x, float y) {
148-
super(Math.round(x), Math.round(y));
150+
this(x, y, RoundingMode.ROUND);
151+
}
152+
153+
public OfFloat(float x, float y, RoundingMode roundingMode) {
154+
super(roundingMode.round(x), roundingMode.round(y));
155+
this.roundingMode = roundingMode;
149156
this.residualX = x - this.x;
150157
this.residualY = y - this.y;
151158
}
@@ -159,12 +166,12 @@ public float getY() {
159166
}
160167

161168
public void setX(float x) {
162-
this.x = Math.round(x);
169+
this.x = roundingMode.round(x);
163170
this.residualX = x - this.x;
164171
}
165172

166173
public void setY(float y) {
167-
this.y = Math.round(y);
174+
this.y = roundingMode.round(y);
168175
this.residualY = y - this.y;
169176
}
170177

‎bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/RoundingMode.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package org.eclipse.swt.graphics;
2+
/**
3+
* @noreference This class is not intended to be referenced by clients
4+
*/
5+
public enum RoundingMode {
6+
ROUND, UP;
7+
8+
public int round(float x) {
9+
if (this == ROUND) {
10+
return Math.round(x);
11+
}
12+
if (this == UP) {
13+
return (int) Math.ceil(x);
14+
}
15+
return (int) x;
16+
}
17+
}

‎bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2036,7 +2036,7 @@ private class DrawPointOperation extends Operation {
20362036

20372037
@Override
20382038
void apply() {
2039-
Point scaleUpLocation = Win32DPIUtils.pointToPixel(location, getZoom());
2039+
Point scaleUpLocation = Win32DPIUtils.pointToPixelAsLocation(location, getZoom());
20402040
drawPointInPixels(scaleUpLocation.x, scaleUpLocation.y);
20412041
}
20422042
}

‎bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Region.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ public boolean contains (Point pt) {
232232
if (pt == null) SWT.error(SWT.ERROR_NULL_ARGUMENT);
233233
return applyUsingAnyHandle(regionHandle -> {
234234
int zoom = regionHandle.zoom();
235-
Point p = Win32DPIUtils.pointToPixel(pt, zoom);
235+
Point p = Win32DPIUtils.pointToPixelAsLocation(pt, zoom);
236236
return containsInPixels(regionHandle.handle(), p.x, p.y);
237237
});
238238
}
@@ -889,7 +889,7 @@ void intersect(long handle, int zoom) {
889889

890890
@Override
891891
void translate(long handle, int zoom) {
892-
Point pt = Win32DPIUtils.pointToPixel((Point) data, zoom);
892+
Point pt = Win32DPIUtils.pointToPixelAsLocation((Point) data, zoom);
893893
OS.OffsetRgn (handle, pt.x, pt.y);
894894
}
895895

‎bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/internal/Win32DPIUtils.java‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,25 @@ public static float pixelToPoint(Drawable drawable, float size, int zoom) {
114114
}
115115

116116
public static Point pixelToPoint(Point point, int zoom) {
117+
//TODO actually all callers should explicitly state what mode the want!
118+
return pixelToPoint(point, zoom, RoundingMode.ROUND);
119+
}
120+
121+
public static Point pixelToPointAsSize(Point point, int zoom) {
122+
return pixelToPoint(point, zoom, RoundingMode.UP);
123+
}
124+
125+
public static Point pixelToPointAsLocation(Point point, int zoom) {
126+
return pixelToPoint(point, zoom, RoundingMode.ROUND);
127+
}
128+
129+
public static Point pixelToPoint(Point point, int zoom, RoundingMode mode) {
117130
if (zoom == 100 || point == null) return point;
118131
Point.OfFloat fPoint = Point.OfFloat.from(point);
119132
float scaleFactor = DPIUtil.getScalingFactor(zoom);
120133
float scaledX = fPoint.getX() / scaleFactor;
121134
float scaledY = fPoint.getY() / scaleFactor;
122-
return new Point.OfFloat(scaledX, scaledY);
135+
return new Point.OfFloat(scaledX, scaledY, mode);
123136
}
124137

125138
public static Point pixelToPoint(Drawable drawable, Point point, int zoom) {
@@ -219,26 +232,34 @@ public static float pointToPixel(Drawable drawable, float size, int zoom) {
219232
return pointToPixel (size, zoom);
220233
}
221234

222-
public static Point pointToPixel(Point point, int zoom) {
235+
public static Point pointToPixel(Point point, int zoom, RoundingMode mode) {
223236
if (zoom == 100 || point == null) return point;
224237
Point.OfFloat fPoint = Point.OfFloat.from(point);
225238
float scaleFactor = DPIUtil.getScalingFactor(zoom);
226239
float scaledX = fPoint.getX() * scaleFactor;
227240
float scaledY = fPoint.getY() * scaleFactor;
228-
return new Point.OfFloat(scaledX, scaledY);
241+
return new Point.OfFloat(scaledX, scaledY, mode);
242+
}
243+
244+
public static Point pointToPixelAsSize(Point point, int zoom) {
245+
return pointToPixel(point, zoom, RoundingMode.UP);
246+
}
247+
248+
public static Point pointToPixelAsLocation(Point point, int zoom) {
249+
return pointToPixel(point, zoom, RoundingMode.ROUND);
229250
}
230251

231252
public static Point pointToPixel(Drawable drawable, Point point, int zoom) {
232253
if (drawable != null && !drawable.isAutoScalable()) return point;
233-
return pointToPixel (point, zoom);
254+
return pointToPixel (point, zoom, RoundingMode.ROUND);
234255
}
235256

236257
public static Rectangle pointToPixel(Rectangle rect, int zoom) {
237258
if (zoom == 100 || rect == null) return rect;
238259
if (rect instanceof Rectangle.OfFloat rectOfFloat) return pointToPixel(rectOfFloat, zoom);
239260
Rectangle scaledRect = new Rectangle.OfFloat(0,0,0,0);
240-
Point scaledTopLeft = pointToPixel (new Point(rect.x, rect.y), zoom);
241-
Point scaledBottomRight = pointToPixel (new Point(rect.x + rect.width, rect.y + rect.height), zoom);
261+
Point scaledTopLeft = pointToPixel (new Point(rect.x, rect.y), zoom, RoundingMode.ROUND);
262+
Point scaledBottomRight = pointToPixel (new Point(rect.x + rect.width, rect.y + rect.height), zoom, RoundingMode.ROUND);
242263

243264
scaledRect.x = scaledTopLeft.x;
244265
scaledRect.y = scaledTopLeft.y;

‎bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Composite.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ Point computeSizeInPixels (int wHint, int hHint, boolean changed) {
219219
changed |= (state & LAYOUT_CHANGED) != 0;
220220
state &= ~LAYOUT_CHANGED;
221221
int zoom = getZoom();
222-
size = Win32DPIUtils.pointToPixel(layout.computeSize (this, DPIUtil.pixelToPoint(wHint, zoom), DPIUtil.pixelToPoint(hHint, zoom), changed), zoom);
222+
size = Win32DPIUtils.pointToPixelAsSize(layout.computeSize (this, DPIUtil.pixelToPoint(wHint, zoom), DPIUtil.pixelToPoint(hHint, zoom), changed), zoom);
223223
} else {
224224
size = new Point (wHint, hHint);
225225
}

‎bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Control.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,9 @@ public Point computeSize (int wHint, int hHint, boolean changed){
620620
int zoom = getZoom();
621621
wHint = (wHint != SWT.DEFAULT ? Win32DPIUtils.pointToPixel(wHint, zoom) : wHint);
622622
hHint = (hHint != SWT.DEFAULT ? Win32DPIUtils.pointToPixel(hHint, zoom) : hHint);
623-
return Win32DPIUtils.pixelToPoint(computeSizeInPixels(wHint, hHint, changed), zoom);
623+
//We should never return a size that is to small, RoundingMode.UP ensures we at worst case report
624+
//a size that is a bit too large by half a point
625+
return Win32DPIUtils.pixelToPoint(computeSizeInPixels(wHint, hHint, changed), zoom, RoundingMode.UP);
624626
}
625627

626628
Point computeSizeInPixels (int wHint, int hHint, boolean changed) {
@@ -1372,7 +1374,8 @@ public Object getLayoutData () {
13721374
*/
13731375
public Point getLocation () {
13741376
checkWidget ();
1375-
return Win32DPIUtils.pixelToPoint(getLocationInPixels(), getZoom());
1377+
//For a location the closest point values is okay
1378+
return Win32DPIUtils.pixelToPoint(getLocationInPixels(), getZoom(), RoundingMode.ROUND);
13761379
}
13771380

13781381
Point getLocationInPixels () {
@@ -1528,7 +1531,7 @@ public Shell getShell () {
15281531
*/
15291532
public Point getSize (){
15301533
checkWidget ();
1531-
return Win32DPIUtils.pixelToPoint(getSizeInPixels (), getZoom());
1534+
return Win32DPIUtils.pixelToPoint(getSizeInPixels (), getZoom(), RoundingMode.UP);
15321535
}
15331536

15341537
Point getSizeInPixels () {
@@ -3552,7 +3555,7 @@ void setLocationInPixels (int x, int y) {
35523555
public void setLocation (Point location) {
35533556
checkWidget ();
35543557
if (location == null) error (SWT.ERROR_NULL_ARGUMENT);
3555-
location = Win32DPIUtils.pointToPixel(location, getZoom());
3558+
location = Win32DPIUtils.pointToPixelAsLocation(location, getZoom());
35563559
setLocationInPixels(location.x, location.y);
35573560
}
35583561

@@ -3815,7 +3818,7 @@ void setSizeInPixels (int width, int height) {
38153818
public void setSize (Point size) {
38163819
checkWidget ();
38173820
if (size == null) error (SWT.ERROR_NULL_ARGUMENT);
3818-
size = Win32DPIUtils.pointToPixel(size, getZoom());
3821+
size = Win32DPIUtils.pointToPixelAsSize(size, getZoom());
38193822
setSizeInPixels(size.x, size.y);
38203823
}
38213824

@@ -4030,7 +4033,7 @@ public Point toControl (int x, int y) {
40304033
checkWidget ();
40314034
Point displayPointInPixels = getDisplay().translateToDisplayCoordinates(new Point(x, y));
40324035
final Point controlPointInPixels = toControlInPixels(displayPointInPixels.x, displayPointInPixels.y);
4033-
return Win32DPIUtils.pixelToPoint(controlPointInPixels, getZoom());
4036+
return Win32DPIUtils.pixelToPoint(controlPointInPixels, getZoom(), RoundingMode.ROUND);
40344037
}
40354038

40364039
Point toControlInPixels (int x, int y) {

0 commit comments

Comments
 (0)