Skip to content

Streamline rounding for rectangles and add rounding mode to them #2720

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 31, 2025
+67 −45
Merged

Streamline rounding for rectangles and add rounding mode to them #2720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/Point.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public void setY(float y) {

@Override
public Point.OfFloat clone() {
return new Point.OfFloat(getX(), getY());
return new Point.OfFloat(getX(), getY(), roundingMode);
}

/**
Expand Down
42 changes: 32 additions & 10 deletions bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/Rectangle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ public Rectangle (int x, int y, int width, int height) {
this.height = height;
}

private Rectangle() {
}

/**
* Destructively replaces the x, y, width and height values
* in the receiver with ones which represent the union of the
Expand Down Expand Up @@ -411,16 +414,25 @@ public static sealed class OfFloat extends Rectangle permits Rectangle.WithMonit

private float residualX, residualY, residualWidth, residualHeight;

private RoundingMode locationRounding = RoundingMode.ROUND;

private RoundingMode sizeRounding = RoundingMode.ROUND;

public OfFloat(int x, int y, int width, int height) {
super(x, y, width, height);
}

public OfFloat(float x, float y, float width, float height) {
super(Math.round(x), Math.round(y), Math.round(width), Math.round(height));
this.residualX = x - this.x;
this.residualY = y - this.y;
this.residualWidth = width - this.width;
this.residualHeight = height - this.height;
this(x, y, width, height, RoundingMode.ROUND, RoundingMode.ROUND);
}

private OfFloat(float x, float y, float width, float height, RoundingMode locationRounding, RoundingMode sizeRounding) {
this.locationRounding = locationRounding;
this.sizeRounding = sizeRounding;
setX(x);
setY(y);
setWidth(width);
setHeight(height);
}

public float getX() {
Expand All @@ -440,28 +452,38 @@ public float getHeight() {
}

public void setX(float x) {
this.x = Math.round(x);
this.x = locationRounding.round(x);
this.residualX = x - this.x;
setWidth(getWidth());
}

public void setY(float y) {
this.y = Math.round(y);
this.y = locationRounding.round(y);
this.residualY = y - this.y;
setHeight(getHeight());
}

public void setWidth(float width) {
this.width = Math.round(width);
this.width = sizeRounding.round(width + getX()) - x;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember there were cases in the past where some places would have different results when you would scale the rectangle using scaleBounds or scaleUp because they both used different approach to scaling. One had height and width calculated using the difference between top left and bottom right and the other as just the width and height values in the object. Are we sure that this approach would not break that behavior for certain consumers? Visually I cannot see anything in the IDE but I just wanted to pick this out just to be careful. I am going to consider that since there's no test failing, this might not be the case anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is no way to be 100% sure that we to not break any behavior for existing consumers. With all the back-and-forth of the kinds of rounding we have implemented, this will probably also have effects that we currently do not see (either positive or negative).
Regarding the different scaling methods that you mention: they are still in place. We still have Win32DPIUtils#scaleBounds() which implement the "other" way of scaling (used for things related to image). The modified way of scaling only applies to what is passed to Win32DPIUtils#pointToPixel()/pixelToPoint(). And there the only removed case distinction is that when a Rectangle.OfFloat was passed, no forwarding to scaleBounds() happens anymore.

this.residualWidth = width - this.width;
}

public void setHeight(float height) {
this.height = Math.round(height);
this.height = sizeRounding.round(height + getY()) - y;
this.residualHeight = height - this.height;
}

public Point.OfFloat getTopLeft() {
return new Point.OfFloat(getX(), getY(), locationRounding);
}

public Point.OfFloat getBottomRight() {
return new Point.OfFloat(getX() + getWidth(), getY() + getHeight(), sizeRounding);
}

@Override
public Rectangle.OfFloat clone() {
return new Rectangle.OfFloat(getX(), getY(), getWidth(), getHeight());
return new Rectangle.OfFloat(getX(), getY(), getWidth(), getHeight(), locationRounding, sizeRounding);
}

/**
Expand Down
68 changes: 34 additions & 34 deletions bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/internal/Win32DPIUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,31 +137,31 @@ public static Point pixelToPointAsConservativeSize(Point point, int zoom) {

private static Point pixelToPoint(Point point, int zoom, RoundingMode mode) {
if (zoom == 100 || point == null) return point;
Point.OfFloat fPoint = Point.OfFloat.from(point);
Point.OfFloat floatPoint = Point.OfFloat.from(point);
return pixelToPoint(new Point.OfFloat(floatPoint.getX(), floatPoint.getY(), mode), zoom);
}

private static Point.OfFloat pixelToPoint(Point.OfFloat point, int zoom) {
Point.OfFloat scaledPoint = point.clone();
float scaleFactor = DPIUtil.getScalingFactor(zoom);
float scaledX = fPoint.getX() / scaleFactor;
float scaledY = fPoint.getY() / scaleFactor;
return new Point.OfFloat(scaledX, scaledY, mode);
scaledPoint.setX(point.getX() / scaleFactor);
scaledPoint.setY(point.getY() / scaleFactor);
return scaledPoint;
}

public static Rectangle pixelToPoint(Rectangle rect, int zoom) {
if (zoom == 100 || rect == null) return rect;
if (rect instanceof Rectangle.OfFloat rectOfFloat) return pixelToPoint(rectOfFloat, zoom);
Rectangle scaledRect = new Rectangle.OfFloat (0,0,0,0);
Point scaledTopLeft = pixelToPointAsLocation(new Point (rect.x, rect.y), zoom);
Point scaledBottomRight = pixelToPointAsSize(new Point (rect.x + rect.width, rect.y + rect.height), zoom);

scaledRect.x = scaledTopLeft.x;
scaledRect.y = scaledTopLeft.y;
scaledRect.width = scaledBottomRight.x - scaledTopLeft.x;
scaledRect.height = scaledBottomRight.y - scaledTopLeft.y;
Rectangle.OfFloat floatRect = Rectangle.OfFloat.from(rect);
Point.OfFloat scaledTopLeft = pixelToPoint(floatRect.getTopLeft(), zoom);
Point.OfFloat scaledBottomRight = pixelToPoint(floatRect.getBottomRight(), zoom);
Rectangle.OfFloat scaledRect = floatRect.clone();
scaledRect.setX(scaledTopLeft.getX());
scaledRect.setY(scaledTopLeft.getY());
scaledRect.setWidth(scaledBottomRight.getX() - scaledTopLeft.getX());
scaledRect.setHeight(scaledBottomRight.getY() - scaledTopLeft.getY());
return scaledRect;
}

private static Rectangle pixelToPoint(Rectangle.OfFloat rect, int zoom) {
return scaleBounds(rect, 100, zoom);
}

public static Rectangle pixelToPoint(Drawable drawable, Rectangle rect, int zoom) {
if (drawable != null && !drawable.isAutoScalable()) return rect;
return pixelToPoint (rect, zoom);
Expand Down Expand Up @@ -229,11 +229,16 @@ public static float pointToPixel(Drawable drawable, float size, int zoom) {

private static Point pointToPixel(Point point, int zoom, RoundingMode mode) {
if (zoom == 100 || point == null) return point;
Point.OfFloat fPoint = Point.OfFloat.from(point);
Point.OfFloat floatPoint = Point.OfFloat.from(point);
return pointToPixel(new Point.OfFloat(floatPoint.getX(), floatPoint.getY(), mode), zoom);
}

private static Point.OfFloat pointToPixel(Point.OfFloat point, int zoom) {
Point.OfFloat scaledPoint = point.clone();
float scaleFactor = DPIUtil.getScalingFactor(zoom);
float scaledX = fPoint.getX() * scaleFactor;
float scaledY = fPoint.getY() * scaleFactor;
return new Point.OfFloat(scaledX, scaledY, mode);
scaledPoint.setX(point.getX() * scaleFactor);
scaledPoint.setY(point.getY() * scaleFactor);
return scaledPoint;
}

public static Point pointToPixelAsSize(Drawable drawable, Point point, int zoom) {
Expand All @@ -256,22 +261,17 @@ public static Point pointToPixelAsLocation(Point point, int zoom) {

public static Rectangle pointToPixel(Rectangle rect, int zoom) {
if (zoom == 100 || rect == null) return rect;
if (rect instanceof Rectangle.OfFloat rectOfFloat) return pointToPixel(rectOfFloat, zoom);
Rectangle scaledRect = new Rectangle.OfFloat(0,0,0,0);
Point scaledTopLeft = pointToPixelAsLocation (new Point(rect.x, rect.y), zoom);
Point scaledBottomRight = pointToPixelAsLocation (new Point(rect.x + rect.width, rect.y + rect.height), zoom);

scaledRect.x = scaledTopLeft.x;
scaledRect.y = scaledTopLeft.y;
scaledRect.width = scaledBottomRight.x - scaledTopLeft.x;
scaledRect.height = scaledBottomRight.y - scaledTopLeft.y;
Rectangle.OfFloat floatRect = Rectangle.OfFloat.from(rect);
Point.OfFloat scaledTopLeft = pointToPixel(floatRect.getTopLeft(), zoom);
Point.OfFloat scaledBottomRight = pointToPixel(floatRect.getBottomRight(), zoom);
Rectangle.OfFloat scaledRect = floatRect.clone();
scaledRect.setX(scaledTopLeft.getX());
scaledRect.setY(scaledTopLeft.getY());
scaledRect.setWidth(scaledBottomRight.getX() - scaledTopLeft.getX());
scaledRect.setHeight(scaledBottomRight.getY() - scaledTopLeft.getY());
return scaledRect;
}

private static Rectangle pointToPixel(Rectangle.OfFloat rect, int zoom) {
return scaleBounds(rect, zoom, 100);
}

public static Rectangle pointToPixel(Drawable drawable, Rectangle rect, int zoom) {
if (drawable != null && !drawable.isAutoScalable()) return rect;
return pointToPixel (rect, zoom);
Expand Down
Loading