[PATCH v4 1/4] comctl32/listbox: Move the item data insertion into a separate function

[Huw Davies]

On Tue, Feb 19, 2019 at 12:47:07PM +0200, Gabriel Ivăncescu wrote:
> On 2/19/19 12:14 PM, Huw Davies wrote:
> > On Mon, Feb 18, 2019 at 03:47:51PM +0200, Gabriel Ivăncescu wrote:
> > > Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
> > > ---
> > > 
> > > This is a no-op patch. nb_items is incremented before the call and this is
> > > taken care of in the helper function. This is for two reasons:
> > > 
> > > 1) This simplifies the nodata patch (only adds a single line to
> > > insert_item_data).
> > > 2) This matches what RemoveItem does, so it's more consistent.
> > > 
> > >   dlls/comctl32/listbox.c | 62 ++++++++++++++++++++++-------------------
> > >   1 file changed, 33 insertions(+), 29 deletions(-)
> > > 
> > > diff --git a/dlls/comctl32/listbox.c b/dlls/comctl32/listbox.c
> > > index 50ff5c5..330d3ae 100644
> > > --- a/dlls/comctl32/listbox.c
> > > +++ b/dlls/comctl32/listbox.c
> > > @@ -176,6 +176,38 @@ static void set_item_selected_state(LB_DESCR *descr, UINT index, BOOL state)
> > >           descr->items[index].selected = state;
> > >   }
> > > +static void insert_item_data(LB_DESCR *descr, INT index, WCHAR *str, ULONG_PTR data)
> > > +{
> > > +    LB_ITEMDATA *item;
> > > +
> > > +    item = &descr->items[index];
> > > +
> > > +    if (index < descr->nb_items - 1)
> > > +        RtlMoveMemory(item + 1, item,
> > > +                      (descr->nb_items - 1 - index) * sizeof(LB_ITEMDATA));
> > > +    item->str      = str;
> > > +    item->data     = HAS_STRINGS(descr) ? 0 : data;
> > > +    item->height   = 0;
> > > +    item->selected = FALSE;
> > > +
> > > +    /* 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 );
> > > +    }
> > 
> > Let's leave the OWNERDRAWVARIABLE bit in InsertItem.  insert_item_data() should
> > just be about manipulating the items array.  You'll want to add a set_item_height()
> > helper first.
> > 
> > Huw.
> > 
> 
> LBS_OWNERDRAWVARIABLE is incompatible with LBS_NODATA (and disabled when
> listbox is created, due to OWNERDRAWFIXED which is mandatory), should I
> still add a set_item_height helper? It's not really needed in this case.

Yes, I know this.

At the risk of repeating myself (but apparently I have to), the aim is
to get rid of direct accesses to item, outside of the item g/setters.

Huw.