[Win32] Fix control bounds exceeding parent size

[HeikoKlare]

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.

This is an extract of:

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

which would, in its current state, introduce a regression that needs to be further analyzed.

How to reproduce

The different issues are demonstrated with the following snippet (conforms to the test cases and same as in #2782)):

public static void main(String[] args) {
	System.setProperty("swt.autoScale.updateOnRuntime", "true");
	Display display = new Display();
	Shell shell = new Shell(display);
	shell.setSize(200, 200);
	int width = 11;
	int height = 11;

	Composite c1 = new Composite(shell, SWT.NONE);
	c1.addPaintListener(e -> {
		e.gc.setBackground(display.getSystemColor(SWT.COLOR_BLUE));
		e.gc.fillRectangle(0, 0, c1.getSize().x + 5, c1.getSize().x + 5);
	});
	c1.setBounds(11, 9, width, height);
	Label l1 = new Label(c1, SWT.BORDER);
	l1.setBounds(0, 0, width, height);

	Composite c2 = new Composite(shell, SWT.NONE);
	c2.addPaintListener(e -> {
		e.gc.setBackground(display.getSystemColor(SWT.COLOR_BLUE));
		e.gc.fillRectangle(0, 0, c2.getSize().x + 5, c2.getSize().x + 5);
	});
	c2.setBounds(10, 30, width, height);
	Label l2 = new Label(c2, SWT.BORDER);
	l2.setBounds(0, 0, width, height);

	width += 2;
	height += 2;
	Composite c3 = new Composite(shell, SWT.NONE);
	c3.addPaintListener(e -> {
		e.gc.setBackground(display.getSystemColor(SWT.COLOR_BLUE));
		e.gc.fillRectangle(0, 0, c3.getSize().x + 5, c3.getSize().y + 5);
	});
	c3.setBounds(13, 53, width, height);
	Label l3 = new Label(c3, SWT.BORDER);
	l3.setBounds(0, 0, width, height);

	Composite c4 = new Composite(shell, SWT.NONE);
	c4.addPaintListener(e -> {
		e.gc.setBackground(display.getSystemColor(SWT.COLOR_BLUE));
		e.gc.fillRectangle(0, 0, c4.getSize().x + 5, c4.getSize().y + 5);
	});
	GridLayout layout = new GridLayout(2, true);
	layout.marginBottom = 0;
	layout.marginTop = 0;
	layout.marginLeft = 0;
	layout.marginRight = 0;
	layout.marginHeight= 0;
	layout.marginWidth= 0;
	layout.horizontalSpacing = 0;
	c4.setLayout(layout);
	c4.setBounds(13, 100, 101, 50);
	System.out.println(c4.getBounds());
	System.out.println(c4.getClientArea());
	Label l4a = new Label(c4, SWT.BORDER);
	l4a.setBackground(display.getSystemColor(SWT.COLOR_GREEN));
	l4a.setLayoutData(new GridData(GridData.FILL_BOTH));
	Label l4b = new Label(c4, SWT.BORDER);
	l4b.setBackground(display.getSystemColor(SWT.COLOR_GREEN));
	l4b.setLayoutData(new GridData(GridData.FILL_BOTH));
	c4.layout();

	shell.open();
	while (!shell.isDisposed()) {
		if (!display.readAndDispatch())
			display.sleep();
	}

	display.dispose();
}

All scenarios except for the one with third composite (becoming too small to fit its parent) are fixed with this change.

Before at 125%:

After at 125%:

Further examples

This change improves layouting behavior and can, e.g., be seen in the Forms UI, such as used in the Manifest editor.

Before at 125%:

After at 125%:

[github-actions]

Test Results

  147 files  +1    147 suites  +1   20m 53s ⏱️ + 3m 46s
4 673 tests +5  4 651 ✅ +5  22 💤 ±0  0 ❌ ±0 
  414 runs  +5    409 ✅ +5   5 💤 ±0  0 ❌ ±0 

Results for commit cbdcdcb. ± Comparison against base commit db8608f.

♻️ This comment has been updated with latest results.

[akoch-yatta]

There is no test that is ensuring, that making the child explicitly bigger, e.g. 2 pt that the parent is working, right? Wouldn't that make sense as some kind of sanity check?

[akoch-yatta]

I think a short comment about the if conditions would be helpful either here or in the javadoc above. what you are testing is that is would only be 1 point bigger - that should be stated somewhere here, I think

[HeikoKlare]

It would indeed help to have more explanation here. I have added a comment with a short explanation and also refactored the code to extract the conditions into variable, so that the variable names hopefully support comprehensibility of the semantics. Please have a look if you think that the changes help.

[HeikoKlare]

There is no test that is ensuring, that making the child explicitly bigger, e.g. 2 pt that the parent is working, right? Wouldn't that make sense as some kind of sanity check?

Valid point. I have extended the tests to cover this case. And I have added a unit test (setting bounds with defined values) for the fit-into-parent scenario in addition to the existing (rather integration-like) test case (setting bounds with values depending on the parent's client area). I also extended the tests to also use setSize() in addition to setBounds(), where I found another rounding issue that is also fixed with the latest commit.

[akoch-yatta]

LGTM, the explanation makes sense to limit the effect of the rounding issues. I tested with different configuration with the IDE and found the mentioned improvements and no regressions