Re: Fix text rotation problem in GM_ADVANCED graphics mode caused by incorrect implementation of GetTextExtentPointW().

[Alexandre Julliard]

Ralf Habacker  writes:
> With other patches i have been told to implement such stuff in the dib
> driver. Unfortunally this do not works in this case, because in the top
> level function it looks like having driver specific stuff using display
> coordinates.

It would still most likely have to be in the driver, though maybe the
driver would not be calling that exact entry point. In any case, you
can't change the DC transform like this. and you'll need test cases.

Fix text rotation problem in GM_ADVANCED graphics mode caused by incorrect implementation of GetTextExtentPointW().

[Ralf Habacker]

On 01.10.2013 12:12, Alexandre Julliard wrote:
> Ralf Habacker  writes:
> > With other patches i have been told to implement such stuff in the dib
> > driver. Unfortunally this do not works in this case, because in the top
> > level function it looks like having driver specific stuff using display
> > coordinates.
>
> It would still most likely have to be in the driver,

which is freetype_GetTextExtentExPoint() ?
> though maybe the driver would not be calling that exact entry point.

not sure i understand right:

GetTextExtentExPointW() calls get_char_positions(), which runs
dev->funcs->pGetTextExtentExPoint(), which is mapped to
freetype_GetTextExtentExPoint(), which is in the driver. Which entry
point your are refering else ?
> In any case, you can't change the DC transform like this

then a real solution requires to move the transformation to logical coordinates stuff in BOOL  GetTextExtentExPointW() to freetype_GetTextExtentExPoint() and to manipulate the related matrixes in freetype_GetTextExtentExPoint() directly wen using GM_ADVANCED ?
> and you'll need test cases.

Do you mean in detail:

1. Create a specific font
2. Run GetTextExtentExPointW() which specific parameters
3. check if it results expected values

Regards
 Ralf

Fix text rotation problem in GM_ADVANCED graphics mode caused by incorrect implementation of GetTextExtentPointW().

[Alexandre Julliard]

Ralf Habacker  writes:
> On 01.10.2013 12:12, Alexandre Julliard wrote:
> > Ralf Habacker  writes:
> > > With other patches i have been told to implement such stuff in the dib
> > > driver. Unfortunally this do not works in this case, because in the top
> > > level function it looks like having driver specific stuff using display
> > > coordinates.
> >
> > It would still most likely have to be in the driver,
>
> which is freetype_GetTextExtentExPoint() ?
> > though maybe the driver would not be calling that exact entry point.
>
> not sure i understand right:
>
> GetTextExtentExPointW() calls get_char_positions(), which runs
> dev->funcs->pGetTextExtentExPoint(), which is mapped to
> freetype_GetTextExtentExPoint(), which is in the driver. Which entry
> point your are refering else ?

The various text rendering entry points in the graphics drivers.
> > In any case, you can't change the DC transform like this
>
> then a real solution requires to move the transformation to logical
> coordinates stuff in BOOL GetTextExtentExPointW() to
> freetype_GetTextExtentExPoint() and to manipulate the related matrixes
> in freetype_GetTextExtentExPoint() directly wen using GM_ADVANCED ?

No, I don't think so. The transform is only used to determine the font
scaling factor.
> > and you'll need test cases.
>
> Do you mean in detail:
> - Create a specific font
> - Run GetTextExtentExPointW() which specific parameters
> - check if it results expected values

Well, yes, that's what all tests do.

Fix text rotation problem in GM_ADVANCED graphics mode caused by incorrect implementation of GetTextExtentPointW().

[Ralf Habacker]

On 01.10.2013 12:40, Alexandre Julliard wrote:
> Ralf Habacker  writes:
> > On 01.10.2013 12:12, Alexandre Julliard wrote:
> > > Ralf Habacker  writes:
> > > > With other patches i have been told to implement such stuff in the dib
> > > > driver. Unfortunally this do not works in this case, because in the top
> > > > level function it looks like having driver specific stuff using display
> > > > coordinates.
> > >
> > > It would still most likely have to be in the driver,
> >
> > which is freetype_GetTextExtentExPoint() ?
> > > though maybe the driver would not be calling that exact entry point.
> >
> > not sure i understand right:
> >
> > GetTextExtentExPointW() calls get_char_positions(), which runs
> > dev->funcs->pGetTextExtentExPoint(), which is mapped to
> > freetype_GetTextExtentExPoint(), which is in the driver. Which entry
> > point your are refering else ?
>
> The various text rendering entry points in the graphics drivers.
> > > In any case, you can't change the DC transform like this
> >
> > then a real solution requires to move the transformation to logical
> > coordinates stuff in BOOL GetTextExtentExPointW() to
> > freetype_GetTextExtentExPoint() and to manipulate the related matrixes
> > in freetype_GetTextExtentExPoint() directly wen using GM_ADVANCED ?
>
> No, I don't think so. The transform is only used to determine the font
> scaling factor.

