November 2021 - Wine-Devel - list.winehq.org

Re: [PATCH resend 2/2] user32: Don't redraw the Combo Box when dropped down if it has an editbox.

by Marvin

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=101650 Your paranoid android. === debiant2 (64 bit WoW report) === user32: win.c:10437: Test failed: Expected foreground window 00000000000D0130, got 00000000018700D2 win.c:10439: Test failed: GetActiveWindow() = 0000000000000000 win.c:10439: Test failed: GetFocus() = 0000000000000000 win.c:10440: Test failed: Received WM_ACTIVATEAPP(1), did not expect it. win.c:10441: Test failed: Received WM_ACTIVATEAPP(0), did not expect it. win.c:10449: Test failed: Expected foreground window 00000000000D0130, got 0000000000000000 win.c:10451: Test failed: GetActiveWindow() = 0000000000000000 win.c:10451: Test failed: GetFocus() = 0000000000000000 win.c:10459: Test failed: Received WM_ACTIVATEAPP(1), did not expect it.

Nov. 10, 2021

[PATCH resend 2/2] user32: Set the selection if it's currently invalid in HandleTimer.

by Gabriel Ivăncescu

Don't skip MoveCaret if it would actually change the selection when it is invalid. This can happen, for example, in a combo box if the dropdown is shown by a mouse click + release followed by the mouse being moved into the dropped listbox, when the listbox has nothing selected. In this case, the item with the index zero would not be selected the first time the mouse moves over it, since the focus_item would be zero, despite the fact the item is not selected. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/user32/listbox.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c index 912adb7..1e8ca15 100644 --- a/dlls/user32/listbox.c +++ b/dlls/user32/listbox.c @@ -2348,7 +2348,7 @@ static LRESULT LISTBOX_HandleTimer( LB_DESCR *descr, INT index, TIMER_DIRECTION case LB_TIMER_NONE: break; } - if (index == descr->focus_item) return FALSE; + if (index == descr->focus_item && descr->selected_item != -1) return FALSE; LISTBOX_MoveCaret( descr, index, FALSE ); return TRUE; } -- 2.31.1

Nov. 10, 2021

[PATCH resend 1/2] comctl32: Set the selection if it's currently invalid in HandleTimer.

by Gabriel Ivăncescu

Don't skip MoveCaret if it would actually change the selection when it is invalid. This can happen, for example, in a combo box if the dropdown is shown by a mouse click + release followed by the mouse being moved into the dropped listbox, when the listbox has nothing selected. In this case, the item with the index zero would not be selected the first time the mouse moves over it, since the focus_item would be zero, despite the fact the item is not selected. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- This can be easily tested with winecfg. Launch winecfg, go to `Libraries` tab, click on the combobox arrow dropdown for `New override for library` to open the listbox, then hover the mouse over the first entry (acledit). Notice that it does not get highlighted, which is the issue. If you move the mouse to the second entry (aclui) and then back to (acledit), it works. With this patch, the first hover over the first entry will also work as it should. dlls/comctl32/listbox.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c index 1edb233..d89d49a 100644 --- a/dlls/comctl32/listbox.c +++ b/dlls/comctl32/listbox.c @@ -2339,7 +2339,7 @@ static LRESULT LISTBOX_HandleTimer( LB_DESCR *descr, INT index, TIMER_DIRECTION case LB_TIMER_NONE: break; } - if (index == descr->focus_item) return FALSE; + if (index == descr->focus_item && descr->selected_item != -1) return FALSE; LISTBOX_MoveCaret( descr, index, FALSE ); return TRUE; } -- 2.31.1

Nov. 10, 2021

[PATCH resend 2/2] user32: Don't redraw the Combo Box when dropped down if it has an editbox.

by Gabriel Ivăncescu

Some applications subclass the combo box and handle WM_ERASEBKGND themselves, without using WS_CLIPCHILDREN. This causes them to erase over the editbox child. There's no reason to redraw it in this case since the editbox is supposed to cover it, anyway. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=22260 Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/user32/combo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/user32/combo.c b/dlls/user32/combo.c index 1b53a2d..5eea89a 100644 --- a/dlls/user32/combo.c +++ b/dlls/user32/combo.c @@ -1002,7 +1002,7 @@ static void CBDropDown( LPHEADCOMBO lphc ) SWP_NOACTIVATE | SWP_SHOWWINDOW ); - if( !(lphc->wState & CBF_NOREDRAW) ) + if( !(lphc->wState & (CBF_NOREDRAW | CBF_EDIT)) ) RedrawWindow( lphc->self, NULL, 0, RDW_INVALIDATE | RDW_ERASE | RDW_UPDATENOW | RDW_NOCHILDREN ); -- 2.31.1

Nov. 10, 2021

[PATCH resend 1/2] comctl32: Don't redraw the Combo Box when dropped down if it has an editbox.

by Gabriel Ivăncescu

