Skip to content

Commit eb2f91a

Browse files
committed
[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.
1 parent ee0856f commit eb2f91a

File tree

2 files changed

+171
-6
lines changed

2 files changed

+171
-6
lines changed

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

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,4 +155,123 @@ private FontComparison updateFont(int scalingFactor) {
155155
return new FontComparison(heightInPixels, currentHeightInPixels);
156156
}
157157

158+
/**
159+
* Scenario:
160+
* <ul>
161+
* <li>parent has bounds with an offset (x != 0) to its parent
162+
* <li>child fills the composite, such that both their widths are equal
163+
* </ul>
164+
* Depending on how the offset of the parent (x value of bounds) is taken
165+
* into account when rounding during point-to-pixel conversion, the parent
166+
* composite may become one pixel too large or small for the child.
167+
*/
168+
@Test
169+
void testChildFillsCompositeWithOffset() {
170+
Win32DPIUtils.setMonitorSpecificScaling(true);
171+
// pixel values at 125%: (2.5, 2.5, 2.5, 2.5) --> when rounding bottom right
172+
// corner (pixel value (5, 5)) instead of width/height independently, will be
173+
// rounded to (3, 3, 2, 2) --> too small for child
174+
Rectangle parentBounds = new Rectangle(2, 2, 2, 2);
175+
// pixel values at 125%: (0, 0, 2.5, 2.5) --> will be rounded to (0, 0, 3, 3)
176+
Rectangle childBounds = new Rectangle(0, 0, 2, 2);
177+
178+
Display display = Display.getDefault();
179+
Shell shell = new Shell(display);
180+
Composite parent = new Composite(shell, SWT.NONE);
181+
DPITestUtil.changeDPIZoom(shell, 125);
182+
parent.setBounds(parentBounds);
183+
Button child = new Button(parent, SWT.PUSH);
184+
child.setBounds(childBounds);
185+
186+
Rectangle parentBoundsInPixels = parent.getBoundsInPixels();
187+
Rectangle childBoundsInPixels = child.getBoundsInPixels();
188+
assertEquals(parentBoundsInPixels.x, 3);
189+
assertEquals(childBoundsInPixels.x, 0);
190+
assertEquals(parentBoundsInPixels.width, childBoundsInPixels.width);
191+
assertEquals(parentBoundsInPixels.height, childBoundsInPixels.height);
192+
assertEquals(childBounds, child.getBounds());
193+
}
194+
195+
/**
196+
* Scenario:
197+
* <ul>
198+
* <li>parent has bounds with an offset (x = 0) to its parent
199+
* <li>child has an offset (x != 0) to parent and exactly fills the rest of the
200+
* composite, such that child.x+child.width is equal to parent.x
201+
* </ul>
202+
* Depending on how the offset of the child (x value of bounds) is taken into
203+
* account when rounding during point-to-pixel conversion, the child may become
204+
* one pixel too large to fit into the parent.
205+
*/
206+
@Test
207+
void testChildWithOffsetFillsComposite() {
208+
Win32DPIUtils.setMonitorSpecificScaling(true);
209+
// pixel values at 125%: (0, 0, 5, 5)
210+
Rectangle parentBounds = new Rectangle(0, 0, 4, 4);
211+
// pixel values at 125%: (2.5, 2.5, 2.5, 2.5) --> when rounding width/height
212+
// independently instead of bottom right corner, will be rounded to
213+
// (3, 3, 3, 3) --> too large for parent
214+
Rectangle childBounds = new Rectangle(2, 2, 2, 2);
215+
216+
Display display = Display.getDefault();
217+
Shell shell = new Shell(display);
218+
Composite parent = new Composite(shell, SWT.NONE);
219+
DPITestUtil.changeDPIZoom(shell, 125);
220+
parent.setBounds(parentBounds);
221+
Button child = new Button(parent, SWT.PUSH);
222+
child.setBounds(childBounds);
223+
224+
Rectangle parentBoundsInPixels = parent.getBoundsInPixels();
225+
Rectangle childBoundsInPixels = child.getBoundsInPixels();
226+
assertEquals(parentBoundsInPixels.x, 0);
227+
assertEquals(childBoundsInPixels.x, 3);
228+
assertEquals(parentBoundsInPixels.width, childBoundsInPixels.x + childBoundsInPixels.width);
229+
assertEquals(parentBoundsInPixels.height, childBoundsInPixels.y + childBoundsInPixels.height);
230+
assertEquals(parentBounds, parent.getBounds());
231+
assertEquals(childBounds, child.getBounds());
232+
}
233+
234+
/**
235+
* Scenario: Layouting
236+
* <p>
237+
* Layouts use client area of composites to calculate the sizes of the contained
238+
* controls. The rounded values of that client area can lead to child bounds be
239+
* calculated larger than the actual available size.
240+
*/
241+
@Test
242+
void testChildFillsScrollableWithBadlyRoundedClientArea() {
243+
Win32DPIUtils.setMonitorSpecificScaling(true);
244+
Display display = Display.getDefault();
245+
Shell shell = new Shell(display);
246+
Composite parent = new Composite(shell, SWT.H_SCROLL|SWT.V_SCROLL);
247+
DPITestUtil.changeDPIZoom(shell, 125);
248+
// Find parent bounds such that client area is rounded to a value that,
249+
// when converted back to pixels, is one pixel too large
250+
Rectangle parentBounds = new Rectangle(0, 0, 4, 4);
251+
Rectangle clientAreaInPixels;
252+
do {
253+
do {
254+
parentBounds.width += 1;
255+
parentBounds.height += 1;
256+
parent.setBounds(parentBounds);
257+
Rectangle clientArea = parent.getClientArea();
258+
clientAreaInPixels = Win32DPIUtils
259+
.pointToPixel(new Rectangle(clientArea.x, clientArea.y, clientArea.width, clientArea.height), 125);
260+
} while (clientAreaInPixels.width <= parent.getClientAreaInPixels().width && clientAreaInPixels.width < 50);
261+
parentBounds.x += 1;
262+
parentBounds.y += 1;
263+
if (parentBounds.x >= 50) {
264+
fail("No scrolable size with non-invertible point/pixel conversion for its client area could be created");
265+
}
266+
} while (clientAreaInPixels.width <= parent.getClientAreaInPixels().width);
267+
Button child = new Button(parent, SWT.PUSH);
268+
Rectangle childBounds = new Rectangle(0, 0, parent.getClientArea().width, parent.getClientArea().height);
269+
child.setBounds(childBounds);
270+
271+
clientAreaInPixels = parent.getClientAreaInPixels();
272+
Rectangle childBoundsInPixels = child.getBoundsInPixels();
273+
assertTrue(clientAreaInPixels.width <= childBoundsInPixels.x + childBoundsInPixels.width);
274+
assertTrue(clientAreaInPixels.height <= childBoundsInPixels.y + childBoundsInPixels.height);
275+
}
276+
158277
}

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

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3302,7 +3302,46 @@ public void setBounds (Rectangle rect) {
33023302
checkWidget ();
33033303
if (rect == null) error (SWT.ERROR_NULL_ARGUMENT);
33043304
int zoom = computeBoundsZoom();
3305-
setBoundsInPixels(Win32DPIUtils.pointToPixel(rect, zoom));
3305+
Rectangle boundsInPixels = Win32DPIUtils.pointToPixel(rect, zoom);
3306+
fitInParentBounds(boundsInPixels, zoom);
3307+
setBoundsInPixels(boundsInPixels);
3308+
}
3309+
3310+
3311+
/**
3312+
* Cope with limited invertibility of pixel/point conversions.
3313+
* <p>
3314+
* Example: 125% monitor, layout fills composite with single child
3315+
* <ul>
3316+
* <li>Composite with client area of 527 pixels
3317+
* <li>getClientArea() returns 527 / 1,25 = 421,6 points
3318+
* <li>So layout sets rounded 422 points to child
3319+
* <li>This conforms to 422 * 1,25 = 527,5 pixels, which is rounded up to 528,
3320+
* i.e., one more than parent size
3321+
* </ul>
3322+
* Alternatives:
3323+
* <ul>
3324+
* <li>rounding down the client area instead could lead to areas not redrawn, as
3325+
* rounded down 421 points result in 526 pixels, one less than the actual size
3326+
* of the composite
3327+
* <li>rounding down the passed bounds leads to controls becoming unnecessarily
3328+
* smaller than their calculated size
3329+
* </ul>
3330+
* Thus, reduce the control size in case it would not fit anyway
3331+
*/
3332+
private void fitInParentBounds(Rectangle boundsInPixels, int zoom) {
3333+
if (parent == null) {
3334+
return;
3335+
}
3336+
Rectangle parentBounds = parent.getBoundsInPixels();
3337+
if (parentBounds.width < boundsInPixels.x + boundsInPixels.width
3338+
&& parentBounds.width >= boundsInPixels.x + boundsInPixels.width - Win32DPIUtils.pointToPixel(1.0f, zoom)) {
3339+
boundsInPixels.width = parentBounds.width - boundsInPixels.x;
3340+
}
3341+
if (parentBounds.height < boundsInPixels.y + boundsInPixels.height && parentBounds.height >= boundsInPixels.y
3342+
+ boundsInPixels.height - Win32DPIUtils.pointToPixel(1.0f, zoom)) {
3343+
boundsInPixels.height = parentBounds.height - boundsInPixels.y;
3344+
}
33063345
}
33073346

