Skip to content

Commit a4d689b

Browse files
laeubiShahzaibIbrahim
authored andcommitted
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 - eclipse-platform#2381 - eclipse-platform#2166
1 parent 00106b4 commit a4d689b

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

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
@@ -2039,7 +2039,7 @@ private class DrawPointOperation extends Operation {
20392039

20402040
@Override
20412041
void apply() {
2042-
Point scaleUpLocation = Win32DPIUtils.pointToPixel(location, getZoom());
2042+
Point scaleUpLocation = Win32DPIUtils.pointToPixelAsLocation(location, getZoom());
20432043
drawPointInPixels(scaleUpLocation.x, scaleUpLocation.y);
20442044
}
20452045
}

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
@@ -215,7 +215,7 @@ Point computeSizeInPixels (int wHint, int hHint, boolean changed) {
215215
changed |= (state & LAYOUT_CHANGED) != 0;
216216
state &= ~LAYOUT_CHANGED;
217217
int zoom = getZoom();
218-
size = Win32DPIUtils.pointToPixel(layout.computeSize (this, DPIUtil.pixelToPoint(wHint, zoom), DPIUtil.pixelToPoint(hHint, zoom), changed), zoom);
218+
size = Win32DPIUtils.pointToPixelAsSize(layout.computeSize (this, DPIUtil.pixelToPoint(wHint, zoom), DPIUtil.pixelToPoint(hHint, zoom), changed), zoom);
219219
} else {
220220
size = new Point (wHint, hHint);
221221
}

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
@@ -616,7 +616,9 @@ public Point computeSize (int wHint, int hHint, boolean changed){
616616
int zoom = getZoom();
617617
wHint = (wHint != SWT.DEFAULT ? Win32DPIUtils.pointToPixel(wHint, zoom) : wHint);
618618
hHint = (hHint != SWT.DEFAULT ? Win32DPIUtils.pointToPixel(hHint, zoom) : hHint);
619-
return Win32DPIUtils.pixelToPoint(computeSizeInPixels(wHint, hHint, changed), zoom);
619+
//We should never return a size that is to small, RoundingMode.UP ensures we at worst case report
620+
//a size that is a bit too large by half a point
621+
return Win32DPIUtils.pixelToPoint(computeSizeInPixels(wHint, hHint, changed), zoom, RoundingMode.UP);
620622
}
621623

622624
Point computeSizeInPixels (int wHint, int hHint, boolean changed) {
@@ -1368,7 +1370,8 @@ public Object getLayoutData () {
13681370
*/
13691371
public Point getLocation () {
13701372
checkWidget ();
1371-
return Win32DPIUtils.pixelToPoint(getLocationInPixels(), getZoom());
1373+
//For a location the closest point values is okay
1374+
return Win32DPIUtils.pixelToPoint(getLocationInPixels(), getZoom(), RoundingMode.ROUND);
13721375
}
13731376

13741377
Point getLocationInPixels () {
@@ -1524,7 +1527,7 @@ public Shell getShell () {
15241527
*/
15251528
public Point getSize (){
15261529
checkWidget ();
1527-
return Win32DPIUtils.pixelToPoint(getSizeInPixels (), getZoom());
1530+
return Win32DPIUtils.pixelToPoint(getSizeInPixels (), getZoom(), RoundingMode.UP);
15281531
}
15291532

15301533
Point getSizeInPixels () {
@@ -3548,7 +3551,7 @@ void setLocationInPixels (int x, int y) {
35483551
public void setLocation (Point location) {
35493552
checkWidget ();
35503553
if (location == null) error (SWT.ERROR_NULL_ARGUMENT);
3551-
location = Win32DPIUtils.pointToPixel(location, getZoom());
3554+
location = Win32DPIUtils.pointToPixelAsLocation(location, getZoom());
35523555
setLocationInPixels(location.x, location.y);
35533556
}
35543557

@@ -3811,7 +3814,7 @@ void setSizeInPixels (int width, int height) {
38113814
public void setSize (Point size) {
38123815
checkWidget ();
38133816
if (size == null) error (SWT.ERROR_NULL_ARGUMENT);
3814-
size = Win32DPIUtils.pointToPixel(size, getZoom());
3817+
size = Win32DPIUtils.pointToPixelAsSize(size, getZoom());
38153818
setSizeInPixels(size.x, size.y);
38163819
}
38173820

@@ -4026,7 +4029,7 @@ public Point toControl (int x, int y) {
40264029
checkWidget ();
40274030
Point displayPointInPixels = getDisplay().translateToDisplayCoordinates(new Point(x, y));
40284031
final Point controlPointInPixels = toControlInPixels(displayPointInPixels.x, displayPointInPixels.y);
4029-
return Win32DPIUtils.pixelToPoint(controlPointInPixels, getZoom());
4032+
return Win32DPIUtils.pixelToPoint(controlPointInPixels, getZoom(), RoundingMode.ROUND);
40304033
}
40314034

40324035
Point toControlInPixels (int x, int y) {

0 commit comments

Comments
 (0)