how then ?
> > > and you'll need test cases.
> >
> > Do you mean in detail:
> > - Create a specific font
> > - Run GetTextExtentExPointW() which specific parameters
> > - check if it results expected values
>
> Well, yes, that's what all tests do.

Fix text rotation problem in GM_ADVANCED graphics mode caused by incorrect implementation of GetTextExtentPointW().

[Ralf Habacker]

On 01.10.2013 12:40, Alexandre Julliard wrote:
> Ralf Habacker  writes:
> > On 01.10.2013 12:12, Alexandre Julliard wrote:
> > > Ralf Habacker  writes:
> > > > With other patches i have been told to implement such stuff in the dib
> > > > driver. Unfortunally this do not works in this case, because in the top
> > > > level function it looks like having driver specific stuff using display
> > > > coordinates.
> > >
> > > It would still most likely have to be in the driver,
> >
> > which is freetype_GetTextExtentExPoint() ?
> > > though maybe the driver would not be calling that exact entry point.
> >
> > not sure i understand right:
> >
> > GetTextExtentExPointW() calls get_char_positions(), which runs
> > dev->funcs->pGetTextExtentExPoint(), which is mapped to
> > freetype_GetTextExtentExPoint(), which is in the driver. Which entry
> > point your are refering else ?
>
> The various text rendering entry points in the graphics drivers.

You mean there are more affected places ?
> > > In any case, you can't change the DC transform like this
> >
> > then a real solution requires to move the transformation to logical
> > coordinates stuff in BOOL GetTextExtentExPointW() to
> > freetype_GetTextExtentExPoint() and to manipulate the related matrixes
> > in freetype_GetTextExtentExPoint() directly wen using GM_ADVANCED ?
>
> No, I don't think so. The transform is only used to determine the font
> scaling factor.

the recent implementation of get_glyph_outline() uses in GM_ADVANCED
mode a scale which depends somehow on the rotation angle.

I got better results with saving the "use GM_ADVANCED case" in
font->font_desc.advanced_graphics_mode and the following hunks for
freetype.c

@@ -6426,10 +6426,24 @@ static DWORD get_glyph_outline(GdiFont
*incoming_font, UINT glyph, UINT format,
         worldMat.xy = -FTFixedFromFloat(font->font_desc.matrix.eM21);
         worldMat.yx = -FTFixedFromFloat(font->font_desc.matrix.eM12);
         worldMat.yy = FTFixedFromFloat(font->font_desc.matrix.eM22);
         pFT_Matrix_Multiply(&worldMat, &transMat);
-        pFT_Matrix_Multiply(&worldMat, &transMatUnrotated);
         pFT_Matrix_Multiply(&worldMat, &transMatTategaki);
-        needsTransform = TRUE;
+        if (font->font_desc.advanced_graphics_mode) {
+            worldMat.xx =
FTFixedFromFloat(1.0/font->font_desc.matrix.eM11);
+            worldMat.xy = -FTFixedFromFloat(font->font_desc.matrix.eM21);
+            worldMat.yx = -FTFixedFromFloat(font->font_desc.matrix.eM12);
+            worldMat.yy =
FTFixedFromFloat(1.0/font->font_desc.matrix.eM22);
+        }
+        else {
+            worldMat = identityMat;
+        }
+
+        pFT_Matrix_Multiply(&worldMat, &transMatUnrotated);
+       needsTransform = TRUE;
     }

and

@@ -6571,35 +6590,50 @@ static DWORD get_glyph_outline(GdiFont
*incoming_font, UINT glyph, UINT format,
             origin_y = top;
         }

+        if (font->font_desc.advanced_graphics_mode)
+            pFT_Vector_Transform(&vec, &transMatUnrotated);
+        else
+            pFT_Vector_Transform(&vec, &transMat);
+

which works except for using 90° and 270° world transformations angles.
Transforming back the list of character width in the top level function
GetTextExtentExPointW(). The transformation is done with

static inline INT INTERNAL_XDSTOWS(DC *dc, INT width)
{
    double floatWidth;

floatWidth = (double)width * dc->xformVport2World.eM11;
    /* Round to integers */
    return GDI_ROUND(floatWidth);
}

dc->xformVport2World.eM11 is zero in the mentioned angles, so it will
always return zero, which means at now the submitted patch is the only
working solution.
> From what i can see an alternative would require to move the device to

logical back transformation into get_glyph_outline() and to refactor all
function get_glyph_outline()  is called from.

Regards
Ralf

Fix text rotation problem in GM_ADVANCED graphics mode caused by incorrect implementation of GetTextExtentPointW().

[Ralf Habacker]

On 01.10.2013 12:12, Alexandre Julliard wrote:
> Ralf Habacker  writes:
> > With other patches i have been told to implement such stuff in the dib
> > driver. Unfortunally this do not works in this case, because in the top
> > level function it looks like having driver specific stuff using display
> > coordinates.
>
> It would still most likely have to be in the driver, though maybe the
> driver would not be calling that exact entry point. In any case, you
> can't change the DC transform like this.

You are refering to the usage of setting hard coded values ? Instead I
should decompose the rotation and reset only that part ?

Ralf