Testbot D3D failures (Re: [PATCH 6/6] ddraw: Avoid passing invalid device type to wined3d.)

[Zebediah Figura]

On 4/5/19 4:04 AM, Marvin wrote:
> Hi,
> 
> While running your changed tests, I think I found new failures.
> Being a bot and all I'm not very good at pattern recognition, so I might be
> wrong, but could you please double-check?
> 
> Full results can be found at:
> https://testbot.winehq.org/JobDetails.pl?Key=50559
> 
> Your paranoid android.
> 
> 
> === debian9 (build log) ===
> 
> X Error of failed request:  BadValue (integer parameter out of range for operation)
>    Major opcode of failed request:  140 (RANDR)
>    Minor opcode of failed request:  21 (RRSetCrtcConfig)

This has finally annoyed me enough to look into it, and here is what I 
came up with:

 From reading the source to the X server [1] and doing some testing with 
a local setup identical enough to the TestBot, it appears that we 
receive a BadValue in the "x" parameter passed in to XRRSetCrtcConfig(). 
This function (sometimes) validates the passed-in offsets against the 
screen size, and this will of course fail if we try to increase the 
display resolution, as the d3d tests invariably do.

A more recent version of the qxl driver, or possibly the X server, or 
both (I rather lazily just made a new VM with Arch, which of course has 
more recent packages than Debian Stable) doesn't suffer from this 
failure. However, it doesn't quite change the display resolution either: 
it changes the device mode, but not the screen size.

 From reading the XRandR protocol spec [2], it seems like the problem is 
on our end. From RRSetCrtcConfig():

"'x' and 'y' contain the desired location within the screen for this 
monitor's content. 'x' and 'y' must be within the screen size, else a 
Value error results."

and

"The entire area of the CRTC must fit with the screen size, else a Match 
error results. As an example, rotating the screen so that a single CRTC 
fills the entire screen before and after may necessitate disabling the 
CRTC, resizing the screen, then re-enabling the CRTC at the new 
configuration to avoid an invalid intermediate configuration."

It seems the X.Org server implementation doesn't quite respect this: we 
would expect a BadMatch error here. But the point seems to remain: the 
spec states clearly that the screen must be large enough to contain the 
new CRTC size. So this suggests that the problem is not really with qxl, 
but rather on our end.

It seems that qxl is not the only driver affected by this: bug 33290 [3] 
describes a similar problem with NVIDIA binary drivers (those, at least, 
that actually report multiple modes, and so don't fall back to XRandR 
1.0 in Wine regardless). This was diagnosed there too, in comment 15, 
and a patch was submitted, which received the comment [4] "Updating the 
screen size isn't necessarily wrong, although avoiding panning is the 
wrong reason." I can only imagine, then, that "because the spec says so" 
is the right reason, but I may be misunderstanding or missing any number 
of things.

If there's any obvious way in which I've misanalyzed the problem, or am 
misreading the specification, please let me know, otherwise I think I'll 
try to work on an improved version of Gabriel Corona's patch that takes 
multi-head into account.

ἔρρωσθε,
Zeb

[1] https://cgit.freedesktop.org/xorg/xserver/tree/randr/rrcrtc.c#n1366
[2] https://www.x.org/releases/X11R7.7/doc/randrproto/randrproto.txt
[3] https://bugs.winehq.org/show_bug.cgi?id=33290
[4] https://www.winehq.org/pipermail/wine-devel/2016-July/113932.html