<div dir="ltr"><blockquote class="gmail_quote" style="font-size:12.8px;margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+· /* padding0 should be unused on Wine */<br>+· if(!strcmp(winetest_platform, "wine"))<br>+· {<br>+· ok_(__FILE__, line)(data->padding0 == 0, "padding0 %04x, got %04x\n", 0, data->padding0);<br>+· ok_(__FILE__, line)(data->textWidth == textWidth, "textWidth %04x, got %04x\n", textWidth, data->textWidth);<br>+· }<br>+· /* TODO: textWidth computation on Windows is alway that of Wine plus 4, remove this test when it got corrected.<br>+· Also, in Windows textWidth is calculated even if tree item is invisible whereas in Wine is postponed */<br>+· else<br>+· {<br>+· ok_(__FILE__, line)(textWidth == 0 || data->textWidth == 0 || data->textWidth == textWidth + 4,<br>+· "textWidth %04x, got %04x\n", textWidth, data->textWidth);<br>+· }<br></blockquote><span style="font-size:12.8px">+ 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.</span><br><div><br></div><div><span style="font-size:12.8px">You're right, that what I later found out what's wrong with the tests. </span><span style="font-size:12.8px">Thanks </span><span style="font-size:12.8px">for the review.</span></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Jan 21, 2018 at 12:45 AM, Nikolay Sivov <span dir="ltr"><<a href="mailto:nsivov@codeweavers.com" target="_blank">nsivov@codeweavers.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 01/20/2018 08:40 AM, Zhiyi Zhang wrote:<br>
<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
Hi Nikolay,<br>
<br>
This fixes <a href="https://bugs.winehq.org/show_bug.cgi?id=35127" rel="noreferrer" target="_blank">https://bugs.winehq.org/show_b<wbr>ug.cgi?id=35127</a><br>
<br>
As I was working through bug 35127, I noticed that you had once changed the layout of struct _TREEITEM in commit 6ec621e835e03f715d5fee27c5de5b<wbr>0d361814de.<br>
<br>
Now I am sure that textWidth in struct _TREEITEM is a WORD and is at the<br>
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.<br>
<br>
 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.<br>
<br>
<br>
<br></span>
struct·_ITEM_DATA<br>
{<br>
-· HTREEITEM parent; /* for root value of parent field is unidetified */<br>
+· HTREEITEM parent; /* for root value of parent field is unidentified */<br>
· HTREEITEM nextsibling;<br>
· HTREEITEM firstchild;<br>
+· UINT unknownField0; /* untested */<br>
+· UINT unknownField1; /* untested */<br>
+· UINT unknownField2; /* untested */<br>
+· WORD padding0; /* unknown field to make textWidth offset compatible with the native one */<br>
+· WORD textWidth; /* horizontal text extent for pszText */<br>
};<br>
</blockquote>
They are all unknown, so a single unknown[7] is better, assuming this will work on 64 bits.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+· /* padding0 should be unused on Wine */<br>
+· if(!strcmp(winetest_platform, "wine"))<br>
+· {<br>
+· ok_(__FILE__, line)(data->padding0 == 0, "padding0 %04x, got %04x\n", 0, data->padding0);<br>
+· ok_(__FILE__, line)(data->textWidth == textWidth, "textWidth %04x, got %04x\n", textWidth, data->textWidth);<br>
+· }<br>
+· /* TODO: textWidth computation on Windows is alway that of Wine plus 4, remove this test when it got corrected.<br>
+· Also, in Windows textWidth is calculated even if tree item is invisible whereas in Wine is postponed */<br>
+· else<br>
+· {<br>
+· ok_(__FILE__, line)(textWidth == 0 || data->textWidth == 0 || data->textWidth == textWidth + 4,<br>
+· "textWidth %04x, got %04x\n", textWidth, data->textWidth);<br>
+· }<br>
</blockquote>
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.<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+· /* Set font to make textWidth calculation consistent */<br>
+· hdc=GetDC(hMainWnd);<br>
+· SystemParametersInfoA(SPI_GETI<wbr>CONTITLELOGFONT, sizeof(lf), &lf, 0);<br>
+· SelectObject(hdc, CreateFontIndirectA(&lf));<br>
</blockquote>
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.<br>
<br>
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.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
typedef·struct _TREEITEM /* HTREEITEM is a _TREEINFO *. */<br>
· UINT callbackMask;<br>
· UINT state;<br>
· UINT stateMask;<br>
+· WORD padding0; /* unknown field to make textWidth offset compatible with the native one */<br>
+· WORD textWidth; /* horizontal text extent for pszText */<br>
</blockquote>
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.<br>
</blockquote></div><br></div>