[Win32] Fix control bounds exceeding parent client area size

The invertibility of point/pixel conversions is limited as point values
are int-based and with lower resolution than pixel values. In
consequence, values need to be rounded when converted between the two,
which inevitably leads to rounded values that do not fit for every use
case. This adds test cases that demonstrate such use cases, including
simple parent/child scenarios, in which the child is supposed to fill
the parent, and including layouting scenarios incorporating the client
area of a composite, and how the current implementation is not capable
of producing proper results for them.

This change also adapts the methods for setting bounds/size of controls
to deal with the limited invertibility. They shrink the calculated pixel
values by at most the maximum error that can be made when transforming
from point to pixel values, such that rounding errors due to layouts
that calculated control bounds based on a composites client area are
evened out. Without that, layouted controls may be up to one point too
large to fit into the composite.

Author: HeikoKlare

bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/widgets/ControlWin32Tests.java

@@ -155,4 +155,205 @@ 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);
+		assertChildFitsIntoParent(parent, child);
+		assertEquals(parentBounds, parent.getBounds());
+		assertEquals(childBounds, child.getBounds());
+
+		child.setSize(childBounds.width, childBounds.height);
+		assertChildFitsIntoParent(parent, child);
+		assertEquals(parentBounds, parent.getBounds());
+		assertEquals(childBounds, child.getBounds());
+	}
+
+	private void assertChildFitsIntoParent(Control parent, Control child) {
+		Rectangle parentBoundsInPixels = parent.getBoundsInPixels();
+		Rectangle childBoundsInPixels = child.getBoundsInPixels();
+		Rectangle childBounds = child.getBounds();
+		assertEquals(parentBoundsInPixels.width, childBoundsInPixels.x + childBoundsInPixels.width);
+		assertEquals(parentBoundsInPixels.height, childBoundsInPixels.y + childBoundsInPixels.height);
+		assertEquals(parent.getBounds().width, childBounds.x + childBounds.width);
+		assertEquals(parent.getBounds().height, childBounds.y + childBounds.height);
+	}
+
+	/**
+	 * 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. This serves as unit test
+	 * for that behavior in addition to the integration test
+	 * {@link #testChildFillsScrollableWithBadlyRoundedClientArea()}
+	 */
+	@Test
+	void testFitChildIntoParent() {
+		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, 3.75, 3.75) --> rounded to (3, 3, 3, 3)
+		Rectangle childBounds = new Rectangle(2, 2, 3, 3);
+
+		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);
+		assertChildFitsIntoParent(parent, child);
+
+		child.setSize(childBounds.width, childBounds.height);
+		assertChildFitsIntoParent(parent, child);
+	}
+
+	// Ensure that the fitting logic does only apply at off-by-one calculation results
+	// and not for fitting actually larger childs into parts
+	@Test
+	void testFitChildIntoParent_limitedSizeDifference() {
+		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, 5, 5) --> rounded to (3, 3, 5, 5)
+		Rectangle childBounds = new Rectangle(2, 2, 4, 4);
+
+		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);
+		assertChildNotFitsIntoParent(parent, child);
+
+		child.setSize(childBounds.width, childBounds.height);
+		assertChildNotFitsIntoParent(parent, child);
+	}
+
+	private void assertChildNotFitsIntoParent(Control parent, Control child) {
+		Rectangle parentBoundsInPixels = parent.getBoundsInPixels();
+		Rectangle childBoundsInPixels = child.getBoundsInPixels();
+		Rectangle childBounds = child.getBounds();
+		assertNotEquals(parentBoundsInPixels.width, childBoundsInPixels.x + childBoundsInPixels.width);
+		assertNotEquals(parentBoundsInPixels.height, childBoundsInPixels.y + childBoundsInPixels.height);
+		assertNotEquals(parent.getBounds().width, childBounds.x + childBounds.width);
+		assertNotEquals(parent.getBounds().height, childBounds.y + childBounds.height);
+	}
+
+	/**
+	 * 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);
+
+		child.setSize(childBounds.width, childBounds.height);
+
+		clientAreaInPixels  = parent.getClientAreaInPixels();
+		childBoundsInPixels = child.getBoundsInPixels();
+		assertTrue(clientAreaInPixels.width <= childBoundsInPixels.x + childBoundsInPixels.width);
+		assertTrue(clientAreaInPixels.height <= childBoundsInPixels.y + childBoundsInPixels.height);
+	}
+
 }

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

@@ -3302,7 +3302,54 @@ public void setBounds (Rectangle rect) {
 	checkWidget ();
 	if (rect == null) error (SWT.ERROR_NULL_ARGUMENT);
 	int zoom = computeBoundsZoom();
-	setBoundsInPixels(Win32DPIUtils.pointToPixel(rect, zoom));
+	Rectangle boundsInPixels = Win32DPIUtils.pointToPixel(rect, zoom);
+	fitInParentBounds(boundsInPixels, zoom);
+	setBoundsInPixels(boundsInPixels);
+}
+
+
+/**
+ * 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 parentBoundsInPixels = parent.getBoundsInPixels();
+	// Check if child does not fit into parent, but that it would fit when taking an
+	// off-by-one for the width/height in points into account, as such an off-by-one
+	// can happen when doing bounds calculations in points due to rounding (e.g., in
+	// layouts)
+	boolean childNotFittingHorizontally = parentBoundsInPixels.width < boundsInPixels.x + boundsInPixels.width;
+	boolean childFittingHorizontallyWithOffByOne = parentBoundsInPixels.width >= boundsInPixels.x + boundsInPixels.width
+			- Win32DPIUtils.pointToPixel(1.0f, zoom);
+	if (childNotFittingHorizontally && childFittingHorizontallyWithOffByOne) {
+		boundsInPixels.width = parentBoundsInPixels.width - boundsInPixels.x;
+	}
+	boolean childNotFittingVertically = parentBoundsInPixels.height < boundsInPixels.y + boundsInPixels.height;
+	boolean childFittingVerticallyWithOffByOne = parentBoundsInPixels.height >= boundsInPixels.y + boundsInPixels.height
+			- Win32DPIUtils.pointToPixel(1.0f, zoom);
+	if (childNotFittingVertically && childFittingVerticallyWithOffByOne) {
+		boundsInPixels.height = parentBoundsInPixels.height - boundsInPixels.y;
+	}
 }
 
 void setBoundsInPixels (Rectangle rect) {
@@ -3817,9 +3864,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) {
@@ -3853,8 +3898,18 @@ 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) {
+	// Use the exact (non-rounded) bounds to apply proper rounding when converting the size from point to pixel
+	Rectangle bounds = Win32DPIUtils.pixelToPoint(Rectangle.OfFloat.from(getBoundsInPixels()), zoom);
+	bounds.width = size.x;
+	bounds.height = size.y;
+	Rectangle boundsInPixels = Win32DPIUtils.pointToPixel(bounds, zoom);
+	fitInParentBounds(boundsInPixels, zoom);
+	setSizeInPixels(boundsInPixels.width, boundsInPixels.height);
 }
 
 @Override