New subject: [PATCH v2 2/2] user32/listbox: Implement LBS_NODATA for single-selection listboxes

20 Nov 2018

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=32374
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
---
I reduced the patch to just single-selection changes so that it still
works. After these are in, the multi-selection patches should be smaller.
Please note that the new helper functions (such as NODATA_is_sel) should
remain as-is because they will be extended later for multi-selection
listboxes, even if they seem redundant for now, so that I can really trim
down the size of the multi-selection patches. :-)
By the way, this patch really looks larger than it is. Most of the changes
are in SetCount and the helper functions. InsertItem and RemoveItem seem
large, but the changes are minor -- it's mostly just indentation, so it
should be simple to review.
The reason I changed the entirety of SetCount is to reduce the amount of
changes for the multi-selection patches that will come later, otherwise I'd
have to go back-and-forth with indentation changes unless I used a goto :-/
v2: Fix a dumb mistake when rounding the size.
dlls/comctl32/listbox.c | 280 ++++++++++++++++++++++++++++------------
 1 file changed, 201 insertions(+), 79 deletions(-)

diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c
index dbc7b4a..7ba6b1b 100644
--- a/dlls/comctl32/listbox.c
+++ b/dlls/comctl32/listbox.c
@@ -19,7 +19,7 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
  *
  * TODO:
- *    - LBS_NODATA
+ *    - LBS_NODATA for multi-selection listboxes
  */
