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