Off-by-one scaling error

[mark_k]

WinUAE 3.4.0 on Windows 10 64-bit 1703. AMD FirePro W5100 graphics card.

The attached config uses Direct3D, null filter, "No scaling", Horiz. size and Vert. size both set to 2x. The image looks like it's being scaled to one (Windows desktop) pixel shorter than it should be.

[rsn8887]

I wonder if this is related to my problems getting a perfect user-made "null filter" to work with exact 1:1 pixels.

[mark_k]

Hmm... I tried using the -nod3d9shader option, which means WinUAE doesn't use its built-in winuae.fx shader. No change.

Then I set Integer scaling, and made the window large enough so the image was scaled 2×. The problem went away then.

Edit to add: Maybe there's an incorrectly-rounded floating point to integer conversion in the no scaling (with 2x H&V size) code path, but not in the integer scaling one?

[Toni Wilen]

There most likely is something but I have no idea where and what...

[mark_k]

I'll have a look through the code but I'm sure nothing will jump out.

This problem isn't driver-specific, though perhaps the manifestation could differ between drivers. I checked with my laptop (Nvidia driver). Setting h. and v. size controls to 2×, the issue was still present. At varying window sizes there were sometimes both horizontal and vertical "glitch lines". Same for 3×.

Attached pics taken on my laptop.

[mark_k]

I'm probablyalmost certainly barking up the wrong tree here, but...

extraw and extrah are of type int

win32_scaler.cpp:95

static void sizeoffset (RECT *dr, RECT *zr, int w, int h)
{
	dr->right -= w;
	dr->bottom -= h;
	OffsetRect (zr, w / 2, h / 2);
}

win32_scaler.cpp:164

float filter_horiz_zoom_mult = currprefs.gf[picasso_on].gfx_filter_horiz_zoom_mult;
float filter_vert_zoom_mult = currprefs.gf[picasso_on].gfx_filter_vert_zoom_mult;

win32_scaler.cpp:188 near start of getfilterrect2():

extraw = -aws * (filter_horiz_zoom - currprefs.gf[picasso_on].gfx_filteroverlay_overscan * 10) / 2.0f;
extrah = -ahs * (filter_vert_zoom - currprefs.gf[picasso_on].gfx_filteroverlay_overscan * 10) / 2.0f;

Later on in an if (scalemode == AUTOSCALE_INTEGER || scalemode == AUTOSCALE_INTEGER_AUTOSCALE) {...} statement, you have (line 309):

	extraw = 0;
	extrah = 0;

After that if statement's closing brace, there is this (line 571):

