From b549c4feece196e8fd7bd07823bd393a0100791c Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Wed, 21 Aug 2024 22:17:39 -0600
Subject: [PATCH] Drawing sheet can't share glyph cache entries with editors.

(They use different internal units.)

Also fixes a fialure of CAIRO_GAL to reset the fill
and stroke after drawing outline glyphs.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/18579
---
 common/drawing_sheet/ds_painter.cpp           |  2 +-
 common/font/font.cpp                          |  9 ++---
 common/font/outline_font.cpp                  | 33 +++++++++++++------
 common/gal/cairo/cairo_gal.cpp                | 12 ++-----
 common/widgets/font_choice.cpp                | 11 +++++--
 include/font/font.h                           |  6 ++--
 include/font/outline_font.h                   |  5 ++-
 include/widgets/font_choice.h                 |  2 +-
 .../dialogs/properties_frame.cpp              |  2 +-
 9 files changed, 51 insertions(+), 31 deletions(-)

diff --git a/common/drawing_sheet/ds_painter.cpp b/common/drawing_sheet/ds_painter.cpp
index 43c8d94130..85a6e179d5 100644
--- a/common/drawing_sheet/ds_painter.cpp
+++ b/common/drawing_sheet/ds_painter.cpp
@@ -279,7 +279,7 @@ void KIGFX::DS_PAINTER::draw( const DS_DRAW_ITEM_TEXT* aItem, int aLayer ) const
     if( !font )
     {
         font = KIFONT::FONT::GetFont( m_renderSettings.GetDefaultFont(), aItem->IsBold(),
-                                      aItem->IsItalic() );
+                                      aItem->IsItalic(), nullptr, true );
     }
 
     const COLOR4D& color = m_renderSettings.GetColor( aItem, aLayer );
diff --git a/common/font/font.cpp b/common/font/font.cpp
index ad36d7706c..ca27995d19 100644
--- a/common/font/font.cpp
+++ b/common/font/font.cpp
@@ -57,7 +57,7 @@ const METRICS& METRICS::Default()
 
 FONT* FONT::s_defaultFont = nullptr;
 
-std::map< std::tuple<wxString, bool, bool>, FONT*> FONT::s_fontMap;
+std::map< std::tuple<wxString, bool, bool, bool>, FONT*> FONT::s_fontMap;
 
 class MARKUP_CACHE
 {
@@ -143,12 +143,13 @@ FONT* FONT::getDefaultFont()
 }
 
 
