Re: [PATCH] user32: Add tests showing that MapWindowPoints, ClientToScreen and ScreenToClient never fail.

[Daniel Santos]

On 10/16/2012 01:12 PM, Christian Costa wrote:
> - /* Choose point inside the client area */
> - point.x = rect.right / 2 - 50;
> - point.y = rect.bottom / 2 - 50;
> - ret = ScreenToClient(wnd, &point);
> - ok(ret, "ScreenToClient failed with %#x\n", GetLastError());
> - ret = MapWindowPoints(NULL, wnd, &point, 1);
> - ok(ret, "MapWindowPoints failed with %#x\n", GetLastError());
> -
> - /* Choose point outside the client area */
> - point.x = rect.right / 2 + 50;
> - point.y = rect.bottom / 2 + 50;
> - ret = ScreenToClient(wnd, &point);
> - ok(ret, "ScreenToClient failed with %#x\n", GetLastError());
> - ret = MapWindowPoints(NULL, wnd, &point, 1);
> - ok(ret, "MapWindowPoints failed with %#x\n", GetLastError());

When I do these tests, I like throwing every stupid thing I can possibly
come up with (and I'm gifted and coming up with stupid things) but
always get shot down because I can't produce an example of a real-world
app doing it.  So here's one from LoTRO:

8.385:0009:Call user32.ScreenToClient(00000000,0033fd68) ret=006d4d87

And if you don't return zero when it does this, the game crashes
resulting in my in-box filling up from desperate game addicts posting to
appdb.

So if you can please include the below in your patch, I can omit it from
mine (see http://bugs.winehq.org/show_bug.cgi?id=31979) and thank you!

+    /* LoTRO will do this and crash if we don't return zero, precious indeed! */
+    ret = ScreenToClient( NULL, &point );
+    ok(!ret, "ScreenToClient didn't fail as expected (got %d)\n", ret);

Daniel

[PATCH] user32: Add tests showing that MapWindowPoints, ClientToScreen and ScreenToClient never fail.

[Christian Costa]

Le 18/10/2012 21:05, Daniel Santos a écrit :
> On 10/16/2012 01:12 PM, Christian Costa wrote:
> > - /* Choose point inside the client area */
> > - point.x = rect.right / 2 - 50;
> > - point.y = rect.bottom / 2 - 50;
> > - ret = ScreenToClient(wnd, &point);
> > - ok(ret, "ScreenToClient failed with %#x\n", GetLastError());
> > - ret = MapWindowPoints(NULL, wnd, &point, 1);
> > - ok(ret, "MapWindowPoints failed with %#x\n", GetLastError());
> > -
> > - /* Choose point outside the client area */
> > - point.x = rect.right / 2 + 50;
> > - point.y = rect.bottom / 2 + 50;
> > - ret = ScreenToClient(wnd, &point);
> > - ok(ret, "ScreenToClient failed with %#x\n", GetLastError());
> > - ret = MapWindowPoints(NULL, wnd, &point, 1);
> > - ok(ret, "MapWindowPoints failed with %#x\n", GetLastError());
>
> When I do these tests, I like throwing every stupid thing I can possibly
> come up with (and I'm gifted and coming up with stupid things) but
> always get shot down because I can't produce an example of a real-world
> app doing it.  So here's one from LoTRO:
>
> 8.385:0009:Call user32.ScreenToClient(00000000,0033fd68) ret=006d4d87
>
> And if you don't return zero when it does this, the game crashes
> resulting in my in-box filling up from desperate game addicts posting to
> appdb.
>
> So if you can please include the below in your patch, I can omit it from
> mine (see http://bugs.winehq.org/show_bug.cgi?id=31979) and thank you!
> - /* LoTRO will do this and crash if we don't return zero, precious indeed! */
> - ret = ScreenToClient( NULL, &point );
> - ok(!ret, "ScreenToClient didn't fail as expected (got %d)\n", ret);
>
> Daniel

I won't have time to work on these tests until the week-end.
My tests are not that significant currently so feel free to submit yours 
before
or wait I submit better patch on monday. As you prefer.

Christian

[PATCH] user32: Add tests showing that MapWindowPoints, ClientToScreen and ScreenToClient never fail.

[Rico Schüller]

On 18.10.2012 21:05, Daniel Santos wrote:
> On 10/16/2012 01:12 PM, Christian Costa wrote:
> > - /* Choose point inside the client area */
> > - point.x = rect.right / 2 - 50;
> > - point.y = rect.bottom / 2 - 50;
> > - ret = ScreenToClient(wnd, &point);
> > - ok(ret, "ScreenToClient failed with %#x\n", GetLastError());
> > - ret = MapWindowPoints(NULL, wnd, &point, 1);
> > - ok(ret, "MapWindowPoints failed with %#x\n", GetLastError());
> > -
> > - /* Choose point outside the client area */
> > - point.x = rect.right / 2 + 50;
> > - point.y = rect.bottom / 2 + 50;
> > - ret = ScreenToClient(wnd, &point);
> > - ok(ret, "ScreenToClient failed with %#x\n", GetLastError());
> > - ret = MapWindowPoints(NULL, wnd, &point, 1);
> > - ok(ret, "MapWindowPoints failed with %#x\n", GetLastError());
>
> When I do these tests, I like throwing every stupid thing I can possibly
> come up with (and I'm gifted and coming up with stupid things) but
> always get shot down because I can't produce an example of a real-world
> app doing it.  So here's one from LoTRO:
>
> 8.385:0009:Call user32.ScreenToClient(00000000,0033fd68) ret=006d4d87
>
> And if you don't return zero when it does this, the game crashes
> resulting in my in-box filling up from desperate game addicts posting to
> appdb.
>
> So if you can please include the below in your patch, I can omit it from
> mine (see http://bugs.winehq.org/show_bug.cgi?id=31979) and thank you!
> - /* LoTRO will do this and crash if we don't return zero, precious indeed! */
> - ret = ScreenToClient( NULL, &point );
> - ok(!ret, "ScreenToClient didn't fail as expected (got %d)\n", ret);
>
> Daniel

Please, be a bit more patient. The problem is known since end of August 
and no one filed a bug for it. Even I asked the guy in #winehq after 
creating the initial hack to file a bug for it. (Someone with a log from 
the channel from 29/30. August might look up the details / logs) I 
didn't send the patch, because it's only a hack and I wasn't able to 
produce a nice test case and later forgot about it.

I was stuck at the different returned point results for different win 
versions (see https://testbot.winehq.org/JobDetails.pl?Key=22315 - I 
used uitools instead of win there to get a smaller log). So someone with 
some more knowledge in that area needs to gave a hint to make a nice 
test. Nevertheless attached is my latest version of the test and 
implementation, but be warned, it is still not ready to be send to 
wine-patches.

If I remember correctly, lotro doesn't need the Get/SetLastError part, 
but I added it anyway. It may be split out and added in a later patch,

Cheers
Rico

[PATCH] user32: Add tests showing that MapWindowPoints, ClientToScreen and ScreenToClient never fail.

[Christian Costa]

Le 18/10/2012 22:27, Rico Schüller a écrit :
> On 18.10.2012 21:05, Daniel Santos wrote:
> > On 10/16/2012 01:12 PM, Christian Costa wrote:
> > > - /* Choose point inside the client area */
> > > - point.x = rect.right / 2 - 50;
> > > - point.y = rect.bottom / 2 - 50;
> > > - ret = ScreenToClient(wnd, &point);
> > > - ok(ret, "ScreenToClient failed with %#x\n", GetLastError());
> > > - ret = MapWindowPoints(NULL, wnd, &point, 1);
> > > - ok(ret, "MapWindowPoints failed with %#x\n", GetLastError());
> > > -
> > > - /* Choose point outside the client area */
> > > - point.x = rect.right / 2 + 50;
> > > - point.y = rect.bottom / 2 + 50;
> > > - ret = ScreenToClient(wnd, &point);
> > > - ok(ret, "ScreenToClient failed with %#x\n", GetLastError());
> > > - ret = MapWindowPoints(NULL, wnd, &point, 1);
> > > - ok(ret, "MapWindowPoints failed with %#x\n", GetLastError());
> >
> > When I do these tests, I like throwing every stupid thing I can possibly
> > come up with (and I'm gifted and coming up with stupid things) but
> > always get shot down because I can't produce an example of a real-world
> > app doing it.  So here's one from LoTRO:
> >
> > 8.385:0009:Call user32.ScreenToClient(00000000,0033fd68) ret=006d4d87
> >
> > And if you don't return zero when it does this, the game crashes
> > resulting in my in-box filling up from desperate game addicts posting to
> > appdb.
> >
> > So if you can please include the below in your patch, I can omit it from
> > mine (see http://bugs.winehq.org/show_bug.cgi?id=31979) and thank you!
> > - /* LoTRO will do this and crash if we don't return zero,
> >
> > precious indeed! */
> > - ret = ScreenToClient( NULL, &point );
> > - ok(!ret, "ScreenToClient didn't fail as expected (got %d)\n", ret);
> >
> > Daniel
>
> Please, be a bit more patient. The problem is known since end of 
> August and no one filed a bug for it. Even I asked the guy in #winehq 
> after creating the initial hack to file a bug for it. (Someone with a 
> log from the channel from 29/30. August might look up the details / 
> logs) I didn't send the patch, because it's only a hack and I wasn't 
> able to produce a nice test case and later forgot about it.
>
> I was stuck at the different returned point results for different win 
> versions (see https://testbot.winehq.org/JobDetails.pl?Key=22315 - I 
> used uitools instead of win there to get a smaller log). So someone 
> with some more knowledge in that area needs to gave a hint to make a 
> nice test. Nevertheless attached is my latest version of the test and 
> implementation, but be warned, it is still not ready to be send to 
> wine-patches.
>
> If I remember correctly, lotro doesn't need the Get/SetLastError part, 
> but I added it anyway. It may be split out and added in a later patch,
>
> Cheers
> Rico

Do you plan to work on you tests soon? Otherwise I can take a look at 
them this week-end.