33083347
void setBoundsInPixels (Rectangle rect) {
@@ -3817,9 +3856,7 @@ public void setRegion (Region region) {
38173856
public void setSize (int width, int height) {
38183857
checkWidget ();
38193858
int zoom = computeBoundsZoom();
3820-
width = DPIUtil.pointToPixel(width, zoom);
3821-
height = DPIUtil.pointToPixel(height, zoom);
3822-
setSizeInPixels(width, height);
3859+
setSize(new Point(width, height), zoom);
38233860
}
38243861

38253862
void setSizeInPixels (int width, int height) {
@@ -3853,8 +3890,17 @@ void setSizeInPixels (int width, int height) {
38533890
public void setSize (Point size) {
38543891
checkWidget ();
38553892
if (size == null) error (SWT.ERROR_NULL_ARGUMENT);
3856-
size = Win32DPIUtils.pointToPixelAsSize(size, computeBoundsZoom());
3857-
setSizeInPixels(size.x, size.y);
3893+
int zoom = computeBoundsZoom();
3894+
setSize(size, zoom);
3895+
}
3896+
3897+
private void setSize(Point size, int zoom) {
3898+
Rectangle bounds = getBounds();
3899+
bounds.width = size.x;
3900+
bounds.height = size.y;
3901+
Rectangle boundsInPixels = Win32DPIUtils.pointToPixel(bounds, zoom);
3902+
fitInParentBounds(boundsInPixels, zoom);
3903+
setSizeInPixels(boundsInPixels.width, boundsInPixels.height);
38583904
}
38593905

38603906
@Override

0 commit comments

Comments
 (0)