cont:
if (!filter_horiz_zoom_mult && !filter_vert_zoom_mult) {
	sizeoffset (dr, zr, extraw, extrah);
	...

So in the case where scalemode is 0 (AUTOSCALE_NONE), extraw and extrah are set by the lines near 188.

Maybe worth adding some logging there???

Also, how are conditions like if (!filter_horiz_zoom_mult && !filter_vert_zoom_mult) evaluated? Because filter_horiz_zoom_mult and filter_vert_zoom_mult are of type float, should you really check whether they are (very) small enough rather than (effectively) compare with zero?

[Toni Wilen]

Maybe worth adding some logging there???

No. I don't add logging that repeats every frame.

Get Visual Studio community edition and start debugging

Also, how are conditions like if (!filter_horiz_zoom_mult && !filter_vert_zoom_mult) evaluated? Because filter_horiz_zoom_mult and filter_vert_zoom_mult are of type float, should you really check whether they are (very) small enough rather than (effectively) compare with zero?

It is working as designed, value is specifically set to zero when it needs to be zero.

[mark_k]

Some more info, maybe this could help? Screenmode is HIGHGFX:1024×768 interlaced. This is with AMD W5100 graphics card/driver.

The missing horizontal line seems to occur only when the window height (as shown in the Windowed numerical box in the screen section of the display settings page) is odd.

I'll check with Nvidia driver and report back. (Since there were in some cases two missing horizontal and two vertical lines.)

[mark_k]

Here are some notes on code in win32_scaler.cpp. If/when I ever finish getting WinUAE into a buildable state I can try this change myself...

	OffsetRect (zr, (int)(-filter_horiz_offset * aws), 0);	// filter_horiz_offset is float, aws is int
	OffsetRect (zr, 0, (int)(-filter_vert_offset * ahs));	// filter_vert_offset is float, ahs is int

	xs = dst_width;				// Both int
	if (xmult)				// xmult is float
		xs -= dst_width / xmult;	// int -= int / float;
	ys = dst_height;			// Both int
	if (ymult)				// ymult is float
		ys -= dst_height / ymult;	// int -= int / float;
	sizeoffset (dr, zr, xs, ys);

	filterxmult = xmult;			// Both float
	filterymult = ymult;			// Both float
	filteroffsetx += (dst_width - aw / filterxmult) / 2;	// float += (int - int / float) / 2;
	filteroffsety += (dst_height - ah / filterymult) / 2;	// float += (int - int / float) / 2;

end:

	if (getscalerect (&mrmx, &mrmy, &mrsx, &mrsy)) {
		sizeoffset (dr, zr, mrmx, mrmy);	// 3rd and 4th args are int but mrsx & mrsy are float
		OffsetRect (dr, mrsx, mrsy);		// 2nd and 3rd args are int but mrsx & mrsy are float
	}

To account for truncation when converting float to it, could/should some or all of these be changed, like this:

// NOTE: I'm just adding 0.5 here but you might need to properly
// round when a float can be either positive or negative

	OffsetRect (zr, (int)(-filter_horiz_offset * aws + 0.5), 0);	// Probably need to round here?
	OffsetRect (zr, 0, (int)(-filter_vert_offset * ahs + 0.5));	// Probably need to round here?

	xs = dst_width;
	if (xmult)
		xs -= dst_width / xmult + 0.5;
	ys = dst_height;
	if (ymult)
		ys -= dst_height / ymult + 0.5;
	sizeoffset (dr, zr, xs, ys);
	...
end:

	if (getscalerect (&mrmx, &mrmy, &mrsx, &mrsy)) {
		sizeoffset (dr, zr, mrmx + 0.5, mrmy + 0.5);
		OffsetRect (dr, mrsx + 0.5, mrsy + 0.5);
	}

getscalerect() just returns false if mask2texture is 0. [So is that statement relevant to this off-by-one issue?]

About sizeoffset()...
Suppose you call sizeoffset() with mrmx = 3.9 say. Then 3.9 (float) is truncated to 3 (int) for the function call.
Then the "int w" argument to sizeoffset() is 3, which means sizeoffset() calls OffsetRect() with 2nd arg 1.

static void sizeoffset (RECT *dr, RECT *zr, int w, int h)
{
	dr->right -= w;
	dr->bottom -= h;
	OffsetRect (zr, w / 2, h / 2);
}

[Toni Wilen]

Above changes done. If no replies or side-effects: it gets removed in next beta, it is too late for random changes at this point.

[DamienD]

Originally Posted by Toni Wilen

it is too late for random changes at this point.

Awesome; I read the above meaning the 3.4.1 release is coming shortly

[mark_k]

Very quick test with the config I used for the first post.

Window size	341b8			latest winuae.7z
2170×1625	1 column missing	1 row + 1 column missing
2170×1626	good			2 rows + 1 column missing
2170×1627	1 column missing	1 row + 1 column missing
2170×1628	good			2 rows + 1 column missing
2170×1629	1 column missing	1 row + 1 column missing
2170×1630	good			2 rows + 1 column missing

So the changes had some effect, but not a beneficial one. Maybe some rounding in the wrong direction??? Maybe if I can get to understand what the code is trying to do, that will make things clearer.

[Toni Wilen]

As I said: too late now.