On Tue, Nov 20, 2018 at 12:00 PM Nikolay Sivov nsivov@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.