Skip to content

[Win32] Make Control bounds point/pixel conversion cope with rounding #2782

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+210 −8
Open

[Win32] Make Control bounds point/pixel conversion cope with rounding #2782

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
123 changes: 121 additions & 2 deletions ...es/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/widgets/ControlWin32Tests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ public void testCorrectScaleUpUsingDifferentSetBoundsMethod() {

button.setBounds(new Rectangle(0, 47, 200, 47));
assertEquals("Control::setBounds(Rectangle) doesn't scale up correctly",
new Rectangle(0, 82, 350, 83), button.getBoundsInPixels());
new Rectangle(0, 82, 350, 82), button.getBoundsInPixels());

button.setBounds(0, 47, 200, 47);
assertEquals("Control::setBounds(int, int, int, int) doesn't scale up correctly",
new Rectangle(0, 82, 350, 83), button.getBoundsInPixels());
new Rectangle(0, 82, 350, 82), button.getBoundsInPixels());
}

@ParameterizedTest
Expand Down Expand Up @@ -155,4 +155,123 @@ private FontComparison updateFont(int scalingFactor) {
return new FontComparison(heightInPixels, currentHeightInPixels);
}

/**
* Scenario:
* <ul>
* <li>parent has bounds with an offset (x != 0) to its parent
* <li>child fills the composite, such that both their widths are equal
* </ul>
* Depending on how the offset of the parent (x value of bounds) is taken
* into account when rounding during point-to-pixel conversion, the parent
* composite may become one pixel too large or small for the child.
*/
@Test
void testChildFillsCompositeWithOffset() {
Win32DPIUtils.setMonitorSpecificScaling(true);
// pixel values at 125%: (2.5, 2.5, 2.5, 2.5) --> when rounding bottom right
// corner (pixel value (5, 5)) instead of width/height independently, will be
// rounded to (3, 3, 2, 2) --> too small for child
Rectangle parentBounds = new Rectangle(2, 2, 2, 2);
// pixel values at 125%: (0, 0, 2.5, 2.5) --> will be rounded to (0, 0, 3, 3)
Rectangle childBounds = new Rectangle(0, 0, 2, 2);

Display display = Display.getDefault();
Shell shell = new Shell(display);
Composite parent = new Composite(shell, SWT.NONE);
DPITestUtil.changeDPIZoom(shell, 125);
parent.setBounds(parentBounds);
Button child = new Button(parent, SWT.PUSH);
child.setBounds(childBounds);

Rectangle parentBoundsInPixels = parent.getBoundsInPixels();
Rectangle childBoundsInPixels = child.getBoundsInPixels();
assertEquals(parentBoundsInPixels.x, 3);
assertEquals(childBoundsInPixels.x, 0);
assertEquals(parentBoundsInPixels.width, childBoundsInPixels.width);
assertEquals(parentBoundsInPixels.height, childBoundsInPixels.height);
assertEquals(childBounds, child.getBounds());
}

/**
* Scenario:
* <ul>
* <li>parent has bounds with an offset (x = 0) to its parent
* <li>child has an offset (x != 0) to parent and exactly fills the rest of the
* composite, such that child.x+child.width is equal to parent.x
* </ul>
* Depending on how the offset of the child (x value of bounds) is taken into
* account when rounding during point-to-pixel conversion, the child may become
* one pixel too large to fit into the parent.
*/
@Test
void testChildWithOffsetFillsComposite() {
Win32DPIUtils.setMonitorSpecificScaling(true);
// pixel values at 125%: (0, 0, 5, 5)
Rectangle parentBounds = new Rectangle(0, 0, 4, 4);
// pixel values at 125%: (2.5, 2.5, 2.5, 2.5) --> when rounding width/height
// independently instead of bottom right corner, will be rounded to
// (3, 3, 3, 3) --> too large for parent
Rectangle childBounds = new Rectangle(2, 2, 2, 2);

Display display = Display.getDefault();
Shell shell = new Shell(display);
Composite parent = new Composite(shell, SWT.NONE);
DPITestUtil.changeDPIZoom(shell, 125);
parent.setBounds(parentBounds);
Button child = new Button(parent, SWT.PUSH);
child.setBounds(childBounds);

Rectangle parentBoundsInPixels = parent.getBoundsInPixels();
Rectangle childBoundsInPixels = child.getBoundsInPixels();
assertEquals(parentBoundsInPixels.x, 0);
assertEquals(childBoundsInPixels.x, 3);
assertEquals(parentBoundsInPixels.width, childBoundsInPixels.x + childBoundsInPixels.width);
assertEquals(parentBoundsInPixels.height, childBoundsInPixels.y + childBoundsInPixels.height);
assertEquals(parentBounds, parent.getBounds());
assertEquals(childBounds, child.getBounds());
}