#include <string.h>
@@ -125,6 +125,71 @@ static TIMER_DIRECTION LISTBOX_Timer = LB_TIMER_NONE;
static LRESULT LISTBOX_GetItemRect( const LB_DESCR *descr, INT index, RECT *rect );
+/***********************************************************************
+ *           Helper functions for LBS_NODATA listboxes
+ *
+ * Since LBS_NODATA listboxes can be extremely large, we need to store
+ * them with minimal overhead, both for performance and memory usage.
+ *
+ * In all cases, it is important to note that descr->items must never be
+ * dereferenced directly with LBS_NODATA, outside of these helpers.
+ *
+ * For single-selection listboxes, we store literally no data for items,
+ * and thus descr->items will always be NULL in this case.
+ *
+ * FIXME: Multi-selection listboxes are not implemented yet for LBS_NODATA.
+ *
+ */
+static BOOL is_singlesel_NODATA(const LB_DESCR *descr)
+{
+    return (descr->style & LBS_NODATA) &&
+           !(descr->style & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL));
+}
+
+static BOOL NODATA_is_sel(const LB_DESCR *descr, UINT index)
+{
+    return index == descr->selected_item;
+}
+
+static BOOL NODATA_multisel_expand(LB_DESCR *descr, UINT num)
+{
+    LB_ITEMDATA *p = descr->items;
+    UINT sz = num * sizeof(*p);
+
+    if (!p || sz > HeapSize(GetProcessHeap(), 0, p))
+    {
+        sz += LB_ARRAY_GRANULARITY * sizeof(*p) - 1;
+        sz -= sz % (LB_ARRAY_GRANULARITY * sizeof(*p));
+        if (!p) p = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sz);
+        else  p = HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, p, sz);
+        if (!p) return FALSE;
+        descr->items = p;
+    }
+    return TRUE;
+}
+
+static void NODATA_multisel_shrink(LB_DESCR *descr, UINT orig_num)
+{
+    LB_ITEMDATA *p = descr->items;
+    UINT sz = descr->nb_items * sizeof(*p);
+    UINT orig_sz = orig_num * sizeof(*p);
+
+    /* Shrink the array if needed */
+    if (sz + LB_ARRAY_GRANULARITY * sizeof(*p) * 2 < HeapSize(GetProcessHeap(), 0, p))
+    {
+        UINT rnd_sz = sz +  LB_ARRAY_GRANULARITY * sizeof(*p) - 1;
+        rnd_sz -= rnd_sz % (LB_ARRAY_GRANULARITY * sizeof(*p));
+        if ((p = HeapReAlloc(GetProcessHeap(), 0, p, rnd_sz)))
+        {
+            descr->items = p;
+            orig_sz = rnd_sz;
+        }
+    }
+    memset(&p[sz / sizeof(*p)], 0, orig_sz - sz);
+}
+
+
+
 /***********************************************************************
  *           LISTBOX_GetCurrentPageSize
  *
@@ -494,7 +559,7 @@ static void LISTBOX_PaintItem( LB_DESCR *descr, HDC hdc, const RECT *rect,
         RECT r;
         HRGN hrgn;
-	if (!item)
+	if (index >= descr->nb_items)
    {
        if (action == ODA_FOCUS)
    	DrawFocusRect( hdc, rect );
@@ -517,16 +582,19 @@ static void LISTBOX_PaintItem( LB_DESCR *descr, HDC hdc, const RECT *rect,
         dis.hDC          = hdc;
         dis.itemID       = index;
         dis.itemState    = 0;
-        if (item->selected) dis.itemState |= ODS_SELECTED;
+        if ( ( is_singlesel_NODATA(descr) && NODATA_is_sel(descr, index)) ||
+             (!is_singlesel_NODATA(descr) && item->selected) )
+            dis.itemState |= ODS_SELECTED;
+
         if (!ignoreFocus && (descr->focus_item == index) &&
             (descr->caret_on) &&
             (descr->in_focus)) dis.itemState |= ODS_FOCUS;
         if (!IsWindowEnabled(descr->self)) dis.itemState |= ODS_DISABLED;
-        dis.itemData     = item->data;
+        dis.itemData     = (descr->style & LBS_NODATA) ? 0 : item->data;
         dis.rcItem       = *rect;
         TRACE("[%p]: drawitem %d (%s) action=%02x state=%02x rect=%s\n",
-              descr->self, index, debugstr_w(item->str), action,
-              dis.itemState, wine_dbgstr_rect(rect) );
+              descr->self, index, debugstr_w((descr->style & LBS_NODATA) ? NULL : item->str),
+              action, dis.itemState, wine_dbgstr_rect(rect) );
         SendMessageW(descr->owner, WM_DRAWITEM, dis.CtlID, (LPARAM)&dis);
         SelectClipRgn( hdc, hrgn );
         if (hrgn) DeleteObject( hrgn );
@@ -673,6 +741,9 @@ static LRESULT LISTBOX_InitStorage( LB_DESCR *descr, INT nb_items )
 {
     LB_ITEMDATA *item;
+    if (is_singlesel_NODATA(descr))
+        return LB_OKAY;
+
     nb_items += LB_ARRAY_GRANULARITY - 1;
     nb_items -= (nb_items % LB_ARRAY_GRANULARITY);
     if (descr->items) {
@@ -1440,10 +1511,13 @@ static LRESULT LISTBOX_SetSelection( LB_DESCR *descr, INT index,
     {
         INT oldsel = descr->selected_item;
         if (index == oldsel) return LB_OKAY;
-        if (oldsel != -1) descr->items[oldsel].selected = FALSE;
-        if (index != -1) descr->items[index].selected = TRUE;
-        if (oldsel != -1) LISTBOX_RepaintItem( descr, oldsel, ODA_SELECT );
+        if (!(descr->style & LBS_NODATA))
+        {
+            if (oldsel != -1) descr->items[oldsel].selected = FALSE;
+            if (index != -1) descr->items[index].selected = TRUE;
+        }
         descr->selected_item = index;
+        if (oldsel != -1) LISTBOX_RepaintItem( descr, oldsel, ODA_SELECT );
         if (index != -1) LISTBOX_RepaintItem( descr, index, ODA_SELECT );
         if (send_notify && descr->nb_items) SEND_NOTIFICATION( descr,
                                (index != -1) ? LBN_SELCHANGE : LBN_SELCANCEL );
@@ -1516,54 +1590,59 @@ static LRESULT LISTBOX_InsertItem( LB_DESCR *descr, INT index,
if (index == -1) index = descr->nb_items;
     else if ((index < 0) || (index > descr->nb_items)) return LB_ERR;
-    if (!descr->items) max_items = 0;
-    else max_items = HeapSize( GetProcessHeap(), 0, descr->items ) / sizeof(*item);
-    if (descr->nb_items == max_items)
-    {
-        /* We need to grow the array */
-        max_items += LB_ARRAY_GRANULARITY;
-	if (descr->items)
-	    item = HeapReAlloc( GetProcessHeap(), 0, descr->items,
-                                  max_items * sizeof(LB_ITEMDATA) );
-	else
-	    item = HeapAlloc( GetProcessHeap(), 0,
-                                  max_items * sizeof(LB_ITEMDATA) );
-        if (!item)
+    if (is_singlesel_NODATA(descr))
+    {
+        descr->nb_items++;
+    }
+    else
+    {
+        if (!descr->items) max_items = 0;
+        else max_items = HeapSize( GetProcessHeap(), 0, descr->items ) / sizeof(*item);
+        if (descr->nb_items == max_items)
         {
-            SEND_NOTIFICATION( descr, LBN_ERRSPACE );
-            return LB_ERRSPACE;
+            /* We need to grow the array */
+            max_items += LB_ARRAY_GRANULARITY;
+            if (descr->items)
+                item = HeapReAlloc( GetProcessHeap(), 0, descr->items,
+                                      max_items * sizeof(LB_ITEMDATA) );
+            else
+                item = HeapAlloc( GetProcessHeap(), 0,
+                                      max_items * sizeof(LB_ITEMDATA) );
+            if (!item)
+            {
+                SEND_NOTIFICATION( descr, LBN_ERRSPACE );
+                return LB_ERRSPACE;
+            }
+            descr->items = item;
         }
-        descr->items = item;
-    }
-
-    /* Insert the item structure */
-
-    item = &descr->items[index];
-    if (index < descr->nb_items)
-        RtlMoveMemory( item + 1, item,
-                       (descr->nb_items - index) * sizeof(LB_ITEMDATA) );
-    item->str      = str;
-    item->data     = HAS_STRINGS(descr) ? 0 : data;
-    item->height   = 0;
-    item->selected = FALSE;
-    descr->nb_items++;
-    /* Get item height */
+        /* Insert the item structure */
+        item = &descr->items[index];
+        if (index < descr->nb_items)
+            RtlMoveMemory( item + 1, item,
+                           (descr->nb_items - index) * sizeof(LB_ITEMDATA) );
+        item->str      = str;
+        item->data     = HAS_STRINGS(descr) ? 0 : data;
+        item->height   = 0;
+        item->selected = FALSE;
+        descr->nb_items++;
-    if (descr->style & LBS_OWNERDRAWVARIABLE)
-    {
-        MEASUREITEMSTRUCT mis;
-        UINT id = (UINT)GetWindowLongPtrW( descr->self, GWLP_ID );
+        /* Get item height */
+        if (descr->style & LBS_OWNERDRAWVARIABLE)
+        {
+            MEASUREITEMSTRUCT mis;
+            UINT id = (UINT)GetWindowLongPtrW( descr->self, GWLP_ID );
-        mis.CtlType    = ODT_LISTBOX;
-        mis.CtlID      = id;
-        mis.itemID     = index;
-        mis.itemData   = data;
-        mis.itemHeight = descr->item_height;
-        SendMessageW( descr->owner, WM_MEASUREITEM, id, (LPARAM)&mis );
-        item->height = mis.itemHeight ? mis.itemHeight : 1;
-        TRACE("[%p]: measure item %d (%s) = %d\n",
-              descr->self, index, str ? debugstr_w(str) : "", item->height );
+            mis.CtlType    = ODT_LISTBOX;
+            mis.CtlID      = id;
+            mis.itemID     = index;
+            mis.itemData   = data;
+            mis.itemHeight = descr->item_height;
+            SendMessageW( descr->owner, WM_MEASUREITEM, id, (LPARAM)&mis );
+            item->height = mis.itemHeight ? mis.itemHeight : 1;
+            TRACE("[%p]: measure item %d (%s) = %d\n",
+                  descr->self, index, str ? debugstr_w(str) : "", item->height );
+        }
     }