Some applications subclass the combo box and handle WM_ERASEBKGND themselves, without using WS_CLIPCHILDREN. This causes them to erase over the editbox child. There's no reason to redraw it in this case since the editbox is supposed to cover it, anyway. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=22260 Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/comctl32/combo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/comctl32/combo.c b/dlls/comctl32/combo.c index 4208ff8..4478e4c 100644 --- a/dlls/comctl32/combo.c +++ b/dlls/comctl32/combo.c @@ -1002,7 +1002,7 @@ static void CBDropDown( LPHEADCOMBO lphc ) SWP_NOACTIVATE | SWP_SHOWWINDOW ); - if( !(lphc->wState & CBF_NOREDRAW) ) + if( !(lphc->wState & (CBF_NOREDRAW | CBF_EDIT)) ) RedrawWindow( lphc->self, NULL, 0, RDW_INVALIDATE | RDW_ERASE | RDW_UPDATENOW | RDW_NOCHILDREN ); -- 2.31.1

Nov. 10, 2021

Re: [PATCH 4/5] win32u: Move null user driver implementation from user32.

by Rémi Bernon

On 11/10/21 15:01, Jacek Caban wrote: > On 11/10/21 12:47 AM, Rémi Bernon wrote: >> On 11/10/21 00:41, Rémi Bernon wrote: >>> On 11/9/21 13:55, Jacek Caban wrote: >>>> Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> >>>> --- >>>> dlls/user32/driver.c | 209 +---------------------------- >>>> dlls/win32u/driver.c | 310 >>>> ++++++++++++++++++++++++++++++++++++++++++- >>>> 2 files changed, 310 insertions(+), 209 deletions(-) >>>> >>>> >>> >>> A little bit related to my previous comment, this patch introduces a >>> reset to "null" graphics driver on driver unload. >>> >>> I think it could also add the same >>> >>> __wine_set_display_driver( &null_driver, WINE_GDI_DRIVER_VERSION ); >>> >>> call when the "null" driver is explicitly requested and loaded, >>> possibly addressing my previous comment too (as the pointer would not >>> be lasy_load_driver anymore). >> >> Also, regarding EnumDisplayMonitors and GetMonitorInfo, for which you >> added some FIXME, I have some patches to move the default >> implementation back to the corresponding user32 functions. >> >> I intended to send them after the previous nulldrv display device >> cache series I sent a week ago but I don't think they actually depend >> on it. >> >> (Similarly, I used the monitor cache to keep the default and initial >> monitor for GetMonitorInfo implementation) >> >> I rebased and pushed the patches there if you're interested, although >> only the commit up to c998fbadab6aa33e3bd2740b0651a0c0509d4c02 are >> really relevant for this: > > > I have null driver functions and some other sysparams.c functions moved > to win32u in my tree. Those needed to be changed due to to advapi32 and > setupapi calls. I used a cache as well, but instead of having two > caches, I extended adapters cache to contain missing monitor rects > informations. There are a few functions that need to enumerate monitors, > they would look like this: > > > update_display_cache(); > > pthread_mutex_lock( &display_lock ); > > LIST_FOR_EACH_ENTRY( adapter, &adapters, struct display_device, > entry ) > { > LIST_FOR_EACH_ENTRY( monitor, &adapter->children, struct > monitor, dev.entry ) > { > > /* .... */ > > } > } > > pthread_mutex_unlock( &display_lock ); > > > What do you think about such approach? > Do you actually need to move the display device cache to win32u or is it just because of the non-trivial nulldrv functions being moved there too? In the tree I linked above, in the end after all the changes, the nulldrv functions are all mostly no-op and the default implementation lives in user32, updating the cache by enumerating setupapi devices. It also moves several things out of graphics drivers, such as reading / writing current display mode, to the user32 EnumDisplaySettingsExW / ChangeDisplaySettingsEx generic code. Overall I think it makes everything much simpler, and reduce the amount of entry points graphics driver would need, and reduce the amount of changes needed if you need to replace all setupapi / advapi32 calls. Cheers, -- Rémi Bernon <rbernon(a)codeweavers.com>

Nov. 10, 2021

Re: [PATCH v2 5/5] win32u: Move NtUserCountClipboardFormats implementation from user32.

by Marvin

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=101648 Your paranoid android. === debiant2 (32 bit Chinese:China report) === user32: win.c:10418: Test failed: GetActiveWindow() = 00000000 win.c:10418: Test failed: GetFocus() = 00000000 win.c:10420: Test failed: Expected foreground window 000D0126, got 00C500D2 win.c:10423: Test failed: Received WM_ACTIVATEAPP(0), did not expect it. win.c:10430: Test failed: Expected foreground window 000D0126, got 00000000 win.c:10432: Test failed: GetActiveWindow() = 00000000 win.c:10432: Test failed: GetFocus() = 00000000 win.c:10440: Test failed: Received WM_ACTIVATEAPP(1), did not expect it.