/**
* Scenario: Layouting
* <p>
* Layouts use client area of composites to calculate the sizes of the contained
* controls. The rounded values of that client area can lead to child bounds be
* calculated larger than the actual available size.
*/
@Test
void testChildFillsScrollableWithBadlyRoundedClientArea() {
Win32DPIUtils.setMonitorSpecificScaling(true);
Display display = Display.getDefault();
Shell shell = new Shell(display);
Composite parent = new Composite(shell, SWT.H_SCROLL|SWT.V_SCROLL);
DPITestUtil.changeDPIZoom(shell, 125);
// Find parent bounds such that client area is rounded to a value that,
// when converted back to pixels, is one pixel too large
Rectangle parentBounds = new Rectangle(0, 0, 4, 4);
Rectangle clientAreaInPixels;
do {
do {
parentBounds.width += 1;
parentBounds.height += 1;
parent.setBounds(parentBounds);
Rectangle clientArea = parent.getClientArea();
clientAreaInPixels = Win32DPIUtils
.pointToPixel(new Rectangle(clientArea.x, clientArea.y, clientArea.width, clientArea.height), 125);
} while (clientAreaInPixels.width <= parent.getClientAreaInPixels().width && clientAreaInPixels.width < 50);
parentBounds.x += 1;
parentBounds.y += 1;
if (parentBounds.x >= 50) {
fail("No scrolable size with non-invertible point/pixel conversion for its client area could be created");
}
} while (clientAreaInPixels.width <= parent.getClientAreaInPixels().width);
Button child = new Button(parent, SWT.PUSH);
Rectangle childBounds = new Rectangle(0, 0, parent.getClientArea().width, parent.getClientArea().height);
child.setBounds(childBounds);

clientAreaInPixels = parent.getClientAreaInPixels();
Rectangle childBoundsInPixels = child.getBoundsInPixels();
assertTrue(clientAreaInPixels.width <= childBoundsInPixels.x + childBoundsInPixels.width);
assertTrue(clientAreaInPixels.height <= childBoundsInPixels.y + childBoundsInPixels.height);
}

}
95 changes: 89 additions & 6 deletions bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Control.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3302,7 +3302,83 @@ public void setBounds (Rectangle rect) {
checkWidget ();
if (rect == null) error (SWT.ERROR_NULL_ARGUMENT);
int zoom = computeBoundsZoom();
setBoundsInPixels(Win32DPIUtils.pointToPixel(rect, zoom));
Rectangle boundsInPixels = boundsToPixelsViaShellCoordinates(rect, zoom);
fitInParentBounds(boundsInPixels, zoom);
setBoundsInPixels(boundsInPixels);
}

/**
* Converts bounds to pixels via the shell coordinate system, such that the
* coordinates for every control are rounded in the same. Otherwise, child and
* parent controls may not fit as their coordinates are relative to different
* coordinate systems (the ones with their individual parent as origin), such
* that applied rounding leads to different values for the actually same
* coordinates. One consequence when not doing this is that child controls with
* x and y set to 0 and width and height set to the parent's bounds may be
* larger than the parent.
*/
private Rectangle boundsToPixelsViaShellCoordinates(Rectangle bounds, int zoom) {
Point.OfFloat parentOffsetToShell = calculateParentOffsetToShell();
Point.OfFloat parentOffsetToShellInPixels = Point.OfFloat
.from(Win32DPIUtils.pointToPixelAsSize(parentOffsetToShell, zoom));
Rectangle.OfFloat offsetRectangle = new Rectangle.OfFloat(bounds.x + parentOffsetToShell.getX(),
bounds.y + parentOffsetToShell.getY(), bounds.width, bounds.height);
Rectangle.OfFloat offsetRectangleInPixels = Rectangle.OfFloat
.from(Win32DPIUtils.pointToPixel(offsetRectangle, zoom));
int xInPixels = offsetRectangleInPixels.x - parentOffsetToShellInPixels.x;
int yInPixels = offsetRectangleInPixels.y - parentOffsetToShellInPixels.y;
int widthInPixels = offsetRectangleInPixels.width;
int heightInPixels = offsetRectangleInPixels.height;
return new Rectangle(xInPixels, yInPixels, widthInPixels, heightInPixels);
}

