Re: gdi32/path.c -- Allow PATH_ExtTextOut() handle nonprintablecharacters

2009-03-24 Thread Massimo Del Fedele

Massimo Del Fedele ha scritto:


But then, I'm still not sure IF GetGlyphOutlineW does return GDI_ERROR 
when called with GGO_GLYPH_INDEX flag (msn is not clear about...) nor I 
know how to make a call to GetGlyphOutlineW() requesting a buffer size 
which simulates an error. I could do with a wrong HDC, but we will be 
never sure that some other kind of error wouldn't return NULL instead of 
GDI_ERROR. My patch simply avoided the problem with :


dwSize = GetGlyphOutlineW(hdc, str[idx], GGO_GLYPH_INDEX | GGO_NATIVE, 
&gm, 0, NULL, &identity);

if(dwSize)
{
   HERE NORMAL PROCESSING - BUF SIZE OK
}
/* GetGlyphOutlineW may return null size for space character,
   so we try to get the metrics for it */
else if(GetGlyphOutlineW(hdc, str[idx], GGO_GLYPH_INDEX | GGO_METRICS, 
&gm, 0, NULL, &identity) == GDI_ERROR)

return FALSE;

This is guaranteed to work as by MSN description.

Ciao

Max



Hm... even better :


if(dwSize == GDI_ERROR)
 return FALSE;
if(dwSize)
{
HERE NORMAL PROCESSING - BUF SIZE OK
}
/* GetGlyphOutlineW may return null size for space character,
   so we try to get the metrics for it */
else if(GetGlyphOutlineW(hdc, str[idx], GGO_GLYPH_INDEX | GGO_METRICS, 
&gm, 0, NULL, &identity) == GDI_ERROR)

 return FALSE;

This will work on evey possible case

Max





Re: gdi32/path.c -- Allow PATH_ExtTextOut() handle nonprintablecharacters

2009-03-24 Thread Alexandre Julliard
Massimo Del Fedele  writes:

> But then, I'm still not sure IF GetGlyphOutlineW does return GDI_ERROR
> when called with GGO_GLYPH_INDEX flag (msn is not clear about...) nor
> I know how to make a call to GetGlyphOutlineW() requesting a buffer
> size which simulates an error. I could do with a wrong HDC, but we
> will be never sure that some other kind of error wouldn't return NULL
> instead of GDI_ERROR.

There's no reason to believe that some errors would return 0, but that's
what test cases are for.

> My patch simply avoided the problem with :
>
> dwSize = GetGlyphOutlineW(hdc, str[idx], GGO_GLYPH_INDEX | GGO_NATIVE,
> &gm, 0, NULL, &identity);
> if(dwSize)
> {
>HERE NORMAL PROCESSING - BUF SIZE OK
> }
> /* GetGlyphOutlineW may return null size for space character,
>so we try to get the metrics for it */
> else if(GetGlyphOutlineW(hdc, str[idx], GGO_GLYPH_INDEX | GGO_METRICS,
> &gm, 0, NULL, &identity) == GDI_ERROR)
> return FALSE;
>
> This is guaranteed to work as by MSN description.

No, you don't handle GDI_ERROR correctly.

-- 
Alexandre Julliard
julli...@winehq.org




Re: gdi32/path.c -- Allow PATH_ExtTextOut() handle nonprintablecharacters

2009-03-24 Thread Massimo Del Fedele

Alexandre Julliard ha scritto:


GDI_ERROR is clearly not a valid buffer size, and must be checked
for. Please write test cases like Dmitry suggested.



Yep, right, GDI_ERROR is -1L, too large to be a buffer size.
Uhmmm... the only non-graphical testcase that occours to me is one 
showing that GetGlyphOutline() returns NULL buffer size on non-printable 
characters It does, both on wine and on windows, is the check inside 
PATH_ExtTextOut() which is wrong.

It's done as

dwSize = GetGlyphOutlineW(hdc, str[idx], GGO_GLYPH_INDEX | GGO_NATIVE, 
&gm, 0, NULL, &identity);

if (!dwSize) return FALSE; <--- ERROR ON SPACE CHARACTER

which should be

dwSize = GetGlyphOutlineW(hdc, str[idx], GGO_GLYPH_INDEX | 
GGO_NATIVE, &gm, 0, NULL, &identity);

if (dwSize == GDI_ERROR) return FALSE;

But then, I'm still not sure IF GetGlyphOutlineW does return GDI_ERROR 
when called with GGO_GLYPH_INDEX flag (msn is not clear about...) nor I 
know how to make a call to GetGlyphOutlineW() requesting a buffer size 
which simulates an error. I could do with a wrong HDC, but we will be 
never sure that some other kind of error wouldn't return NULL instead of 
GDI_ERROR. My patch simply avoided the problem with :


dwSize = GetGlyphOutlineW(hdc, str[idx], GGO_GLYPH_INDEX | GGO_NATIVE, 
&gm, 0, NULL, &identity);