Nov. 10, 2021

Re: [PATCH 4/5] win32u: Move null user driver implementation from user32.

by Jacek Caban

On 11/10/21 12:47 AM, Rémi Bernon wrote: > On 11/10/21 00:41, Rémi Bernon wrote: >> On 11/9/21 13:55, Jacek Caban wrote: >>> Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> >>> --- >>> dlls/user32/driver.c | 209 +---------------------------- >>> dlls/win32u/driver.c | 310 >>> ++++++++++++++++++++++++++++++++++++++++++- >>> 2 files changed, 310 insertions(+), 209 deletions(-) >>> >>> >> >> A little bit related to my previous comment, this patch introduces a >> reset to "null" graphics driver on driver unload. >> >> I think it could also add the same >> >> __wine_set_display_driver( &null_driver, WINE_GDI_DRIVER_VERSION ); >> >> call when the "null" driver is explicitly requested and loaded, >> possibly addressing my previous comment too (as the pointer would not >> be lasy_load_driver anymore). > > Also, regarding EnumDisplayMonitors and GetMonitorInfo, for which you > added some FIXME, I have some patches to move the default > implementation back to the corresponding user32 functions. > > I intended to send them after the previous nulldrv display device > cache series I sent a week ago but I don't think they actually depend > on it. > > (Similarly, I used the monitor cache to keep the default and initial > monitor for GetMonitorInfo implementation) > > I rebased and pushed the patches there if you're interested, although > only the commit up to c998fbadab6aa33e3bd2740b0651a0c0509d4c02 are > really relevant for this: I have null driver functions and some other sysparams.c functions moved to win32u in my tree. Those needed to be changed due to to advapi32 and setupapi calls. I used a cache as well, but instead of having two caches, I extended adapters cache to contain missing monitor rects informations. There are a few functions that need to enumerate monitors, they would look like this: update_display_cache(); pthread_mutex_lock( &display_lock ); LIST_FOR_EACH_ENTRY( adapter, &adapters, struct display_device, entry ) { LIST_FOR_EACH_ENTRY( monitor, &adapter->children, struct monitor, dev.entry ) { /* .... */ } } pthread_mutex_unlock( &display_lock ); What do you think about such approach? Thanks, Jacek

Nov. 10, 2021

Re: [PATCH vkd3d 2/5] vkd3d-shader/hlsl: Perform a copy propagation pass.

by Giovanni Mascellani

Hi, On 10/11/21 10:57, Giovanni Mascellani wrote: >> The CF part is probably going to be a lot trickier, but fortunately I >> think that this patch alone is the one that really matters. Frankly, >> if we were to take this patch, and then a second patch that does >> copy-prop on interior CF blocks with a fresh copy_propagation_state(), >> we might even cover enough that it's not even worrying about CF... > > Mmh, note that you have to handle control flow in some way or another, > you cannot just ignore it. This patch doesn't know about control flow, > but the only way it can do it is by bailing out at the first control > flow instruction in the program. It's not enough to ignore the control > flow block and resume processing after it, because the inner block might > invalidate some of your knowledge about the program state (in 3/5 patch > this is done by copy_propagation_invalidate_from_block). I think I misread your comment, even though my point is still valid: in your model, you have to refresh copy_propagation_state not only when you enter a block, but also when you leave it (actually, you might avoid refreshing when entering a conditional, though not when entering a loop). I think this means dropping quite a lot of useful information, and given that I don't find it particularly hard to avoid dropping it, it seems quite a waste. Notice, in particular, that if you count on this copy propagation pass to fix texture loading, then you need to ensure, at a minimum, that all loads from a variable that is stored once at the beginning of the program (before any control flow) are correctly recognized and simplified. This does not happen with your proposal, I believe, while it happens with mine 3/5 (which currently has a bug which makes it unsound, but I will fix it in my next submission). Well, technically at this point mine proposal wouldn't work as well, because it ignores object variables, but that can be addressed. Giovanni.

Nov. 10, 2021

Re: [PATCH v2 2/5] user32: Use user_driver_funcs to expose user driver function from drivers.

by Marvin

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=101645 Your paranoid android. === debiant2 (32 bit report) === user32: win.c:10437: Test failed: Expected foreground window 000D0126, got 00C500D2 win.c:10439: Test failed: GetActiveWindow() = 00000000 win.c:10439: Test failed: GetFocus() = 00000000 win.c:10440: Test failed: Received WM_ACTIVATEAPP(1), did not expect it. win.c:10441: Test failed: Received WM_ACTIVATEAPP(0), did not expect it. win.c:10449: Test failed: Expected foreground window 000D0126, got 00000000 win.c:10451: Test failed: GetActiveWindow() = 00000000 win.c:10451: Test failed: GetFocus() = 00000000 win.c:10459: Test failed: Received WM_ACTIVATEAPP(1), did not expect it.

Nov. 10, 2021