[PATCH] [v2] gdi32: Use the default fallback list before trying Fontconfig.

[Akihiro Sagawa]

In v2, remove some redundant code pointed by Huw.

This fixes a regression introduced by 11ab9ff7b3c43bb41c0325fe9417b1d7f44ad516.
It overrides default fallback font fix for Latin scripts as seen in
3b9669017bc8425f04860f331d8a1f689c8d63e0.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46285
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46244
Signed-off-by: Akihiro Sagawa 
---
 dlls/gdi32/freetype.c | 47 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 15 deletions(-)

[Huw Davies]

On Wed, Dec 19, 2018 at 12:46:19AM +0900, Akihiro Sagawa wrote:
> In v2, remove some redundant code pointed by Huw.
>
> This fixes a regression introduced by 11ab9ff7b3c43bb41c0325fe9417b1d7f44ad516.
> It overrides default fallback font fix for Latin scripts as seen in
> 3b9669017bc8425f04860f331d8a1f689c8d63e0.
>
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46285
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46244
> Signed-off-by: Akihiro Sagawa
>
> dlls/gdi32/freetype.c | 47 ++++++++++++++++++++++++++++++++---------------
>  1 file changed, 32 insertions(+), 15 deletions(-)
> diff --git a/dlls/gdi32/freetype.c b/dlls/gdi32/freetype.c
> index e07aada..4d1333c 100644
> --- a/dlls/gdi32/freetype.c
> +++ b/dlls/gdi32/freetype.c
> @@ -5452,6 +5452,7 @@ static HFONT freetype_SelectFont( PHYSDEV dev, HFONT hfont, UINT *aa_flags )
>      FontSubst *psub = NULL;
>      DC *dc = get_physdev_dc( dev );
>      const SYSTEM_LINKS *font_link;
> -
>
> const WCHAR **fallback_list;
>
> if (!hfont)  /* notification that the font has been changed by another driver */
> {
>
> @@ -5642,25 +5643,41 @@ static HFONT freetype_SelectFont( PHYSDEV dev, HFONT hfont, UINT *aa_flags )
>         so mask with 0xF0 before testing */
>  if((lf.lfPitchAndFamily & FIXED_PITCH) ||
> -
>   (lf.lfPitchAndFamily & 0xF0) == FF_MODERN)
> -
>   (lf.lfPitchAndFamily & 0xF0) == FF_MODERN) {
>    strcpyW(lf.lfFaceName, defFixed);
> - else if((lf.lfPitchAndFamily & 0xF0) == FF_ROMAN)
> -
>    fallback_list = default_fixed_list;
> - }
> - else if((lf.lfPitchAndFamily & 0xF0) == FF_ROMAN) {
>     strcpyW(lf.lfFaceName, defSerif);
> - else if((lf.lfPitchAndFamily & 0xF0) == FF_SWISS)
> -
>    fallback_list = default_serif_list;
> - }
> - else if((lf.lfPitchAndFamily & 0xF0) == FF_SWISS) {
>     strcpyW(lf.lfFaceName, defSans);
> - else
> -
>    fallback_list = default_sans_list;
> - }
> - else {
>     strcpyW(lf.lfFaceName, defSans);
> - LIST_FOR_EACH_ENTRY( family, &font_list, Family, entry ) {
> -
>    if(!strncmpiW(family->FamilyName, lf.lfFaceName, LF_FACESIZE - 1)) {
> -
>        font_link = find_font_link(family->FamilyName);
> -
>        face_list = get_face_list_from_family(family);
> -
>        LIST_FOR_EACH_ENTRY( face, face_list, Face, entry ) {
> -
>            if (!(face->scalable || can_use_bitmap))
> -
>                continue;
> -
>            if (csi.fs.fsCsb[0] & face->fs.fsCsb[0])
> -
>                goto found;
> -
>            if (font_link != NULL && csi.fs.fsCsb[0] & font_link->fs.fsCsb[0])
> -
>                goto found;
> -
>    fallback_list = default_sans_list;
> - }
> -
> - for (; *fallback_list; fallback_list++) {

Sorry, I wasn't clear, I meant why do you need to loop through the
fallback_list?   Can you explain the problem you're trying to fix.

Huw.

[Akihiro Sagawa]

On Wed, 19 Dec 2018 08:32:45 +0000, Huw Davies wrote:
> On Wed, Dec 19, 2018 at 12:46:19AM +0900, Akihiro Sagawa wrote:
> > @@ -5642,25 +5643,41 @@ static HFONT freetype_SelectFont( PHYSDEV dev, HFONT hfont, UINT *aa_flags )
> >         so mask with 0xF0 before testing */
> >  if((lf.lfPitchAndFamily & FIXED_PITCH) ||
> > -
> >   (lf.lfPitchAndFamily & 0xF0) == FF_MODERN)
> > -
> >   (lf.lfPitchAndFamily & 0xF0) == FF_MODERN) {
> >    strcpyW(lf.lfFaceName, defFixed);
> > - else if((lf.lfPitchAndFamily & 0xF0) == FF_ROMAN)
> > -
> >    fallback_list = default_fixed_list;
> > - }
> > - else if((lf.lfPitchAndFamily & 0xF0) == FF_ROMAN) {
> >     strcpyW(lf.lfFaceName, defSerif);
> > - else if((lf.lfPitchAndFamily & 0xF0) == FF_SWISS)
> > -
> >    fallback_list = default_serif_list;
> > - }
> > - else if((lf.lfPitchAndFamily & 0xF0) == FF_SWISS) {
> >     strcpyW(lf.lfFaceName, defSans);
> > - else
> > -
> >    fallback_list = default_sans_list;
> > - }
> > - else {
> >     strcpyW(lf.lfFaceName, defSans);
> > - LIST_FOR_EACH_ENTRY( family, &font_list, Family, entry ) {
> > -
> >    if(!strncmpiW(family->FamilyName, lf.lfFaceName, LF_FACESIZE - 1)) {
> > -
> >        font_link = find_font_link(family->FamilyName);
> > -
> >        face_list = get_face_list_from_family(family);
> > -
> >        LIST_FOR_EACH_ENTRY( face, face_list, Face, entry ) {
> > -
> >            if (!(face->scalable || can_use_bitmap))
> > -
> >                continue;
> > -
> >            if (csi.fs.fsCsb[0] & face->fs.fsCsb[0])
> > -
> >                goto found;
> > -
> >            if (font_link != NULL && csi.fs.fsCsb[0] & font_link->fs.fsCsb[0])
> > -
> >                goto found;
> > -
> >    fallback_list = default_sans_list;
> > - }
> > -
> > - for (; *fallback_list; fallback_list++) {
>
> Sorry, I wasn't clear, I meant why do you need to loop through the
> fallback_list?   Can you explain the problem you're trying to fix.

Yes, the aim of the loop is finding out an alternative font before
trying Fontconfig. If we have not Arial but Liberation Sans, we should
use the font despite Fontconfig.

After my Fontconfig patch 11ab9ff7b3c43bb41c0325fe9417b1d7f44ad516,
wine tries to use Fontconfig to get more properly font. However,
although depending on the configuration, Fontconfig's best font might
not be metrically compatible with the original one.

For instance, there are two Sans-serif fonts, Liberation Sans and DejaVu
Sans, i.e. Arial isn't installed. In this context, an application
requests Arial font family:
Before my patch, "Liberation Sans" is at the top of the font list, so
Liberation Sans is used instead of Arial.
After my patch, it asks Fontconfig and get DejaVu Sans as the best font
and uses it regardless of the font list.

Akihiro Sagawa

[Huw Davies]

On Wed, Dec 19, 2018 at 10:27:10PM +0900, Akihiro Sagawa wrote:
> On Wed, 19 Dec 2018 08:32:45 +0000, Huw Davies wrote:
> > On Wed, Dec 19, 2018 at 12:46:19AM +0900, Akihiro Sagawa wrote:
> > > @@ -5642,25 +5643,41 @@ static HFONT freetype_SelectFont( PHYSDEV dev, HFONT hfont, UINT *aa_flags )
> > >         so mask with 0xF0 before testing */
> > >  if((lf.lfPitchAndFamily & FIXED_PITCH) ||
> > > -
> > >   (lf.lfPitchAndFamily & 0xF0) == FF_MODERN)
> > > -
> > >   (lf.lfPitchAndFamily & 0xF0) == FF_MODERN) {
> > >    strcpyW(lf.lfFaceName, defFixed);
> > > - else if((lf.lfPitchAndFamily & 0xF0) == FF_ROMAN)
> > > -
> > >    fallback_list = default_fixed_list;
> > > - }
> > > - else if((lf.lfPitchAndFamily & 0xF0) == FF_ROMAN) {
> > >     strcpyW(lf.lfFaceName, defSerif);
> > > - else if((lf.lfPitchAndFamily & 0xF0) == FF_SWISS)
> > > -
> > >    fallback_list = default_serif_list;
> > > - }
> > > - else if((lf.lfPitchAndFamily & 0xF0) == FF_SWISS) {
> > >     strcpyW(lf.lfFaceName, defSans);
> > > - else
> > > -
> > >    fallback_list = default_sans_list;
> > > - }
> > > - else {
> > >     strcpyW(lf.lfFaceName, defSans);
> > > - LIST_FOR_EACH_ENTRY( family, &font_list, Family, entry ) {
> > > -
> > >    if(!strncmpiW(family->FamilyName, lf.lfFaceName, LF_FACESIZE - 1)) {
> > > -
> > >        font_link = find_font_link(family->FamilyName);
> > > -
> > >        face_list = get_face_list_from_family(family);
> > > -
> > >        LIST_FOR_EACH_ENTRY( face, face_list, Face, entry ) {
> > > -
> > >            if (!(face->scalable || can_use_bitmap))
> > > -
> > >                continue;
> > > -
> > >            if (csi.fs.fsCsb[0] & face->fs.fsCsb[0])
> > > -
> > >                goto found;
> > > -
> > >            if (font_link != NULL && csi.fs.fsCsb[0] & font_link->fs.fsCsb[0])
> > > -
> > >                goto found;
> > > -
> > >    fallback_list = default_sans_list;
> > > - }
> > > -
> > > - for (; *fallback_list; fallback_list++) {
> >
> > Sorry, I wasn't clear, I meant why do you need to loop through the
> > fallback_list?   Can you explain the problem you're trying to fix.
>
> Yes, the aim of the loop is finding out an alternative font before
> trying Fontconfig. If we have not Arial but Liberation Sans, we should
> use the font despite Fontconfig.
>
> After my Fontconfig patch 11ab9ff7b3c43bb41c0325fe9417b1d7f44ad516,
> wine tries to use Fontconfig to get more properly font. However,
> although depending on the configuration, Fontconfig's best font might
> not be metrically compatible with the original one.
>
> For instance, there are two Sans-serif fonts, Liberation Sans and DejaVu
> Sans, i.e. Arial isn't installed. In this context, an application
> requests Arial font family:
> Before my patch, "Liberation Sans" is at the top of the font list, so
> Liberation Sans is used instead of Arial.
> After my patch, it asks Fontconfig and get DejaVu Sans as the best font
> and uses it regardless of the font list.

Ok, I've sent something in that should achieve this is a rather
different way.

Huw.

[Akihiro Sagawa]

On Thu, 20 Dec 2018 10:33:27 +0000, Huw Davies wrote:
> On Wed, Dec 19, 2018 at 10:27:10PM +0900, Akihiro Sagawa wrote:
> > On Wed, 19 Dec 2018 08:32:45 +0000, Huw Davies wrote:
> > > On Wed, Dec 19, 2018 at 12:46:19AM +0900, Akihiro Sagawa wrote:
> > > > @@ -5642,25 +5643,41 @@ static HFONT freetype_SelectFont( PHYSDEV dev, HFONT hfont, UINT *aa_flags )
> > > >         so mask with 0xF0 before testing */
> > > >  if((lf.lfPitchAndFamily & FIXED_PITCH) ||
> > > > -
> > > >   (lf.lfPitchAndFamily & 0xF0) == FF_MODERN)
> > > > -
> > > >   (lf.lfPitchAndFamily & 0xF0) == FF_MODERN) {
> > > >    strcpyW(lf.lfFaceName, defFixed);
> > > > - else if((lf.lfPitchAndFamily & 0xF0) == FF_ROMAN)
> > > > -
> > > >    fallback_list = default_fixed_list;
> > > > - }
> > > > - else if((lf.lfPitchAndFamily & 0xF0) == FF_ROMAN) {
> > > >     strcpyW(lf.lfFaceName, defSerif);
> > > > - else if((lf.lfPitchAndFamily & 0xF0) == FF_SWISS)
> > > > -
> > > >    fallback_list = default_serif_list;
> > > > - }
> > > > - else if((lf.lfPitchAndFamily & 0xF0) == FF_SWISS) {
> > > >     strcpyW(lf.lfFaceName, defSans);
> > > > - else
> > > > -
> > > >    fallback_list = default_sans_list;
> > > > - }
> > > > - else {
> > > >     strcpyW(lf.lfFaceName, defSans);
> > > > - LIST_FOR_EACH_ENTRY( family, &font_list, Family, entry ) {
> > > > -
> > > >    if(!strncmpiW(family->FamilyName, lf.lfFaceName, LF_FACESIZE - 1)) {
> > > > -
> > > >        font_link = find_font_link(family->FamilyName);
> > > > -
> > > >        face_list = get_face_list_from_family(family);
> > > > -
> > > >        LIST_FOR_EACH_ENTRY( face, face_list, Face, entry ) {
> > > > -
> > > >            if (!(face->scalable || can_use_bitmap))
> > > > -
> > > >                continue;
> > > > -
> > > >            if (csi.fs.fsCsb[0] & face->fs.fsCsb[0])
> > > > -
> > > >                goto found;
> > > > -
> > > >            if (font_link != NULL && csi.fs.fsCsb[0] & font_link->fs.fsCsb[0])
> > > > -
> > > >                goto found;
> > > > -
> > > >    fallback_list = default_sans_list;
> > > > - }
> > > > -
> > > > - for (; *fallback_list; fallback_list++) {
> > >
> > > Sorry, I wasn't clear, I meant why do you need to loop through the
> > > fallback_list?   Can you explain the problem you're trying to fix.
> >
> > Yes, the aim of the loop is finding out an alternative font before
> > trying Fontconfig. If we have not Arial but Liberation Sans, we should
> > use the font despite Fontconfig.
> >
> > After my Fontconfig patch 11ab9ff7b3c43bb41c0325fe9417b1d7f44ad516,
> > wine tries to use Fontconfig to get more properly font. However,
> > although depending on the configuration, Fontconfig's best font might
> > not be metrically compatible with the original one.
> >
> > For instance, there are two Sans-serif fonts, Liberation Sans and DejaVu
> > Sans, i.e. Arial isn't installed. In this context, an application
> > requests Arial font family:
> > Before my patch, "Liberation Sans" is at the top of the font list, so
> > Liberation Sans is used instead of Arial.
> > After my patch, it asks Fontconfig and get DejaVu Sans as the best font
> > and uses it regardless of the font list.
>
> Ok, I've sent something in that should achieve this is a rather
> different way.

Looks so nice.
Thanks, Huw.

Akihiro Sagawa