Reduce scaling error when DPIUtil#scaleDown -> DPIUtil#scaleUp is used

[akoch-yatta]

Description
Using DPIUtil.scaleDown is used and the value is then scaled up with DPIUtil.scaleUp again, the result could be different than the original values. We need to investigate how to improve that

Reproduction
For instance take a value int size = 12 at 125% (in pixels)
In points, we say DPIUtil.scaleDown(12, 125) = 10;
Then back to pixels using DPIUtil.scaleUp(10, 125) = 13

Hence, on scaling down certain values and scaling back up again, for those zooms which are not factor or multiple of 100, we do not get the same value due to the underlying floating point rounding in the scaleUp and scaleDown method, which leads to errorful results.

Since, we strictly use integer, we can prove esily that in number theory,
a function like f(x)=round(a×x) where a= z/100 rational, is invertible on Z iff a is an integer or reciprocal of an integer (meaning the multiplication factor is exact over integers), otherwise rounding errors break invertibility. Hence, the DPIUtil worked fantastically with full scaling in the past as there were no remainders in the scalingFactor calulation.

Expected Behavior

on DPIUtil.scaleUp, we should get 12 back as the result.

Necessary configuration:

--

Additional knowledge

--

Follow-up work

• Adapt CTabFolder to utilize float precision for better scaling #361

[HeikoKlare]

Should we collect concrete idea of how to resolve the issue first? There is obviously no universal solution to the problem.

Some ideas:

• Try if we can make scaleDown() yield values with floating point precision, such that the consumer has to deal with rounding but may of course also use those higher-precision values for further calculation if possible (i.e., if not shared via API, embedded into Rectangle/Point etc.)
• Introduce some PixelAwarePoint/PixelAwareRectangle that store the original pixel values for the point values in addition to the monitor/zoom, so that when transforming the point values back into pixels they may be used instead if they are still fitting (i.e., if the point values have not been changed since creation) to avoid the loss of information during scaleDown/scaleUp.

Of course, none of them will universally solve the problem, but just like with the MonitorAwarePoint/MonitorAwarePixel we might be able to achieve as much preservation of information as necessary to properly support relevant cases?

[amartya4256]

Should we collect concrete idea of how to resolve the issue first? There is obviously no universal solution to the problem.

Some ideas:

• Try if we can make scaleDown() yield values with floating point precision, such that the consumer has to deal with rounding but may of course also use those higher-precision values for further calculation if possible (i.e., if not shared via API, embedded into Rectangle/Point etc.)
• Introduce some PixelAwarePoint/PixelAwareRectangle that store the original pixel values for the point values in addition to the monitor/zoom, so that when transforming the point values back into pixels they may be used instead if they are still fitting (i.e., if the point values have not been changed since creation) to avoid the loss of information during scaleDown/scaleUp.

Of course, none of them will universally solve the problem, but just like with the MonitorAwarePoint/MonitorAwarePixel we might be able to achieve as much preservation of information as necessary to properly support relevant cases?

For scaleDowns for points and rectangles, we can internally use the scaleDown for Float (which is already available in DPIUtil) inside the method and we can make the fields of rectangle float. Of course it will still work with int assignments but with behaviour of math.floor. We can have a small PoC to see if there is any adverse effect overall. What do you think?

Or extending to your second point, we can overload scaleDown for PixelAwarePoint and slowly start introducting it in relevant places. But we need to make sure that this is compatible with monitorAwareRectangles and points as well

[HeikoKlare]

and we can make the fields of rectangle float

Do you mean the actual fields of the Rectangle class`? Since that's public API, we cannot change it.

Doing a PoC for using float-based conversion where possible internal of SWT to see if it has positive effects sounds good.

[amartya4256]

I worked up a bit using Residuals as float. And it seems to work or atleast reduces the chances of error with scaling. Here's a limited framework around the concept:

class rectangle {
    int x, y, width, height;
    float resWidth, resHeight; // for noreference by the consumers
}

And then we use the concept of storing the residuals from the rounding and use the residuals to reconsile the absolute value later while scaling. Like this:

class DPIUtil {
/**
 * Returns a new rectangle as per the scaleFactor.
 */
public static Rectangle scaleBounds (Rectangle rect, int targetZoom, int currentZoom) {
	if (rect == null || targetZoom == currentZoom) return rect;
	float scaleFactor = ((float)targetZoom) / (float)currentZoom;
	Rectangle returnRect = new Rectangle (0,0,0,0);
	returnRect.x = Math.round (rect.x * scaleFactor);
	returnRect.y = Math.round (rect.y * scaleFactor);
	float absWidth = (rect.width + rect.resWidth) * scaleFactor;
	float absHeight = (rect.height + rect.resHeight) * scaleFactor;
	returnRect.width = Math.round (absWidth);
	returnRect.height = Math.round (absHeight);
	returnRect.resWidth = absWidth - returnRect.width;
	returnRect.resHeight = absHeight - returnRect.height;
	return returnRect;
}
}

I wrote a small test to compare this scaleBounds method with the old scaleUp and scaleDown method. And it works great in both directions:

class DPIUtilTests {

	@Test
	public void testRect() {
		Rectangle rect = new Rectangle(0, 0, 12, 12);
		Rectangle scaleDown = DPIUtil.scaleDown(rect, 125);
		Rectangle scaleUp = DPIUtil.scaleUp(scaleDown, 150);
		scaleDown = DPIUtil.scaleDown(scaleUp, 150);
		scaleUp = DPIUtil.scaleUp(scaleDown, 125);
		assertNotEquals(rect.width, scaleUp.width);
	}

	@Test
	public void testRectNew() {
		Rectangle rect = new Rectangle(0, 0, 12, 12);
		Rectangle scaleDown = DPIUtil.scaleBounds(rect, 100, 125);
		Rectangle scaleUp = DPIUtil.scaleBounds(scaleDown, 150, 100);
		scaleDown = DPIUtil.scaleBounds(scaleUp, 100, 150);
		scaleUp = DPIUtil.scaleBounds(scaleDown, 125, 100);
		assertEquals(rect.width, scaleUp.width);
	}
}

When is this solution not valid:
When you create a new rectangle from the old one using a constructor, the residuals are lost, leading to loss of information. But then our clone methods can be used to extend over this solution and make sure we always clone all the data.

Following this, the scaleUp and scaleDown can be changed to directly call scaleBounds with the right current and target zoom and we will have an invertible flow of scale up and scale down.

Note: This also works with adding magic numbers to fields, i.e. width = 2; residual is 0.2 and consumer calls width+= 5 -> the absolute value will still be 7.2 (2 + 5 + 0.2)

[HeikoKlare]

I guess the PR related this issue is:

• Use Float Aware Geometry for Scaling #62 eclipse-platform/eclipse.platform.swt#2200

isn't it?

That one has pending review comments/proposals, thus moving this item back to "in work".