[PATCH v2 1/8] shell32/autocomplete: Remove flicker while redrawing the AutoComplete listbox
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
This series supersedes 150640, 150641, and 150646.
For some reason the continuous SetParent made it flicker when it changed its contents even with WM_SETREDRAW, so just set it once at listbox creation.
dlls/shell32/autocomplete.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 2153968..b5a053c 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -175,12 +175,15 @@ static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, enum autoappend_ if (len + 1 != size) text = heap_realloc(text, (len + 1) * sizeof(WCHAR));
- SendMessageW(ac->hwndListBox, LB_RESETCONTENT, 0, 0); - /* Set txtbackup to point to text itself (which must not be released) */ heap_free(ac->txtbackup); ac->txtbackup = text;
+ if (ac->options & ACO_AUTOSUGGEST) + { + SendMessageW(ac->hwndListBox, WM_SETREDRAW, FALSE, 0); + SendMessageW(ac->hwndListBox, LB_RESETCONTENT, 0, 0); + } IEnumString_Reset(ac->enumstr); for (cpt = 0;;) { @@ -220,12 +223,12 @@ static void autocomplete_text(IAutoCompleteImpl *ac, HWND hwnd, enum autoappend_ UINT height = SendMessageW(ac->hwndListBox, LB_GETITEMHEIGHT, 0, 0); SendMessageW(ac->hwndListBox, LB_CARETOFF, 0, 0); GetWindowRect(hwnd, &r); - SetParent(ac->hwndListBox, HWND_DESKTOP); /* It seems that Windows XP displays 7 lines at most and otherwise displays a vertical scroll bar */ SetWindowPos(ac->hwndListBox, HWND_TOP, r.left, r.bottom + 1, r.right - r.left, height * min(cpt + 1, 7), SWP_SHOWWINDOW ); + SendMessageW(ac->hwndListBox, WM_SETREDRAW, TRUE, 0); } else ShowWindow(ac->hwndListBox, SW_HIDE); @@ -438,6 +441,7 @@ static void create_listbox(IAutoCompleteImpl *This) if (This->hwndListBox) { This->wpOrigLBoxProc = (WNDPROC) SetWindowLongPtrW( This->hwndListBox, GWLP_WNDPROC, (LONG_PTR) ACLBoxSubclassProc); SetWindowLongPtrW( This->hwndListBox, GWLP_USERDATA, (LONG_PTR)This); + SetParent(This->hwndListBox, HWND_DESKTOP); } else This->options &= ~ACO_AUTOSUGGEST;
Clean the CreateWindowEx code of CW_USEDEFAULT since it's not valid for a child window and ends up being 0 anyway. Even more, it's useless as the actual size is given when the window is shown.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/shell32/autocomplete.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index b5a053c..fe5bba2 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -428,15 +428,10 @@ static LRESULT APIENTRY ACLBoxSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam,
static void create_listbox(IAutoCompleteImpl *This) { - HWND hwndParent; - - hwndParent = GetParent(This->hwndEdit); - /* FIXME : The listbox should be resizable with the mouse. WS_THICKFRAME looks ugly */ This->hwndListBox = CreateWindowExW(0, WC_LISTBOXW, NULL, WS_BORDER | WS_CHILD | WS_VSCROLL | LBS_HASSTRINGS | LBS_NOTIFY | LBS_NOINTEGRALHEIGHT, - CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, - hwndParent, NULL, shell32_hInstance, NULL ); + 0, 0, 0, 0, GetParent(This->hwndEdit), NULL, shell32_hInstance, NULL);
if (This->hwndListBox) { This->wpOrigLBoxProc = (WNDPROC) SetWindowLongPtrW( This->hwndListBox, GWLP_WNDPROC, (LONG_PTR) ACLBoxSubclassProc);
Signed-off-by: Huw Davies huw@codeweavers.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
This looks much nicer and less out of place.
dlls/shell32/autocomplete.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index fe5bba2..a681655 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -384,6 +384,10 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, ret = CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); autocomplete_text(This, hwnd, autoappend_flag_yes); return ret; + case WM_SETFONT: + if (This->hwndListBox) + SendMessageW(This->hwndListBox, WM_SETFONT, wParam, lParam); + break; case WM_DESTROY: { WNDPROC proc = This->wpOrigEditProc; @@ -434,9 +438,16 @@ static void create_listbox(IAutoCompleteImpl *This) 0, 0, 0, 0, GetParent(This->hwndEdit), NULL, shell32_hInstance, NULL);
if (This->hwndListBox) { + HFONT edit_font; + This->wpOrigLBoxProc = (WNDPROC) SetWindowLongPtrW( This->hwndListBox, GWLP_WNDPROC, (LONG_PTR) ACLBoxSubclassProc); SetWindowLongPtrW( This->hwndListBox, GWLP_USERDATA, (LONG_PTR)This); SetParent(This->hwndListBox, HWND_DESKTOP); + + /* Use the same font as the edit control, as it gets destroyed before it anyway */ + edit_font = (HFONT)CallWindowProcW(This->wpOrigEditProc, This->hwndEdit, WM_GETFONT, 0, 0); + if (edit_font) + SendMessageW(This->hwndListBox, WM_SETFONT, (WPARAM)edit_font, FALSE); } else This->options &= ~ACO_AUTOSUGGEST;
On Tue, Sep 25, 2018 at 02:55:29PM +0300, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
This looks much nicer and less out of place.
dlls/shell32/autocomplete.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index fe5bba2..a681655 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -384,6 +384,10 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, ret = CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam); autocomplete_text(This, hwnd, autoappend_flag_yes); return ret;
case WM_SETFONT:if (This->hwndListBox)SendMessageW(This->hwndListBox, WM_SETFONT, wParam, lParam);break; case WM_DESTROY: { WNDPROC proc = This->wpOrigEditProc;@@ -434,9 +438,16 @@ static void create_listbox(IAutoCompleteImpl *This) 0, 0, 0, 0, GetParent(This->hwndEdit), NULL, shell32_hInstance, NULL);
if (This->hwndListBox) {
HFONT edit_font;This->wpOrigLBoxProc = (WNDPROC) SetWindowLongPtrW( This->hwndListBox, GWLP_WNDPROC, (LONG_PTR) ACLBoxSubclassProc); SetWindowLongPtrW( This->hwndListBox, GWLP_USERDATA, (LONG_PTR)This); SetParent(This->hwndListBox, HWND_DESKTOP);/* Use the same font as the edit control, as it gets destroyed before it anyway */edit_font = (HFONT)CallWindowProcW(This->wpOrigEditProc, This->hwndEdit, WM_GETFONT, 0, 0);
Is there any reason you can't use SendMessage here?
if (edit_font)SendMessageW(This->hwndListBox, WM_SETFONT, (WPARAM)edit_font, FALSE);
On Wed, Sep 26, 2018 at 2:37 PM, Huw Davies huw@codeweavers.com wrote:
Is there any reason you can't use SendMessage here?
I don't remember. I think I wanted to get it despite an application subclassing the edit control, but it's more likely I just forgot to rebase this after the previous patch where I had replaced the SendMessage and i missed it. In either case if you think SendMessage is better, then I'll adjust it.
On Wed, Sep 26, 2018 at 02:42:20PM +0300, Gabriel Ivăncescu wrote:
On Wed, Sep 26, 2018 at 2:37 PM, Huw Davies huw@codeweavers.com wrote:
Is there any reason you can't use SendMessage here?
I don't remember. I think I wanted to get it despite an application subclassing the edit control, but it's more likely I just forgot to rebase this after the previous patch where I had replaced the SendMessage and i missed it. In either case if you think SendMessage is better, then I'll adjust it.
Yes, please use SendMessage.
Huw.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
This gets rid of annoying caret movement when replacing a selection that's not at the end (so it matches Windows).
dlls/shell32/autocomplete.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index a681655..243ccba 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -136,9 +136,15 @@ static size_t format_quick_complete(WCHAR *dst, const WCHAR *qc, const WCHAR *st
static void autoappend_str(IAutoCompleteImpl *ac, WCHAR *text, UINT len, WCHAR *str, HWND hwnd) { + DWORD sel_start; WCHAR *tmp; size_t size;
+ /* Don't auto-append unless the caret is at the end */ + SendMessageW(hwnd, EM_GETSEL, (WPARAM)&sel_start, 0); + if (sel_start != len) + return; + /* The character capitalization can be different, so merge text and str into a new string */ size = len + strlenW(&str[len]) + 1;
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
It's meant to be the same as on Windows; these keys always go to the visible bottom or top of the list, unless the selection is already there in which case they scroll by one page.
However, when wrapping around, they go through txtbackup just like the arrow keys. The logic for the arrow keys has not been changed.
PageDown/PageUp also do work with ACO_UPDOWNKEYDROPSLIST on Windows (they show the listbox).
dlls/shell32/autocomplete.c | 93 ++++++++++++++++++++++++++++++--------------- 1 file changed, 63 insertions(+), 30 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 243ccba..05648c7 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -134,6 +134,66 @@ static size_t format_quick_complete(WCHAR *dst, const WCHAR *qc, const WCHAR *st return dst - base; }
+static LRESULT change_selection(IAutoCompleteImpl *ac, HWND hwnd, UINT key) +{ + INT count = SendMessageW(ac->hwndListBox, LB_GETCOUNT, 0, 0); + INT sel = SendMessageW(ac->hwndListBox, LB_GETCURSEL, 0, 0); + if (key == VK_PRIOR || key == VK_NEXT) + { + if (sel < 0) + sel = (key == VK_PRIOR) ? count - 1 : 0; + else + { + INT base = SendMessageW(ac->hwndListBox, LB_GETTOPINDEX, 0, 0); + INT pgsz = SendMessageW(ac->hwndListBox, LB_GETLISTBOXINFO, 0, 0); + pgsz = max(pgsz - 1, 1); + if (key == VK_PRIOR) + { + if (sel == 0) + sel = -1; + else + { + if (sel == base) base -= min(base, pgsz); + sel = base; + } + } + else + { + if (sel == count - 1) + sel = -1; + else + { + base += pgsz; + if (sel >= base) base += pgsz; + sel = min(base, count - 1); + } + } + } + } + else if (key == VK_UP) + sel = ((sel - 1) < -1) ? count - 1 : sel - 1; + else + sel = ((sel + 1) >= count) ? -1 : sel + 1; + + SendMessageW(ac->hwndListBox, LB_SETCURSEL, sel, 0); + if (sel >= 0) + { + WCHAR *msg; + UINT len = SendMessageW(ac->hwndListBox, LB_GETTEXTLEN, sel, 0); + if (!(msg = heap_alloc((len + 1) * sizeof(WCHAR)))) + return 0; + len = SendMessageW(ac->hwndListBox, LB_GETTEXT, sel, (LPARAM)msg); + set_text_and_selection(ac, hwnd, msg, len, len); + heap_free(msg); + } + else + { + UINT len = strlenW(ac->txtbackup); + set_text_and_selection(ac, hwnd, ac->txtbackup, len, len); + } + return 0; +} + static void autoappend_str(IAutoCompleteImpl *ac, WCHAR *text, UINT len, WCHAR *str, HWND hwnd) { DWORD sel_start; @@ -287,6 +347,8 @@ static LRESULT ACEditSubclassProc_KeyDown(IAutoCompleteImpl *ac, HWND hwnd, UINT break; case VK_UP: case VK_DOWN: + case VK_PRIOR: + case VK_NEXT: /* Two cases here: - if the listbox is not visible and ACO_UPDOWNKEYDROPSLIST is set, display it with all the entries, without selecting any @@ -304,36 +366,7 @@ static LRESULT ACEditSubclassProc_KeyDown(IAutoCompleteImpl *ac, HWND hwnd, UINT } } else - { - INT count, sel; - count = SendMessageW(ac->hwndListBox, LB_GETCOUNT, 0, 0); - - /* Change the selection */ - sel = SendMessageW(ac->hwndListBox, LB_GETCURSEL, 0, 0); - if (wParam == VK_UP) - sel = ((sel - 1) < -1) ? count - 1 : sel - 1; - else - sel = ((sel + 1) >= count) ? -1 : sel + 1; - SendMessageW(ac->hwndListBox, LB_SETCURSEL, sel, 0); - if (sel >= 0) - { - WCHAR *msg; - UINT len; - - len = SendMessageW(ac->hwndListBox, LB_GETTEXTLEN, sel, 0); - if (!(msg = heap_alloc((len + 1) * sizeof(WCHAR)))) - return 0; - len = SendMessageW(ac->hwndListBox, LB_GETTEXT, sel, (LPARAM)msg); - set_text_and_selection(ac, hwnd, msg, len, len); - heap_free(msg); - } - else - { - UINT len = strlenW(ac->txtbackup); - set_text_and_selection(ac, hwnd, ac->txtbackup, len, len); - } - return 0; - } + return change_selection(ac, hwnd, wParam); break; case VK_DELETE: {
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
Minor patch split from the next patch in the series from where it helps more. options field only really helps slightly with x86_64 to reduce padding bytes.
dlls/shell32/autocomplete.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 05648c7..b3a6808 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -67,8 +67,9 @@ typedef struct IAutoComplete2 IAutoComplete2_iface; IAutoCompleteDropDown IAutoCompleteDropDown_iface; LONG ref; - BOOL initialized; - BOOL enabled; + BOOLEAN initialized; + BOOLEAN enabled; + AUTOCOMPLETEOPTIONS options; HWND hwndEdit; HWND hwndListBox; WNDPROC wpOrigEditProc; @@ -76,7 +77,6 @@ typedef struct WCHAR *txtbackup; WCHAR *quickComplete; IEnumString *enumstr; - AUTOCOMPLETEOPTIONS options; } IAutoCompleteImpl;
enum autoappend_flag
On Tue, Sep 25, 2018 at 02:55:32PM +0300, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
Minor patch split from the next patch in the series from where it helps more. options field only really helps slightly with x86_64 to reduce padding bytes.
dlls/shell32/autocomplete.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index 05648c7..b3a6808 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -67,8 +67,9 @@ typedef struct IAutoComplete2 IAutoComplete2_iface; IAutoCompleteDropDown IAutoCompleteDropDown_iface; LONG ref;
- BOOL initialized;
- BOOL enabled;
- BOOLEAN initialized;
- BOOLEAN enabled;
- AUTOCOMPLETEOPTIONS options; HWND hwndEdit; HWND hwndListBox; WNDPROC wpOrigEditProc;
@@ -76,7 +77,6 @@ typedef struct WCHAR *txtbackup; WCHAR *quickComplete; IEnumString *enumstr;
- AUTOCOMPLETEOPTIONS options;
} IAutoCompleteImpl;
I doubt all of this micro-optimization is really worth it (but please let's not get into a debate about it). If you're going to do this stuff then using a bitfield for initialized and enabled is probably what you want to do.
Huw.
On Wed, Sep 26, 2018 at 2:55 PM, Huw Davies huw@codeweavers.com wrote:
I doubt all of this micro-optimization is really worth it (but please let's not get into a debate about it). If you're going to do this stuff then using a bitfield for initialized and enabled is probably what you want to do.
Huw.
Originally I re-arranged them as part of the other patch to fit better (which added a UCHAR member), which also helps with future expansion, but since it was unrelated I decided to split it. So now it was split it is indeed minor but ok I'll use bitfields then. :-)
When selecting an item from the AutoComplete's listbox, the Return key should act the same as a left click on it (place the text, select it, and hide the listbox). This matches Windows behavior.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
Also fixes quickComplete since a VK_RETURN is sent as a WM_CHAR of '\n' when CTRL is pressed and it must not be forwarded in WM_CHAR. Furthermore, we have to process this in WM_KEYDOWN to ensure correct behavior.
The problem isn't the autocompletion here, but rather the edit control, which must not receive that character at all.
no_fwd_char is also needed for future patches (and even the next patch) to suppress forwarding the respective char from the KeyDown (for example, when ACO_USETAB will be implemented).
dlls/shell32/autocomplete.c | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index b3a6808..e2ae8aa 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -69,6 +69,7 @@ typedef struct LONG ref; BOOLEAN initialized; BOOLEAN enabled; + UCHAR no_fwd_char; AUTOCOMPLETEOPTIONS options; HWND hwndEdit; HWND hwndListBox; @@ -134,6 +135,34 @@ static size_t format_quick_complete(WCHAR *dst, const WCHAR *qc, const WCHAR *st return dst - base; }
+static BOOL select_item_with_return_key(IAutoCompleteImpl *ac, HWND hwnd) +{ + WCHAR *text; + HWND hwndListBox = ac->hwndListBox; + if (!(ac->options & ACO_AUTOSUGGEST)) + return FALSE; + + if (IsWindowVisible(hwndListBox)) + { + INT sel = SendMessageW(hwndListBox, LB_GETCURSEL, 0, 0); + if (sel >= 0) + { + UINT len = SendMessageW(hwndListBox, LB_GETTEXTLEN, sel, 0); + if ((text = heap_alloc((len + 1) * sizeof(WCHAR)))) + { + len = SendMessageW(hwndListBox, LB_GETTEXT, sel, (LPARAM)text); + set_text_and_selection(ac, hwnd, text, 0, len); + ShowWindow(hwndListBox, SW_HIDE); + ac->no_fwd_char = '\r'; /* RETURN char */ + heap_free(text); + return TRUE; + } + } + } + ShowWindow(hwndListBox, SW_HIDE); + return FALSE; +} + static LRESULT change_selection(IAutoCompleteImpl *ac, HWND hwnd, UINT key) { INT count = SendMessageW(ac->hwndListBox, LB_GETCOUNT, 0, 0); @@ -324,6 +353,8 @@ static LRESULT ACEditSubclassProc_KeyDown(IAutoCompleteImpl *ac, HWND hwnd, UINT WCHAR *text, *buf; size_t sz; UINT len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0); + ac->no_fwd_char = '\n'; /* CTRL+RETURN char */ + if (!(text = heap_alloc((len + 1) * sizeof(WCHAR)))) return 0; len = SendMessageW(hwnd, WM_GETTEXT, len + 1, (LPARAM)text); @@ -342,8 +373,8 @@ static LRESULT ACEditSubclassProc_KeyDown(IAutoCompleteImpl *ac, HWND hwnd, UINT return 0; }
- if (ac->options & ACO_AUTOSUGGEST) - ShowWindow(ac->hwndListBox, SW_HIDE); + if (select_item_with_return_key(ac, hwnd)) + return 0; break; case VK_UP: case VK_DOWN: @@ -375,6 +406,7 @@ static LRESULT ACEditSubclassProc_KeyDown(IAutoCompleteImpl *ac, HWND hwnd, UINT return ret; } } + ac->no_fwd_char = '\0'; return CallWindowProcW(ac->wpOrigEditProc, hwnd, uMsg, wParam, lParam); }
@@ -404,6 +436,9 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam, return ACEditSubclassProc_KeyDown(This, hwnd, uMsg, wParam, lParam); case WM_CHAR: case WM_UNICHAR: + if (wParam == This->no_fwd_char) return 0; + This->no_fwd_char = '\0'; + /* Don't autocomplete at all on most control characters */ if (iscntrlW(wParam) && !(wParam >= '\b' && wParam <= '\r')) break;
When the listbox is visible, ESC should hide it. Only when it's not visible should it be forwarded to the edit control. This matches Windows behavior.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
no_fwd_char is needed because we cannot send an ESC character in WM_CHAR to the edit control, which clears the text. We have to handle it in KeyDown though, just like VK_RETURN in previous patch.
dlls/shell32/autocomplete.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index e2ae8aa..f0b7f61 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -346,6 +346,15 @@ static LRESULT ACEditSubclassProc_KeyDown(IAutoCompleteImpl *ac, HWND hwnd, UINT { switch (wParam) { + case VK_ESCAPE: + /* When pressing ESC, Windows hides the auto-suggest listbox, if visible */ + if ((ac->options & ACO_AUTOSUGGEST) && IsWindowVisible(ac->hwndListBox)) + { + ShowWindow(ac->hwndListBox, SW_HIDE); + ac->no_fwd_char = 0x1B; /* ESC char */ + return 0; + } + break; case VK_RETURN: /* If quickComplete is set and control is pressed, replace the string */ if (ac->quickComplete && (GetKeyState(VK_CONTROL) & 0x8000))
-
Gabriel Ivăncescu -
Huw Davies