Re: [PATCH v2 3/7] gdi32: Properly handle exact fits in GetTextExtentExPoint.

[Sven Baars]

On 12-11-2020 10:25, Huw Davies wrote:
> On Mon, Nov 09, 2020 at 03:07:15PM +0100, Sven Baars wrote:
> > Signed-off-by: Sven Baars
> >
> > dlls/gdi32/font.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/dlls/gdi32/font.c b/dlls/gdi32/font.c
> > index 6ad03df176b..4a6ca1916ac 100644
> > --- a/dlls/gdi32/font.c
> > +++ b/dlls/gdi32/font.c
> > @@ -4897,6 +4897,11 @@ BOOL WINAPI GetTextExtentExPointI( HDC hdc, const WORD *indices, INT count, INT
> >                  unsigned int dx = abs( INTERNAL_XDSTOWS( dc, pos[i] )) + (i + 1) * dc->charExtra;
> >                  if (nfit && dx > (unsigned int)max_ext) break;
> >                  if (dxs) dxs[i] = dx;
> > -
> >            if (nfit && dx == (unsigned int)max_ext)
> > -
> >            {
> > -
> >                i++;
> > -
> >                break;
> > -
> >            }
> >        }
> >        if (nfit) *nfit = i;
> >    }
> >
> > @@ -5033,7 +5038,12 @@ BOOL WINAPI GetTextExtentExPointW( HDC hdc, LPCWSTR str, INT count, INT max_ext,
> >              {
> >                  unsigned int dx = abs( INTERNAL_XDSTOWS( dc, pos[i] )) + (i + 1) * dc->charExtra;
> >                  if (nfit && dx > (unsigned int)max_ext) break;
> > -
> > if (dxs) dxs[i] = dx;
> > -
> >            if (dxs) dxs[i] = dx;
> > -
> >            if (nfit && dx == (unsigned int)max_ext)
> > -
> >            {
> > -
> >                i++;
> > -
> >                break;
> > -
> >            }
> >        }
> >        if (nfit) *nfit = i;
> >    }
>
> These will need tests.  Also, combining the new if()s with the test
> two lines above would most likely make things simpler.
>
> Huw.

Hi Huw,

The tests are already present in

https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/gdi32/tests/font.c#l1...

and I further extended these in patch 2. They currently wrongfully
succeed because the characters that are supposed to have width 0 do not
have width 0. They would start failing after patch 5 without this patch.
If this is not sufficient, are there any characters with width 0 that
are currently reported correctly to have width 0 by wine that can be
used for this test?

As for the if statement, it would look like this:

if (dxs) dxs[i] = dx;
                 if (nfit && dx >= (unsigned int)max_ext)
                 {
                     if (dx == (unsigned int)max_ext)
                         i++;
                     break;
                 }

however, that would mean that dx is also set in the > case, which is not
the case on Windows. I could add a test for this, but that will
currently fail on wine because of the earlier call to
get_char_positions(), so it will not detect a wrong implementation of
this if statement.

Cheers,
Sven