FontOptions: Add some assertions to warn about misuse

modules/juce_graphics/fonts/juce_Font.cpp

@@ -275,12 +275,7 @@ public:
         jassert (getReferenceCount() == 1);
         typeface = newTypeface;
 
-        if (newTypeface != nullptr)
-        {
-            options = options.withTypeface (typeface)
-                             .withName (typeface->getName())
-                             .withStyle (typeface->getStyle());
-        }
+        options = options.withTypeface (typeface);
     }
 
     void setTypefaceName (String x)
@@ -477,8 +472,8 @@ void Font::setTypefaceName (const String& faceName)
         jassert (faceName.isNotEmpty());
 
         dupeInternalIfShared();
-        font->setTypefaceName (faceName);
         font->setTypeface (nullptr);
+        font->setTypefaceName (faceName);
     }
 }
 
@@ -487,8 +482,8 @@ void Font::setTypefaceStyle (const String& typefaceStyle)
     if (typefaceStyle != font->getTypefaceStyle())
     {
         dupeInternalIfShared();
-        font->setTypefaceStyle (typefaceStyle);
         font->setTypeface (nullptr);
+        font->setTypefaceStyle (typefaceStyle);
     }
 }
 
@@ -935,4 +930,104 @@ Typeface::Ptr Font::getDefaultTypefaceForFont (const Font& font)
     return Native::getDefaultPlatformTypefaceForFont (font);
 }
 
+//==============================================================================
+//==============================================================================
+#if JUCE_UNIT_TESTS
+
+class FontTests : public UnitTest
+{
+public:
+    FontTests() : UnitTest ("Font", UnitTestCategories::graphics) {}
+
+    void runTest() override
+    {
+        const Span data { FontBinaryData::Karla_Regular_Typo_On_Offsets_Off };
+        const auto face = Typeface::createSystemTypefaceFor (data.data(), data.size());
+
+        beginTest ("Old constructor from Typeface");
+        {
+            JUCE_BEGIN_IGNORE_WARNINGS_GCC_LIKE ("-Wdeprecated-declarations")
+            JUCE_BEGIN_IGNORE_WARNINGS_MSVC (4996)
+            Font f { face };
+            JUCE_END_IGNORE_WARNINGS_MSVC
+            JUCE_END_IGNORE_WARNINGS_GCC_LIKE
+
+            expect (f.getTypefaceName() == face->getName());
+            expect (f.getTypefaceStyle() == face->getStyle());
+            expect (f.getTypefacePtr() == face);
+
+            f.setTypefaceStyle ("Italic");
+
+            expect (f.getTypefaceName() == face->getName());
+            expect (f.getTypefaceStyle() == "Italic");
+            expect (f.getTypefacePtr() != face);
+        }
+
+        beginTest ("FontOptions constructor from Typeface");
+        {
+            const FontOptions opt { face };
+            expect (opt.getName() == face->getName());
+            expect (opt.getStyle() == face->getStyle());
+            expect (opt.getTypeface() == face);
+
+            Font f { opt };
+
+            expect (f.getTypefaceName() == face->getName());
+            expect (f.getTypefaceStyle() == face->getStyle());
+            expect (f.getTypefacePtr() == face);
+
+            f.setTypefaceStyle ("Italic");
+
+            expect (f.getTypefaceName() == face->getName());
+            expect (f.getTypefaceStyle() == "Italic");
+            expect (f.getTypefacePtr() != face);
+        }
+
+        beginTest ("FontOptions constructor from Typeface with style and name set");
+        {
+            const auto opt = FontOptions { face }.withName ("placeholder").withStyle ("Italic");
+            expect (opt.getName() == face->getName());
+            expect (opt.getStyle() == face->getStyle());
+            expect (opt.getTypeface() == face);
+
+            Font f { opt };
+
+            expect (f.getTypefaceName() == face->getName());
+            expect (f.getTypefaceStyle() == face->getStyle());
+            expect (f.getTypefacePtr() == face);
+
+            f.setTypefaceStyle ("Italic");
+
+            expect (f.getTypefaceName() == face->getName());
+            expect (f.getTypefaceStyle() == "Italic");
+            expect (f.getTypefacePtr() != face);
+        }
+
+        auto a = FontOptions().withName ("placeholder").withStyle ("Italic");
+
+        beginTest ("Setting Typeface on FontOptions replaces previous name/style");
+        {
+            auto b = a.withTypeface (face);
+
+            expect (b.getName() == face->getName());
+            expect (b.getStyle() == face->getStyle());
+        }
+
+        beginTest ("Setting a name or style on a FontOptions holding a typeface has no effect");
+        {
+            auto b = a.withTypeface (face).withName ("name").withStyle ("style");
+            expect (b.getName() == face->getName());
+            expect (b.getStyle() == face->getStyle());
+
+            auto c = b.withTypeface (nullptr).withName ("name").withStyle ("style");
+            expect (c.getName() == "name");
+            expect (c.getStyle() == "style");
+        }
+    }
+};
+
+static FontTests fontTests;
+
+#endif
+
 } // namespace juce

modules/juce_graphics/fonts/juce_FontOptions.h

@@ -90,18 +90,44 @@ public:
         If the options include a non-null Typeface::Ptr, this will be ignored.
         Otherwise, a suitable typeface will be located based on the typeface name and style strings.
     */
-    [[nodiscard]] FontOptions withName            (String x)               const { return withMember (*this, &FontOptions::name, x); }
+    [[nodiscard]] FontOptions withName (String x) const
+    {
+        if (typeface == nullptr)
+            return withMember (*this, &FontOptions::name, x);
+
+        // This field will be ignored if the typeface pointer is non-null.
+        // If you want to set a custom name, first set the typeface pointer to null.
+        jassertfalse;
+        return *this;
+    }
 
     /** Returns a copy of these options with a new typeface style.
         If the options include a non-null Typeface::Ptr, this will be ignored.
         Otherwise, a suitable typeface will be located based on the typeface name and style strings.
     */
-    [[nodiscard]] FontOptions withStyle           (String x)               const { return withMember (*this, &FontOptions::style, x); }
+    [[nodiscard]] FontOptions withStyle (String x) const
+    {
+        if (typeface == nullptr)
+            return withMember (*this, &FontOptions::style, x);
+
+        // This field will be ignored if the typeface pointer is non-null.
+        // If you want to set a custom style, first set the typeface pointer to null.
+        jassertfalse;
+        return *this;
+    }
 
     /** Returns a copy of these options with a new typeface.
         If the typeface is non-null, it takes precedence over the name and style strings.
     */
-    [[nodiscard]] FontOptions withTypeface        (Typeface::Ptr x)        const { return withMember (*this, &FontOptions::typeface, x); }
+    [[nodiscard]] FontOptions withTypeface (Typeface::Ptr x) const
+    {
+        // If the typeface is non-null, then the name and style fields will be ignored.
+        jassert (x == nullptr || name.isEmpty());
+        jassert (x == nullptr || style.isEmpty());
+
+        auto result = (x != nullptr ? withName (x->getName()).withStyle (x->getStyle()) : *this);
+        return withMember (std::move (result), &FontOptions::typeface, x);
+    }
 
     /** Returns a copy of these options with a new set of preferred fallback family names. */
     [[nodiscard]] FontOptions withFallbacks       (std::vector<String> x)  const { return withMember (*this, &FontOptions::fallbacks, std::move (x)); }