if(dwSize)
{
   HERE NORMAL PROCESSING - BUF SIZE OK
}
/* GetGlyphOutlineW may return null size for space character,
   so we try to get the metrics for it */
else if(GetGlyphOutlineW(hdc, str[idx], GGO_GLYPH_INDEX | GGO_METRICS, 
&gm, 0, NULL, &identity) == GDI_ERROR)

return FALSE;

This is guaranteed to work as by MSN description.

Ciao

Max





Re: gdi32/path.c -- Allow PATH_ExtTextOut() handle nonprintablecharacters

2009-03-24 Thread Alexandre Julliard
Massimo Del Fedele  writes:

> Yep, of course, but we don't need to check for GetGlyphOutlineW()
> correctness (well, not for this case, at least). It IS correct.
> If you ask for buffer size (as done on first call) it returns 0 on
> error AND on unprintable glyphs. The GDI_ERROR value is returned ONLY
> when asking just for metrics or trying to get the glyph data, NOT when
> asking for buffer size (on which GDI_ERROR could be a valid buffer
> size), as well documented on MSDN :
> http://msdn.microsoft.com/en-us/library/dd144891(VS.85).aspx
> (see return value)

GDI_ERROR is clearly not a valid buffer size, and must be checked
for. Please write test cases like Dmitry suggested.

-- 
Alexandre Julliard
julli...@winehq.org




Re: gdi32/path.c -- Allow PATH_ExtTextOut() handle nonprintablecharacters

2009-03-17 Thread Massimo Del Fedele

Dmitry Timoshkov ha scritto:

"Massimo Del Fedele"  wrote:


This patch doesn't match the normal logic of ExtTextOut implemented
in dlls/winex11.drv/xrender.c, also this requires a test case for
both code paths.



Why ? As is it now PATH_ExtTextOut() simply returns FALSE on first 
space or non-printable glyph, you can see easily printing something like

"THIS IS A TEST" on an open path.
BTW, this behaviour (GetGlyphOutlineW returning NULL on spaces) is 
quite well known, and is NOT an error value, simply there's no glyph 
outline for non-printable characters, so the requested buffer size is 
NULL; the function just outputs the metrics.


You need to make PATH_ExtTextOut() behave like dlls/winex11.drv/xrender.c,
X11DRV_XRender_ExtTextOut() does, i.e. treat GDI_ERROR as an error case,
not 0.

I do :
dwSize = GetGlyphOutlineW(hdc, str[idx], GGO_GLYPH_INDEX | GGO_NATIVE, 
&gm, 0, NULL, &identity);

if(dwSize)
{
/* get and process glyph data */

}
else if(GetGlyphOutlineW(hdc, str[idx], GGO_GLYPH_INDEX | GGO_METRICS, 
&gm, 0, NULL, &identity) == GDI_ERROR) <== HERE !

return FALSE;

The check against GDI_ERROR was indeed missing on original code, which 
just checked for 0 result when asking for buffer size, returning FALSE 
if found.
It could be done the way around, first getting the metrics and then if 
no error the buffer size and the glyph data, but you'd have an 
unnecessary added call to GetGlyphOutlineW() for printable chars.


Also, I don't know how to make a testcase with no graphic output 
If you have some suggestion about, I can make one.


GetGlyphOutlineW doesn't produce any graphic output.

Yep, of course, but we don't need to check for GetGlyphOutlineW() 
correctness (well, not for this case, at least). It IS correct.
If you ask for buffer size (as done on first call) it returns 0 on error 
AND on unprintable glyphs. The GDI_ERROR value is returned ONLY when 
asking just for metrics or trying to get the glyph data, NOT when asking 
for buffer size (on which GDI_ERROR could be a valid buffer size), as 
well documented on MSDN :

http://msdn.microsoft.com/en-us/library/dd144891(VS.85).aspx
(see return value)
The only test case I can see here is a graphic one, showing the missing 
text after the space char on wine.

Ciao

Max





Re: gdi32/path.c -- Allow PATH_ExtTextOut() handle nonprintablecharacters

2009-03-16 Thread Dmitry Timoshkov

"Massimo Del Fedele"  wrote:


This patch doesn't match the normal logic of ExtTextOut implemented
in dlls/winex11.drv/xrender.c, also this requires a test case for
both code paths.



Why ? As is it now PATH_ExtTextOut() simply returns FALSE on first space 
or non-printable glyph, you can see easily printing something like

"THIS IS A TEST" on an open path.
BTW, this behaviour (GetGlyphOutlineW returning NULL on spaces) is quite 
well known, and is NOT an error value, simply there's no glyph outline 
for non-printable characters, so the requested buffer size is NULL; the 
function just outputs the metrics.


You need to make PATH_ExtTextOut() behave like dlls/winex11.drv/xrender.c,
X11DRV_XRender_ExtTextOut() does, i.e. treat GDI_ERROR as an error case,
not 0.

Also, I don't know how to make a testcase with no graphic output If 
you have some suggestion about, I can make one.


GetGlyphOutlineW doesn't produce any graphic output.

--
Dmitry.