[PATCH 2/2] comctl32/listbox: Implement LBS_NODATA

[Gabriel Ivăncescu]

On Tue, Nov 20, 2018 at 12:00 PM Nikolay Sivov <nsivov at codeweavers.com> wrote:
>
> On 11/19/18 9:31 PM, Gabriel Ivăncescu wrote:
>
> > But don't we already have that? Right now, LBS_NODATA is implemented
> > using the normal item structure. It works, but it's super slow,
> > especially for single-selection listboxes (which should store no data
> > at all).
> >
> > I'm not sure what exactly you want me to implement? Can you please elaborate.
>
> Can we fix single-selection case first without major rewrite?That's what
> was reported as https://bugs.winehq.org/show_bug.cgi?id=32374.
>

I don't think that's possible, if by major rewrite you mean the
specialization of it everywhere, that can't be avoided.

Everything about LBS_NODATA has been implemented already with the
previous patchsets last week, *except* for performance (so it was left
at the end already, which is now). This style was designed for
performance by Microsoft, and applications make use of it for this
reason (as long as it exists, they expect performance back).

Note that the helper functions are mostly about multi-selection
listboxes, but I'm afraid they can't go away because otherwise this
patch would break on LBS_NODATA multi-selection listboxes, whereas
they worked before (but slowly).

A temporary alternative is to add even *more* checks to disable it for
multi-selection listboxes, so that only single-selection listboxes are
handled in this patch. However, I don't think that's a good idea or
elegant at all and would probably be considered a "hack" because it
will fill the code with arbitrary checks that shouldn't be there (and
would also go away on next patch), so I didn't bother with that but it
was my first thought, I considered many possibilities. :-/

> > That's not the job of LBS_NODATA. That's LBS_OWNERDRAWFIXED, which is
> > mandatory for LBS_NODATA (amongst other things) and is already tested,
> > though.
>
> Can we test this combination too?
>

Ah sure that can be done in test_ownerdraw in a loop, unless you are
referring to a different approach. The test can be done independently
anyway, it will work just fine, since LBS_NODATA's functionality is
all there right now, only missing part is performance, which happens
to be its entire purpose though.