private Point.OfFloat calculateParentOffsetToShell() {
float parentX = 0;
float parentY = 0;
Control parent = getParent();
while (parent != null & !(parent instanceof Shell)) {
Rectangle.OfFloat parentLocation = Rectangle.OfFloat.from(parent.getBounds());
parentX += parentLocation.getX();
parentY += parentLocation.getY();
parent = parent.getParent();
}
return new Point.OfFloat(parentX, parentY);
}

/**
* Cope with limited invertibility of pixel/point conversions.
* <p>
* Example: 125% monitor, layout fills composite with single child
* <ul>
* <li>Composite with client area of 527 pixels
* <li>getClientArea() returns 527 / 1,25 = 421,6 points
* <li>So layout sets rounded 422 points to child
* <li>This conforms to 422 * 1,25 = 527,5 pixels, which is rounded up to 528,
* i.e., one more than parent size
* </ul>
* Alternatives:
* <ul>
* <li>rounding down the client area instead could lead to areas not redrawn, as
* rounded down 421 points result in 526 pixels, one less than the actual size
* of the composite
* <li>rounding down the passed bounds leads to controls becoming unnecessarily
* smaller than their calculated size
* </ul>
* Thus, reduce the control size in case it would not fit anyway
*/
private void fitInParentBounds(Rectangle boundsInPixels, int zoom) {
if (parent == null) {
return;
}
Rectangle parentBounds = parent.getBoundsInPixels();
if (parentBounds.width < boundsInPixels.x + boundsInPixels.width
&& parentBounds.width >= boundsInPixels.x + boundsInPixels.width - Win32DPIUtils.pointToPixel(1.0f, zoom)) {
boundsInPixels.width = parentBounds.width - boundsInPixels.x;
}
if (parentBounds.height < boundsInPixels.y + boundsInPixels.height && parentBounds.height >= boundsInPixels.y
+ boundsInPixels.height - Win32DPIUtils.pointToPixel(1.0f, zoom)) {
boundsInPixels.height = parentBounds.height - boundsInPixels.y;
}
}

void setBoundsInPixels (Rectangle rect) {
Expand Down Expand Up @@ -3817,9 +3893,7 @@ public void setRegion (Region region) {
public void setSize (int width, int height) {
checkWidget ();
int zoom = computeBoundsZoom();
width = DPIUtil.pointToPixel(width, zoom);
height = DPIUtil.pointToPixel(height, zoom);
setSizeInPixels(width, height);
setSize(new Point(width, height), zoom);
}

void setSizeInPixels (int width, int height) {
Expand Down Expand Up @@ -3853,8 +3927,17 @@ void setSizeInPixels (int width, int height) {
public void setSize (Point size) {
checkWidget ();
if (size == null) error (SWT.ERROR_NULL_ARGUMENT);
size = Win32DPIUtils.pointToPixelAsSize(size, computeBoundsZoom());
setSizeInPixels(size.x, size.y);
int zoom = computeBoundsZoom();
setSize(size, zoom);
}

private void setSize(Point size, int zoom) {
Rectangle bounds = getBounds();
bounds.width = size.x;
bounds.height = size.y;
Rectangle boundsInPixels = boundsToPixelsViaShellCoordinates(bounds, zoom);
fitInParentBounds(boundsInPixels, zoom);
setSizeInPixels(boundsInPixels.width, boundsInPixels.height);
}

@Override
Expand Down
Loading