From 412242774820c162be14124bc525704df446c690 Mon Sep 17 00:00:00 2001 From: reuk Date: Thu, 30 May 2024 15:54:34 +0100 Subject: [PATCH] FontOptions: Add some assertions to warn about misuse --- modules/juce_graphics/fonts/juce_Font.cpp | 111 ++++++++++++++++-- .../juce_graphics/fonts/juce_FontOptions.h | 32 ++++- 2 files changed, 132 insertions(+), 11 deletions(-) diff --git a/modules/juce_graphics/fonts/juce_Font.cpp b/modules/juce_graphics/fonts/juce_Font.cpp index ea574c74d3..77436ac122 100644 --- a/modules/juce_graphics/fonts/juce_Font.cpp +++ b/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 diff --git a/modules/juce_graphics/fonts/juce_FontOptions.h b/modules/juce_graphics/fonts/juce_FontOptions.h index 2dcdd8c1e5..d2d0543709 100644 --- a/modules/juce_graphics/fonts/juce_FontOptions.h +++ b/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 x) const { return withMember (*this, &FontOptions::fallbacks, std::move (x)); }