wined3d: Properly handle WS_VISIBLE and WS_EX_TOPMOST style flags on fullscreen windows.

Sam Edwards cfsworks at gmail.com
Sat Mar 30 17:50:09 CDT 2013


On 03/29/2013 06:16 AM, Henri Verbeet wrote:
> I think the idea is basically ok, but I do have some comments:
>
> I think test_window_style() is a more appropriate place for the tests,
> and you should add them to the variants in ddraw as well. Ddraw
> applications in particular are very fragile in this regard. Note that
> the test_window_style() tests are fairly similar to what your tests
> do, except that they don't test restoring the window as such. That
> test also shows that we're actually not supposed to touch the styles
> at all, but that's hard to make work without calling into winex11.

Ack, you're right! I saw the test_window_style() but second-guessed 
myself and put it into test_reset() instead... That was foolish of me. 
Thanks.

>> -    SetWindowPos(window, HWND_TOP, 0, 0, w, h, SWP_FRAMECHANGED | SWP_SHOWWINDOW | SWP_NOACTIVATE);
>> +    SetWindowPos(window, HWND_TOPMOST, 0, 0, w, h, SWP_FRAMECHANGED | SWP_SHOWWINDOW | SWP_NOACTIVATE);
> This is really a separate change.
I'm a bit confused, then: Without this change, the exstyle test breaks, 
and the guidelines request that every patch be atomic. They also request 
that all tests be included in the same patch.What is the proper 
procedure for this when it comes to multipatch patchsets?
Off the top of my head, I would guess it's one of these:
1. The "all tests in the same patch" rule is only appropriate for 
single-patch submissions, so in multipatch patchsets, it's acceptable to 
include a final patch with nothing but tests.
2. Only introduce the tests in the last patch of the patchset. (That is, 
add them all at once, but only when all of the necessary changes have 
been made to allow the tests to work first.)
3. Include each test in the first patch that will allow the test to 
pass. (That is, add them incrementally.)
4. Add the tests as todo_wine in the first patch, then remove the 
todo_wine in later patches as each test becomes functional. (That is, 
add them all at once as todo_wine, and mark them as non-todo incrementally.)

>> +    device->style ^= (device->style^style) & WS_VISIBLE;
> Spaces around the (second) ^, please.
Done. :)

Thank you,
Sam



More information about the wine-devel mailing list