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

[Nikolay Sivov]

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 
> 6ec621e835e03f715d5fee27c5de5b0d361814de.
>
> 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.