[PATCH] comctl32/treeview: Make textWidth in struct _TREEITEM compatible with native one.

[Eric Zhang]

>
> +· /* padding0 should be unused on Wine */
> +· if(!strcmp(winetest_platform, "wine"))
> +· {
> +· ok_(__FILE__, line)(data->padding0 == 0, "padding0 %04x, got %04x\n",
> 0, data->padding0);
> +· ok_(__FILE__, line)(data->textWidth == textWidth, "textWidth %04x, got
> %04x\n", textWidth, data->textWidth);
> +· }
> +· /* TODO: textWidth computation on Windows is alway that of Wine plus 4,
> remove this test when it got corrected.
> +· Also, in Windows textWidth is calculated even if tree item is invisible
> whereas in Wine is postponed */
> +· else
> +· {
> +· ok_(__FILE__, line)(textWidth == 0 || data->textWidth == 0 ||
> data->textWidth == textWidth + 4,
> +· "textWidth %04x, got %04x\n", textWidth, data->textWidth);
> +· }
>
+ This is very dirty. You should get rid of platform check; testing
padding0 does not look useful; and last test line looks suspicious, if
expected width is 0 nothing gets tested.

You're right, that what I later found out what's wrong with the tests.
Thanks for the review.

On Sun, Jan 21, 2018 at 12:45 AM, Nikolay Sivov <nsivov at codeweavers.com>
wrote:

> On 01/20/2018 08:40 AM, Zhiyi Zhang wrote:
>
> Hi Nikolay,
>>
>> This fixes https://bugs.winehq.org/show_bug.cgi?id=35127
>>
>> As I was working through bug 35127, I noticed that you had once changed
>> the layout of struct _TREEITEM in commit 6ec621e835e03f715d5fee27c5de5b
>> 0d361814de.
>>
>> Now I am sure that textWidth in struct _TREEITEM is a WORD and is at the
>> offset 0x1a. You can check out the bug comments for details. I tried to
>> find out what is at 0x18 but no luck, thus I am adding a unknown member to
>> make textWidth at the right offset.
>>
>>  What do you think of the patch? Do you know what member is at struct
>> _TREEITEM+0x18? I can use your opinion with this patch.
>>
>>
>>
>> struct·_ITEM_DATA
>> {
>> -· HTREEITEM parent; /* for root value of parent field is unidetified */
>> +· HTREEITEM parent; /* for root value of parent field is unidentified */
>> · HTREEITEM nextsibling;
>> · HTREEITEM firstchild;
>> +· UINT unknownField0; /* untested */
>> +· UINT unknownField1; /* untested */
>> +· UINT unknownField2; /* untested */
>> +· WORD padding0; /* unknown field to make textWidth offset compatible
>> with the native one */
>> +· WORD textWidth; /* horizontal text extent for pszText */
>> };
>>
> They are all unknown, so a single unknown[7] is better, assuming this will
> work on 64 bits.
>
> +· /* padding0 should be unused on Wine */
>> +· if(!strcmp(winetest_platform, "wine"))
>> +· {
>> +· ok_(__FILE__, line)(data->padding0 == 0, "padding0 %04x, got %04x\n",
>> 0, data->padding0);
>> +· ok_(__FILE__, line)(data->textWidth == textWidth, "textWidth %04x, got
>> %04x\n", textWidth, data->textWidth);
>> +· }
>> +· /* TODO: textWidth computation on Windows is alway that of Wine plus
>> 4, remove this test when it got corrected.
>> +· Also, in Windows textWidth is calculated even if tree item is
>> invisible whereas in Wine is postponed */
>> +· else
>> +· {
>> +· ok_(__FILE__, line)(textWidth == 0 || data->textWidth == 0 ||
>> data->textWidth == textWidth + 4,
>> +· "textWidth %04x, got %04x\n", textWidth, data->textWidth);
>> +· }
>>
> This is very dirty. You should get rid of platform check; testing padding0
> does not look useful; and last test line looks suspicious, if expected
> width is 0 nothing gets tested.
>
>> +· /* Set font to make textWidth calculation consistent */
>> +· hdc=GetDC(hMainWnd);
>> +· SystemParametersInfoA(SPI_GETICONTITLELOGFONT, sizeof(lf), &lf, 0);
>> +· SelectObject(hdc, CreateFontIndirectA(&lf));
>>
> This doesn't make it consistent, because you select one font, and control
> could be using another one, also you're using parent window for some
> reason. WM_GETFONT returns what you need.
>
> Instead of calculating this width manually, it's better to use
> TVM_GETITEMRECT (with wparam == 1), if that gives expected width. If text
> portion rectangle width returned by this message matches internal field
> value on Windows, then we should use that. This will also hide +4
> difference you see, unless it was caused by a wrong font.
>
> typedef·struct _TREEITEM /* HTREEITEM is a _TREEINFO *. */
>> · UINT callbackMask;
>> · UINT state;
>> · UINT stateMask;
>> +· WORD padding0; /* unknown field to make textWidth offset compatible
>> with the native one */
>> +· WORD textWidth; /* horizontal text extent for pszText */
>>
> Instead this should match structure used in tests, i.e. it should start
> with same fields and not have incompatible data mixed with what we try to
> match.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20180121/c76ed216/attachment.html>