/* Repaint the items */
@@ -1678,28 +1757,38 @@ static LRESULT LISTBOX_RemoveItem( LB_DESCR *descr, INT index )
     LISTBOX_InvalidateItems( descr, index );
descr->nb_items--;
-    LISTBOX_DeleteItem( descr, index );
-
-    if (!descr->nb_items) return LB_OKAY;
-
-    /* Remove the item */
+    if (is_singlesel_NODATA(descr))
+    {
+        if (!descr->nb_items)
+        {
+            SendMessageW(descr->self, LB_RESETCONTENT, 0, 0);
+            return LB_OKAY;
+        }
+    }
+    else
+    {
+        LISTBOX_DeleteItem( descr, index );
-    item = &descr->items[index];
-    if (index < descr->nb_items)
-        RtlMoveMemory( item, item + 1,
-                       (descr->nb_items - index) * sizeof(LB_ITEMDATA) );
-    if (descr->anchor_item == descr->nb_items) descr->anchor_item--;
+        if (!descr->nb_items) return LB_OKAY;
-    /* Shrink the item array if possible */
+        /* Remove the item */
+        item = &descr->items[index];
+        if (index < descr->nb_items)
+            RtlMoveMemory( item, item + 1,
+                           (descr->nb_items - index) * sizeof(LB_ITEMDATA) );
-    max_items = HeapSize( GetProcessHeap(), 0, descr->items ) / sizeof(LB_ITEMDATA);
-    if (descr->nb_items < max_items - 2*LB_ARRAY_GRANULARITY)
-    {
-        max_items -= LB_ARRAY_GRANULARITY;
-        item = HeapReAlloc( GetProcessHeap(), 0, descr->items,
-                            max_items * sizeof(LB_ITEMDATA) );
-        if (item) descr->items = item;
+        /* Shrink the item array if possible */
+        max_items = HeapSize( GetProcessHeap(), 0, descr->items ) / sizeof(LB_ITEMDATA);
+        if (descr->nb_items < max_items - 2*LB_ARRAY_GRANULARITY)
+        {
+            max_items -= LB_ARRAY_GRANULARITY;
+            item = HeapReAlloc( GetProcessHeap(), 0, descr->items,
+                                max_items * sizeof(LB_ITEMDATA) );
+            if (item) descr->items = item;
+        }
     }
+    descr->anchor_item = min(descr->anchor_item, descr->nb_items - 1);
+
     /* Repaint the items */
LISTBOX_UpdateScroll( descr );
@@ -1737,7 +1826,8 @@ static void LISTBOX_ResetContent( LB_DESCR *descr )
 {
     INT i;
-    for(i = descr->nb_items - 1; i>=0; i--) LISTBOX_DeleteItem( descr, i);
+    if (!is_singlesel_NODATA(descr))
+        for (i = descr->nb_items - 1; i >= 0; i--) LISTBOX_DeleteItem(descr, i);
     HeapFree( GetProcessHeap(), 0, descr->items );
     descr->nb_items      = 0;
     descr->top_item      = 0;
@@ -1753,22 +1843,52 @@ static void LISTBOX_ResetContent( LB_DESCR *descr )
  */
 static LRESULT LISTBOX_SetCount( LB_DESCR *descr, INT count )
 {
-    LRESULT ret;
+    INT orig_num;
if (!(descr->style & LBS_NODATA)) return LB_ERR;
+    count = max(count, 0);
-    /* FIXME: this is far from optimal... */
     if (count > descr->nb_items)
     {
-        while (count > descr->nb_items)
-            if ((ret = LISTBOX_InsertString( descr, -1, 0 )) < 0)
-                return ret;
+        if ((descr->style & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL)) &&
+            !NODATA_multisel_expand(descr, count))
+        {
+            SEND_NOTIFICATION(descr, LBN_ERRSPACE);
+            return LB_ERRSPACE;
+        }
+        orig_num = descr->nb_items;
+        descr->nb_items = count;
+
+        LISTBOX_UpdateScroll(descr);
+        LISTBOX_InvalidateItems(descr, orig_num);
+
+        /* If listbox was empty, set focus to the first item */
+        if (orig_num == 0) LISTBOX_SetCaretIndex(descr, 0, FALSE);
     }
     else if (count < descr->nb_items)
     {
-        while (count < descr->nb_items)
-            if ((ret = LISTBOX_RemoveItem( descr, (descr->nb_items - 1) )) < 0)
-                return ret;
+        LISTBOX_InvalidateItems(descr, count);
+        orig_num = descr->nb_items;
+        descr->nb_items = count;
+
+        if (count == 0) SendMessageW(descr->self, LB_RESETCONTENT, 0, 0);
+        else
+        {
+            descr->anchor_item = min(descr->anchor_item, count - 1);
+
+            if (descr->style & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL))
+                NODATA_multisel_shrink(descr, orig_num);
+            else if (descr->selected_item >= descr->nb_items)
+                descr->selected_item = -1;
+
+            LISTBOX_UpdateScroll(descr);
+
+            /* If we removed the scrollbar, reset the top of the list */
+            if (descr->nb_items <= descr->page_size && orig_num > descr->page_size)
+                LISTBOX_SetTopItem(descr, 0, TRUE);
+
+            descr->focus_item = min(descr->focus_item, descr->nb_items - 1);
+        }
     }
InvalidateRect( descr->self, NULL, TRUE );
@@ -2763,6 +2883,8 @@ static LRESULT CALLBACK LISTBOX_WindowProc( HWND hwnd, UINT msg, WPARAM wParam,
     case LB_GETSEL:
         if (((INT)wParam < 0) || ((INT)wParam >= descr->nb_items))
             return LB_ERR;
+        if (is_singlesel_NODATA(descr))
+            return NODATA_is_sel(descr, wParam);
         return descr->items[wParam].selected;
case LB_SETSEL:
-- 
2.19.1



    

[PATCH v2 1/2] comctl32/listbox: Implement LBS_NODATA for single-selection listboxes