-FONT* FONT::GetFont( const wxString& aFontName, bool aBold, bool aItalic, const std::vector<wxString>* aEmbeddedFiles )
+FONT* FONT::GetFont( const wxString& aFontName, bool aBold, bool aItalic,
+                     const std::vector<wxString>* aEmbeddedFiles, bool aForDrawingSheet )
 {
     if( aFontName.empty() || aFontName.StartsWith( KICAD_FONT_NAME ) )
         return getDefaultFont();
 
-    std::tuple<wxString, bool, bool> key = { aFontName, aBold, aItalic };
+    std::tuple<wxString, bool, bool, bool> key = { aFontName, aBold, aItalic, aForDrawingSheet };
 
     FONT* font = nullptr;
 
@@ -156,7 +157,7 @@ FONT* FONT::GetFont( const wxString& aFontName, bool aBold, bool aItalic, const
         font = s_fontMap[key];
 
     if( !font )
-        font = OUTLINE_FONT::LoadFont( aFontName, aBold, aItalic, aEmbeddedFiles );
+        font = OUTLINE_FONT::LoadFont( aFontName, aBold, aItalic, aEmbeddedFiles, aForDrawingSheet );
 
     if( !font )
         font = getDefaultFont();
diff --git a/common/font/outline_font.cpp b/common/font/outline_font.cpp
index eb92518154..25fc1dddf5 100644
--- a/common/font/outline_font.cpp
+++ b/common/font/outline_font.cpp
@@ -48,7 +48,8 @@ OUTLINE_FONT::OUTLINE_FONT() :
         m_face(NULL),
         m_faceSize( 16 ),
         m_fakeBold( false ),
-        m_fakeItal( false )
+        m_fakeItal( false ),
+        m_forDrawingSheet( false )
 {
     std::lock_guard<std::mutex> guard( m_freeTypeMutex );
 
@@ -83,7 +84,8 @@ OUTLINE_FONT::EMBEDDING_PERMISSION OUTLINE_FONT::GetEmbeddingPermission() const
 
 
 OUTLINE_FONT* OUTLINE_FONT::LoadFont( const wxString& aFontName, bool aBold, bool aItalic,
-                                      const std::vector<wxString>* aEmbeddedFiles )
+                                      const std::vector<wxString>* aEmbeddedFiles,
+                                      bool aForDrawingSheet )
 {
     std::unique_ptr<OUTLINE_FONT> font = std::make_unique<OUTLINE_FONT>();
 
@@ -109,6 +111,7 @@ OUTLINE_FONT* OUTLINE_FONT::LoadFont( const wxString& aFontName, bool aBold, boo
 
     font->m_fontName = aFontName;       // Keep asked-for name, even if we substituted.
     font->m_fontFileName = fontFile;
+    font->m_forDrawingSheet = aForDrawingSheet;
 
     return font.release();
 }
@@ -292,6 +295,7 @@ struct GLYPH_CACHE_KEY {
     FT_Face        face;
     hb_codepoint_t codepoint;
     double         scaler;
+    bool           forDrawingSheet;
     bool           fakeItalic;
     bool           fakeBold;
     bool           mirror;
@@ -299,9 +303,14 @@ struct GLYPH_CACHE_KEY {
 
     bool operator==(const GLYPH_CACHE_KEY& rhs ) const
     {
-        return face == rhs.face && codepoint == rhs.codepoint && scaler == rhs.scaler
-                   && fakeItalic == rhs.fakeItalic && fakeBold == rhs.fakeBold
-                   && mirror == rhs.mirror && angle == rhs.angle;
+        return face == rhs.face
+                && codepoint == rhs.codepoint
+                && scaler == rhs.scaler
+                && forDrawingSheet == rhs.forDrawingSheet
+                && fakeItalic == rhs.fakeItalic
+                && fakeBold == rhs.fakeBold
+                && mirror == rhs.mirror
+                && angle == rhs.angle;
     }
 };
 
@@ -312,10 +321,14 @@ namespace std
     {
         std::size_t operator()( const GLYPH_CACHE_KEY& k ) const
         {
-            return hash<const void*>()( k.face ) ^ hash<unsigned>()( k.codepoint )
+            return hash<const void*>()( k.face )
+                        ^ hash<unsigned>()( k.codepoint )
                         ^ hash<double>()( k.scaler )
-                        ^ hash<int>()( k.fakeItalic ) ^ hash<int>()( k.fakeBold )
-                        ^ hash<int>()( k.mirror ) ^ hash<int>()( k.angle.AsTenthsOfADegree() );
+                        ^ hash<double>()( k.forDrawingSheet )
+                        ^ hash<int>()( k.fakeItalic )
+                        ^ hash<int>()( k.fakeBold )
+                        ^ hash<int>()( k.mirror )
+                        ^ hash<int>()( k.angle.AsTenthsOfADegree() );
         }
     };
 }
@@ -373,8 +386,8 @@ VECTOR2I OUTLINE_FONT::getTextAsGlyphsUnlocked( BOX2I* aBBox,
 
         if( aGlyphs )
         {
-            GLYPH_CACHE_KEY key = { face, glyphInfo[i].codepoint, scaler, m_fakeItal, m_fakeBold,
-                                    aMirror, aAngle };
+            GLYPH_CACHE_KEY key = { face, glyphInfo[i].codepoint, scaler, m_forDrawingSheet,
+                                    m_fakeItal, m_fakeBold, aMirror, aAngle };
             GLYPH_DATA&     glyphData = s_glyphCache[ key ];
 
             if( glyphData.m_Contours.empty() )
diff --git a/common/gal/cairo/cairo_gal.cpp b/common/gal/cairo/cairo_gal.cpp
index 1e5581568d..848330ccf2 100644
--- a/common/gal/cairo/cairo_gal.cpp
+++ b/common/gal/cairo/cairo_gal.cpp
@@ -1862,7 +1862,7 @@ void CAIRO_GAL_BASE::DrawGlyph( const KIFONT::GLYPH& aGlyph, int aNth, int aTota
         for( const std::vector<VECTOR2D>& pointList : glyph )
             drawPoly( pointList );
     }
-    else if( aGlyph.IsOutline() )
+else if( aGlyph.IsOutline() )
     {
         const KIFONT::OUTLINE_GLYPH& glyph = static_cast<const KIFONT::OUTLINE_GLYPH&>( aGlyph );
 
@@ -1900,15 +1900,9 @@ void CAIRO_GAL_BASE::DrawGlyph( const KIFONT::GLYPH& aGlyph, int aNth, int aTota
 
         if( aNth == aTotal - 1 )
         {
-            /*
-            cairo_close_path( currentContext );
-            setSourceRgba( currentContext, fillColor );
-            cairo_set_fill_rule( currentContext, CAIRO_FILL_RULE_EVEN_ODD );
-            cairo_fill_preserve( currentContext );
-            setSourceRgba( currentContext, strokeColor );
-            cairo_stroke( currentContext );
-            */
             flushPath();
+            SetIsFill( false );
+            SetIsStroke( true );
             m_isElementAdded = true;
         }
     }
diff --git a/common/widgets/font_choice.cpp b/common/widgets/font_choice.cpp
index bf173251b4..f048fec132 100644
--- a/common/widgets/font_choice.cpp
+++ b/common/widgets/font_choice.cpp
@@ -105,13 +105,20 @@ bool FONT_CHOICE::HaveFontSelection() const
 }
 
 
-KIFONT::FONT* FONT_CHOICE::GetFontSelection( bool aBold, bool aItalic ) const
+KIFONT::FONT* FONT_CHOICE::GetFontSelection( bool aBold, bool aItalic, bool aForDrawingSheet ) const
 {
     if( GetSelection() <= 0 )
+    {
         return nullptr;
+    }
     else if( GetSelection() == 1 && m_systemFontCount == 2 )
+    {
         return KIFONT::FONT::GetFont( KICAD_FONT_NAME, aBold, aItalic );
+    }
     else
-        return KIFONT::FONT::GetFont( GetStringSelection(), aBold, aItalic );
+    {
+        return KIFONT::FONT::GetFont( GetStringSelection(), aBold, aItalic, nullptr,
+                                      aForDrawingSheet );
+    }
 }
 
diff --git a/include/font/font.h b/include/font/font.h
index 0fbab48f32..dcc175cc65 100644
--- a/include/font/font.h
+++ b/include/font/font.h
@@ -141,7 +141,9 @@ public:
     virtual bool IsItalic() const  { return false; }
 
     static FONT* GetFont( const wxString& aFontName = wxEmptyString, bool aBold = false,
-                          bool aItalic = false, const std::vector<wxString>* aEmbeddedFiles = nullptr );
+                          bool aItalic = false,
+                          const std::vector<wxString>* aEmbeddedFiles = nullptr,
+                          bool aForDrawingSheet = false );
     static bool IsStroke( const wxString& aFontName );
 
     const wxString& GetName() const { return m_fontName; };
@@ -279,7 +281,7 @@ protected:
 private:
     static FONT* s_defaultFont;
 
-    static std::map< std::tuple<wxString, bool, bool>, FONT* > s_fontMap;
+    static std::map< std::tuple<wxString, bool, bool, bool>, FONT* > s_fontMap;
 };
 
 } //namespace KIFONT
diff --git a/include/font/outline_font.h b/include/font/outline_font.h
index 9a9e943959..fcfd9f3ecf 100644
--- a/include/font/outline_font.h
+++ b/include/font/outline_font.h
@@ -95,7 +95,8 @@ public:
      * @param aFontFileName is the (platform-specific) fully qualified name of the font file
      */
     static OUTLINE_FONT* LoadFont( const wxString& aFontFileName, bool aBold, bool aItalic,
-                                   const std::vector<wxString>* aEmbeddedFiles );
+                                   const std::vector<wxString>* aEmbeddedFiles,
+                                   bool aForDrawingSheet );
 
     /**
      * Compute the distance (interline) between 2 lines of text (for multiline texts).  This is
@@ -152,6 +153,8 @@ private:
     bool              m_fakeBold;
     bool              m_fakeItal;
 
+    bool              m_forDrawingSheet;
+
     // cache for glyphs converted to straight segments
     // key is glyph index (FT_GlyphSlot field glyph_index)
     std::map<unsigned int, std::vector<std::vector<VECTOR2D>>> m_contourCache;
diff --git a/include/widgets/font_choice.h b/include/widgets/font_choice.h
index 8652ee4e50..8a7a728f86 100644
--- a/include/widgets/font_choice.h
+++ b/include/widgets/font_choice.h
@@ -37,7 +37,7 @@ public:
 
     bool HaveFontSelection() const;
 
-    KIFONT::FONT* GetFontSelection( bool aBold, bool aItalic ) const;
+    KIFONT::FONT* GetFontSelection( bool aBold, bool aItalic, bool aForDrawingSheet = false ) const;
 
 private:
     int       m_systemFontCount;
diff --git a/pagelayout_editor/dialogs/properties_frame.cpp b/pagelayout_editor/dialogs/properties_frame.cpp
index 5a6aee83b9..cdc3153d41 100644
--- a/pagelayout_editor/dialogs/properties_frame.cpp
+++ b/pagelayout_editor/dialogs/properties_frame.cpp
@@ -593,7 +593,7 @@ bool PROPERTIES_FRAME::CopyPrmsFromPanelToItem( DS_DATA_ITEM* aItem )
             item->m_Vjustify = GR_TEXT_V_ALIGN_BOTTOM;
 
         if( m_fontCtrl->HaveFontSelection() )
-            item->m_Font = m_fontCtrl->GetFontSelection( item->m_Bold, item->m_Italic );
+            item->m_Font = m_fontCtrl->GetFontSelection( item->m_Bold, item->m_Italic, true );
 
         msg = m_textCtrlRotation->GetValue();
         item->m_Orient = EDA_UNIT_UTILS::UI::DoubleValueFromString( drawSheetIUScale,