[PATCH 2/4] comctl32/listbox: Implement LBS_NODATA for multi-selection listboxes

[Huw Davies]

On Fri, Feb 22, 2019 at 02:00:57PM +0200, Gabriel Ivăncescu wrote:
> Use a byte array to store selection state of items, since we don't need any
> other data for LBS_NODATA multi-selection listboxes. This improves memory
> usage dramatically for large lists, and performance boosts are nice too.
> 
> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
> ---
> 
> For now, this should be good enough (byte array). Unfortunately, using
> other storage representations in the future (like selection ranges, or bit
> arrays) would probably need the avoided NODATA helpers to split the functions
> (in order to be easier maintainable), so for now this will suffice.
> 
>  dlls/comctl32/listbox.c | 48 +++++++++++++++++++++++++++++++++++------
>  1 file changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c
> index b6024c3..5afdf5c 100644
> --- a/dlls/comctl32/listbox.c
> +++ b/dlls/comctl32/listbox.c
> @@ -18,8 +18,6 @@
>   * License along with this library; if not, write to the Free Software
>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
>   *
> - * TODO:
> - *    - LBS_NODATA for multi-selection listboxes
>   */
>  
>  #include <string.h>
> @@ -127,6 +125,15 @@ static TIMER_DIRECTION LISTBOX_Timer = LB_TIMER_NONE;
>  
>  static LRESULT LISTBOX_GetItemRect( const LB_DESCR *descr, INT index, RECT *rect );
>  
> +/*
> +   Resize the item storage array if needed.
> +
> +   For single-selection LBS_NODATA listboxes, no storage is allocated,
> +   and thus descr->items will always be NULL.
> +
> +   For multi-selection LBS_NODATA listboxes, one byte per item is stored
> +   for the item's selection state, instead of the usual LB_ITEMDATA.
> +*/
>  static BOOL resize_storage(LB_DESCR *descr, UINT items_size)
>  {
>      LB_ITEMDATA *items;
> @@ -137,7 +144,10 @@ static BOOL resize_storage(LB_DESCR *descr, UINT items_size)
>          items_size = (items_size + LB_ARRAY_GRANULARITY - 1) & ~(LB_ARRAY_GRANULARITY - 1);
>          if ((descr->style & (LBS_NODATA | LBS_MULTIPLESEL | LBS_EXTENDEDSEL)) != LBS_NODATA)
>          {
> -            items = heap_realloc(descr->items, items_size * sizeof(LB_ITEMDATA));
> +            size_t size = items_size;
> +            if (!(descr->style & LBS_NODATA)) size *= sizeof(LB_ITEMDATA);
> +
> +            items = heap_realloc(descr->items, size);
>              if (!items)
>              {
>                  SEND_NOTIFICATION(descr, LBN_ERRSPACE);
> @@ -150,8 +160,7 @@ static BOOL resize_storage(LB_DESCR *descr, UINT items_size)
>  
>      if ((descr->style & LBS_NODATA) && descr->items && items_size > descr->nb_items)
>      {
> -        memset(&descr->items[descr->nb_items], 0,
> -               (items_size - descr->nb_items) * sizeof(LB_ITEMDATA));
> +        memset((BYTE*)descr->items + descr->nb_items, 0, items_size - descr->nb_items);
>      }
>      return TRUE;
>  }
> @@ -180,13 +189,21 @@ static BOOL is_item_selected( const LB_DESCR *descr, UINT index )
>  {
>      if (!(descr->style & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL)))
>          return index == descr->selected_item;
> -    return descr->items[index].selected;
> +    if (descr->style & LBS_NODATA)
> +        return ((BYTE*)descr->items)[index];
> +    else
> +        return descr->items[index].selected;

Make descr->items a union of the two types, that will get rid of these
horrible casts.  If you find you need to change a lot of code to make
this work, then you need to add more calls to item getter/setter helpers.

>  }
>  
>  static void set_item_selected_state(LB_DESCR *descr, UINT index, BOOL state)
>  {
>      if (descr->style & (LBS_MULTIPLESEL | LBS_EXTENDEDSEL))
> -        descr->items[index].selected = state;
> +    {
> +        if (descr->style & LBS_NODATA)
> +            ((BYTE*)descr->items)[index] = state;
> +        else
> +            descr->items[index].selected = state;
> +    }
>  }
>  
>  static void insert_item_data(LB_DESCR *descr, UINT index, WCHAR *str, ULONG_PTR data)
> @@ -194,6 +211,15 @@ static void insert_item_data(LB_DESCR *descr, UINT index, WCHAR *str, ULONG_PTR
>      LB_ITEMDATA *item;
>  
>      if (!descr->items) return;
> +    if (descr->style & LBS_NODATA)
> +    {
> +        BYTE *item = (BYTE*)descr->items + index;
> +
> +        if (index < descr->nb_items)
> +            memmove(item + 1, item, descr->nb_items - index);
> +        *item = FALSE;
> +        return;
> +    }

This is essentially the same code as what follows for the !LBS_NODATA
case.  If you added a helper to return the size of an item then these
could be merged.  This helper could also be used in resize_storage().

Huw.