Thanks for the review.
> They fail on my Windows XP box, r250 GPU, see the attached log file. It seems that It doesn't follow the alpha = 0xff behavior of newer GPUs and just returns alpha 0x0 in either case. The window positioning test needlessly depends on it. If I mask out the alpha channel in test_get_front_buffer_data_windowed_positioning() it passes.
Interesting. I'd be perfectly fine with dropping the separate alpha test entirely. I also meant to mask out the alpha but I guess I forgot it there or lost it during rebase/cleanup.
> if (foo) { something(); }
> Please put the { on a new line consistently.
Ah, I'm trying. I thought I got it right but I guess old habits die hard. I use the same line { in every other project, so not surprised that I slipped up a couple of times. I'll fix that.
> test_swapchain_buffer_swapping() and test_get_front_buffer_data_alpha() leak your initial device, the one you use to populate present_parameters
> In the case of test_swapchain_buffer_swapping I don't see why you need to call IDirect3D9::CreateDevice yourself - what prevents you from using the device returned by create_device?
Oops, that's a leftover of my original approach. I originally meant to create a new device for each test case and pass the required presentation params but later changed it to just use a single device created using `create_device` and reset that as needed. I meant to get rid of the second device that's created manually.
I will also look into using the current monitor resolution for the fullscreen tests.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/5500#note_67838
The tests pass on my Windows 11 machine with a Radeon 560 GPU.
They fail on my Windows XP box, r250 GPU, see the attached log file. It seems that It doesn't follow the alpha = 0xff behavior of newer GPUs and just returns alpha 0x0 in either case. The window positioning test needlessly depends on it. If I mask out the alpha channel in test_get_front_buffer_data_windowed_positioning() it passes.
I haven't looked closely at the code yet - I see some style inconsistencies, like
if (foo) {
something();
}
Please put the { on a new line consistently.
Rather than hardcoding the positions to check in the windowed mode tests please call GetWindowRect after creating the device and calculate the checked positions relative to it. There are window managers on the Linux side that don't allow applications to decide their placement and will break this kind of test. I suspect fvwm2 will cause you trouble otherwise.
test_swapchain_buffer_swapping() and test_get_front_buffer_data_alpha() leak your initial device, the one you use to populate present_parameters. The other unfortunate side effect of initializing present_parameters like that is that you switch the screen to 640x480 fullscreen mode. I recommend to use EnumDisplaySettingsW(ENUM_CURRENT_SETTINGS) to retrieve the current mode and use it to create a fullscreen device that doesn't change the mode. Changing the mode moves around Windows on Windows 10/11 and most Linux desktops.
In the case of test_swapchain_buffer_swapping I don't see why you need to call IDirect3D9::CreateDevice yourself - what prevents you from using the device returned by create_device?
[r200-xp.txt](/uploads/5f577583ec5703e2a040077de884faf3/r200-xp.txt)
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/5500#note_67826
To make https://gitlab.winehq.org/wine/wine/-/merge_requests/5422 simpler. This will also make it easier to write the mode list to the registry as a single binary blob, although it is a larger change that could be done later.
--
v3: win32u: Add all display device source modes at once.
win32u: Keep the primary current mode in the device manager context.
win32u: Remove fake source creation when adding display mode.
win32u: Close device manager source key in write_source_to_registry.
https://gitlab.winehq.org/wine/wine/-/merge_requests/5488