Add std::shared_ptr overloads for assigning resources while maintaining lifetime safety

include/SFML/Graphics/Sprite.hpp

@@ -34,6 +34,9 @@
 #include <SFML/Graphics/Transformable.hpp>
 #include <SFML/Graphics/Vertex.hpp>
 
+#include <memory>
+#include <variant>
+
 
 namespace sf
 {
@@ -57,6 +60,16 @@ public:
     ////////////////////////////////////////////////////////////
     explicit Sprite(const Texture& texture);
 
+    ////////////////////////////////////////////////////////////
+    /// \brief Construct the sprite from a source texture
+    ///
+    /// \param texture Source texture
+    ///
+    /// \see setTexture
+    ///
+    ////////////////////////////////////////////////////////////
+    explicit Sprite(std::shared_ptr<const Texture> texture);
+
     ////////////////////////////////////////////////////////////
     /// \brief Disallow construction from a temporary texture
     ///
@@ -74,6 +87,17 @@ public:
     ////////////////////////////////////////////////////////////
     Sprite(const Texture& texture, const IntRect& rectangle);
 
+    ////////////////////////////////////////////////////////////
+    /// \brief Construct the sprite from a sub-rectangle of a source texture
+    ///
+    /// \param texture   Source texture
+    /// \param rectangle Sub-rectangle of the texture to assign to the sprite
+    ///
+    /// \see setTexture, setTextureRect
+    ///
+    ////////////////////////////////////////////////////////////
+    Sprite(std::shared_ptr<const Texture> texture, const IntRect& rectangle);
+
     ////////////////////////////////////////////////////////////
     /// \brief Disallow construction from a temporary texture
     ///
@@ -101,6 +125,21 @@ public:
     ////////////////////////////////////////////////////////////
     void setTexture(const Texture& texture, bool resetRect = false);
 
+    ////////////////////////////////////////////////////////////
+    /// \brief Change the source texture of the sprite
+    ///
+    /// Same as above but using a shared pointer to a texture to
+    /// which guarantees that the texture remains alive for the
+    /// entire lifetime of the sprite.
+    ///
+    /// \param texture   New texture
+    /// \param resetRect Should the texture rect be reset to the size of the new texture?
+    ///
+    /// \see getTexture, setTextureRect
+    ///
+    ////////////////////////////////////////////////////////////
+    void setTexture(std::shared_ptr<const Texture> texture, bool resetRect = false);
+
     ////////////////////////////////////////////////////////////
     /// \brief Disallow setting from a temporary texture
     ///
@@ -222,9 +261,10 @@ private:
     ////////////////////////////////////////////////////////////
     // Member data
     ////////////////////////////////////////////////////////////
-    Vertex         m_vertices[4]; //!< Vertices defining the sprite's geometry
+    using TexturePtr = std::variant<const Texture*, std::shared_ptr<const Texture>>;
-    const Texture* m_texture{};   //!< Texture of the sprite
+    Vertex     m_vertices[4]; //!< Vertices defining the sprite's geometry
-    IntRect        m_textureRect; //!< Rectangle defining the area of the source texture to display
+    TexturePtr m_texture;     //!< Texture of the sprite
+    IntRect    m_textureRect; //!< Rectangle defining the area of the source texture to display
 };
 
 } // namespace sf

include/SFML/Graphics/Text.hpp

@@ -36,6 +36,8 @@
 
 #include <SFML/System/String.hpp>
 
+#include <memory>
+#include <variant>
 #include <vector>
 
 
@@ -80,6 +82,20 @@ public:
     ////////////////////////////////////////////////////////////
     Text(const Font& font, String string = "", unsigned int characterSize = 30);
 
+    ////////////////////////////////////////////////////////////
+    /// \brief Construct the text from a string, font and size
+    ///
+    /// Construct as above but using a shared pointer to a font
+    /// which guarantees the font remains alive for the lifetime
+    /// of the text object.
+    ///
+    /// \param string         Text assigned to the string
+    /// \param font           Font used to draw the string
+    /// \param characterSize  Base size of characters, in pixels
+    ///
+    ////////////////////////////////////////////////////////////
+    Text(std::shared_ptr<const Font> font, String string = "", unsigned int characterSize = 30);
+
     ////////////////////////////////////////////////////////////
     /// \brief Disallow construction from a temporary font
     ///
@@ -124,6 +140,18 @@ public:
     ////////////////////////////////////////////////////////////
     void setFont(const Font& font);
 
+    ////////////////////////////////////////////////////////////
+    /// \brief Set the text's font
+    ///
+    /// Same as above but using a shared pointer to a font
+    /// which guarantees the font remains alive for the lifetime
+    /// of the text object.
+    ///
+    /// \see getFont
+    ///
+    ////////////////////////////////////////////////////////////
+    void setFont(std::shared_ptr<const Font> font);
+
     ////////////////////////////////////////////////////////////
     /// \brief Disallow setting from a temporary font
     ///
@@ -406,8 +434,9 @@ private:
     ////////////////////////////////////////////////////////////
     // Member data
     ////////////////////////////////////////////////////////////
+    using FontPtr = std::variant<const Font*, std::shared_ptr<const Font>>;
     String                m_string;                                    //!< String to display
-    const Font*           m_font{};                                    //!< Font used to display the string
+    FontPtr               m_font;                                      //!< Font used to display the string
     unsigned int          m_characterSize{30};                         //!< Base size of characters, in pixels
     float                 m_letterSpacingFactor{1.f};                  //!< Spacing factor between letters
     float                 m_lineSpacingFactor{1.f};                    //!< Spacing factor between lines

src/SFML/Graphics/Sprite.cpp

@@ -42,6 +42,13 @@ Sprite::Sprite(const Texture& texture)
 }
 
 
+////////////////////////////////////////////////////////////
+Sprite::Sprite(std::shared_ptr<const Texture> texture)
+{
+    setTexture(std::move(texture), true);
+}
+
+
 ////////////////////////////////////////////////////////////
 Sprite::Sprite(const Texture& texture, const IntRect& rectangle)
 {
@@ -52,6 +59,16 @@ Sprite::Sprite(const Texture& texture, const IntRect& rectangle)
 }
 
 
+////////////////////////////////////////////////////////////
+Sprite::Sprite(std::shared_ptr<const Texture> texture, const IntRect& rectangle)
+{
+    // Compute the texture area
+    setTextureRect(rectangle);
+    // Assign texture
+    setTexture(std::move(texture), false);
+}
+
+
 ////////////////////////////////////////////////////////////
 void Sprite::setTexture(const Texture& texture, bool resetRect)
 {
@@ -66,6 +83,22 @@ void Sprite::setTexture(const Texture& texture, bool resetRect)
 }
 
 
+////////////////////////////////////////////////////////////
+void Sprite::setTexture(std::shared_ptr<const Texture> texture, bool resetRect)
+{
+    assert(texture && "Sprite::setTexture() Texture cannot be null");
+
+    // Recompute the texture area if requested
+    if (resetRect)
+    {
+        setTextureRect(IntRect({0, 0}, Vector2i(texture->getSize())));
+    }
+
+    // Assign the new texture
+    m_texture = std::move(texture);
+}
+
+
 ////////////////////////////////////////////////////////////
 void Sprite::setTextureRect(const IntRect& rectangle)
 {
@@ -92,7 +125,18 @@ void Sprite::setColor(const Color& color)
 ////////////////////////////////////////////////////////////
 const Texture& Sprite::getTexture() const
 {
-    return *m_texture;
+    static const struct Visitor
+    {
+        const sf::Texture* operator()(const sf::Texture* texture) const
+        {
+            return texture;
+        }
+        const sf::Texture* operator()(const std::shared_ptr<const sf::Texture>& texture) const
+        {
+            return texture.get();
+        }
+    } visitor;
+    return *std::visit(visitor, m_texture);
 }
 
 
@@ -133,7 +177,7 @@ void Sprite::draw(RenderTarget& target, const RenderStates& states) const
     RenderStates statesCopy(states);
 
     statesCopy.transform *= getTransform();
-    statesCopy.texture = m_texture;
+    statesCopy.texture = &getTexture();
     target.draw(m_vertices, 4, PrimitiveType::TriangleStrip, statesCopy);
 }
 

src/SFML/Graphics/Text.cpp

@@ -33,6 +33,7 @@
 #include <algorithm>
 #include <utility>
 
+#include <cassert>
 #include <cmath>
 
 
@@ -104,6 +105,16 @@ m_characterSize(characterSize)
 }
 
 
+////////////////////////////////////////////////////////////
+Text::Text(std::shared_ptr<const Font> font, String string, unsigned int characterSize) :
+m_string(std::move(string)),
+m_characterSize(characterSize)
+{
+    assert(font && "Text::Text() Font cannot be null");
+    m_font = std::move(font);
+}
+
+
 ////////////////////////////////////////////////////////////
 void Text::setString(const String& string)
 {
@@ -118,7 +129,7 @@ void Text::setString(const String& string)
 ////////////////////////////////////////////////////////////
 void Text::setFont(const Font& font)
 {
-    if (m_font != &font)
+    if (&getFont() != &font)
     {
         m_font               = &font;
         m_geometryNeedUpdate = true;
@@ -126,6 +137,19 @@ void Text::setFont(const Font& font)
 }
 
 
+////////////////////////////////////////////////////////////
+void Text::setFont(std::shared_ptr<const Font> font)
+{
+    assert(font && "Text::setFont() Font cannot be null");
+
+    if (&getFont() != font.get())
+    {
+        m_font               = std::move(font);
+        m_geometryNeedUpdate = true;
+    }
+}
+
+
 ////////////////////////////////////////////////////////////
 void Text::setCharacterSize(unsigned int size)
 {
@@ -227,7 +251,18 @@ const String& Text::getString() const
 ////////////////////////////////////////////////////////////
 const Font& Text::getFont() const
 {
-    return *m_font;
+    static const struct Visitor
+    {
+        const sf::Font* operator()(const sf::Font* font) const
+        {
+            return font;
+        }
+        const sf::Font* operator()(const std::shared_ptr<const sf::Font>& font) const
+        {
+            return font.get();
+        }
+    } visitor;
+    return *std::visit(visitor, m_font);
 }
 
 
@@ -289,10 +324,10 @@ Vector2f Text::findCharacterPos(std::size_t index) const
 
     // Precompute the variables needed by the algorithm
     const bool  isBold          = m_style & Bold;
-    float       whitespaceWidth = m_font->getGlyph(U' ', m_characterSize, isBold).advance;
+    float       whitespaceWidth = getFont().getGlyph(U' ', m_characterSize, isBold).advance;
     const float letterSpacing   = (whitespaceWidth / 3.f) * (m_letterSpacingFactor - 1.f);
     whitespaceWidth += letterSpacing;
-    const float lineSpacing = m_font->getLineSpacing(m_characterSize) * m_lineSpacingFactor;
+    const float lineSpacing = getFont().getLineSpacing(m_characterSize) * m_lineSpacingFactor;
 
     // Compute the position
     Vector2f      position;
@@ -302,7 +337,7 @@ Vector2f Text::findCharacterPos(std::size_t index) const
         const std::uint32_t curChar = m_string[i];
 
         // Apply the kerning offset
-        position.x += m_font->getKerning(prevChar, curChar, m_characterSize, isBold);
+        position.x += getFont().getKerning(prevChar, curChar, m_characterSize, isBold);
         prevChar = curChar;
 
         // Handle special characters
@@ -321,7 +356,7 @@ Vector2f Text::findCharacterPos(std::size_t index) const
         }
 
         // For regular characters, add the advance offset of the glyph
-        position.x += m_font->getGlyph(curChar, m_characterSize, isBold).advance + letterSpacing;
+        position.x += getFont().getGlyph(curChar, m_characterSize, isBold).advance + letterSpacing;
     }
 
     // Transform the position to global coordinates
@@ -355,7 +390,7 @@ void Text::draw(RenderTarget& target, const RenderStates& states) const
     RenderStates statesCopy(states);
 
     statesCopy.transform *= getTransform();
-    statesCopy.texture = &m_font->getTexture(m_characterSize);
+    statesCopy.texture = &getFont().getTexture(m_characterSize);
 
     // Only draw the outline if there is something to draw
     if (m_outlineThickness != 0)
@@ -369,11 +404,11 @@ void Text::draw(RenderTarget& target, const RenderStates& states) const
 void Text::ensureGeometryUpdate() const
 {
     // Do nothing, if geometry has not changed and the font texture has not changed
-    if (!m_geometryNeedUpdate && m_font->getTexture(m_characterSize).m_cacheId == m_fontTextureId)
+    if (!m_geometryNeedUpdate && getFont().getTexture(m_characterSize).m_cacheId == m_fontTextureId)
         return;
 
     // Save the current fonts texture id
-    m_fontTextureId = m_font->getTexture(m_characterSize).m_cacheId;
+    m_fontTextureId = getFont().getTexture(m_characterSize).m_cacheId;
 
     // Mark geometry as updated
     m_geometryNeedUpdate = false;
@@ -392,20 +427,20 @@ void Text::ensureGeometryUpdate() const
     const bool  isUnderlined       = m_style & Underlined;
     const bool  isStrikeThrough    = m_style & StrikeThrough;
     const float italicShear        = (m_style & Italic) ? sf::degrees(12).asRadians() : 0.f;
-    const float underlineOffset    = m_font->getUnderlinePosition(m_characterSize);
+    const float underlineOffset    = getFont().getUnderlinePosition(m_characterSize);
-    const float underlineThickness = m_font->getUnderlineThickness(m_characterSize);
+    const float underlineThickness = getFont().getUnderlineThickness(m_characterSize);
 
     // Compute the location of the strike through dynamically
     // We use the center point of the lowercase 'x' glyph as the reference
     // We reuse the underline thickness as the thickness of the strike through as well
-    const FloatRect xBounds             = m_font->getGlyph(U'x', m_characterSize, isBold).bounds;
+    const FloatRect xBounds             = getFont().getGlyph(U'x', m_characterSize, isBold).bounds;
     const float     strikeThroughOffset = xBounds.top + xBounds.height / 2.f;
 
     // Precompute the variables needed by the algorithm
-    float       whitespaceWidth = m_font->getGlyph(U' ', m_characterSize, isBold).advance;
+    float       whitespaceWidth = getFont().getGlyph(U' ', m_characterSize, isBold).advance;
     const float letterSpacing   = (whitespaceWidth / 3.f) * (m_letterSpacingFactor - 1.f);
     whitespaceWidth += letterSpacing;
-    const float lineSpacing = m_font->getLineSpacing(m_characterSize) * m_lineSpacingFactor;
+    const float lineSpacing = getFont().getLineSpacing(m_characterSize) * m_lineSpacingFactor;
     float       x           = 0.f;
     auto        y           = static_cast<float>(m_characterSize);
 
@@ -424,7 +459,7 @@ void Text::ensureGeometryUpdate() const
             continue;
 
         // Apply the kerning offset
-        x += m_font->getKerning(prevChar, curChar, m_characterSize, isBold);
+        x += getFont().getKerning(prevChar, curChar, m_characterSize, isBold);
 
         // If we're using the underlined style and there's a new line, draw a line
         if (isUnderlined && (curChar == U'\n' && prevChar != U'\n'))
@@ -478,14 +513,14 @@ void Text::ensureGeometryUpdate() const
         // Apply the outline
         if (m_outlineThickness != 0)
         {
-            const Glyph& glyph = m_font->getGlyph(curChar, m_characterSize, isBold, m_outlineThickness);
+            const Glyph& glyph = getFont().getGlyph(curChar, m_characterSize, isBold, m_outlineThickness);
 
             // Add the outline glyph to the vertices
             addGlyphQuad(m_outlineVertices, Vector2f(x, y), m_outlineColor, glyph, italicShear);
         }
 
         // Extract the current glyph's description
-        const Glyph& glyph = m_font->getGlyph(curChar, m_characterSize, isBold);
+        const Glyph& glyph = getFont().getGlyph(curChar, m_characterSize, isBold);
 
         // Add the glyph to the vertices
         addGlyphQuad(m_vertices, Vector2f(x, y), m_fillColor, glyph, italicShear);

test/Graphics/Sprite.test.cpp

@@ -20,36 +20,72 @@ TEST_CASE("[Graphics] sf::Sprite", runDisplayTests())
     }
 
     const sf::Texture texture;
+    const auto        texturePtr = std::make_shared<const sf::Texture>();
 
     SECTION("Construction")
     {
         SECTION("Texture constructor")
         {
-            const sf::Sprite sprite(texture);
+            SECTION("const Texture&")
-            CHECK(&sprite.getTexture() == &texture);
+            {
-            CHECK(sprite.getTextureRect() == sf::IntRect());
+                const sf::Sprite sprite(texture);
-            CHECK(sprite.getColor() == sf::Color::White);
+                CHECK(&sprite.getTexture() == &texture);
-            CHECK(sprite.getLocalBounds() == sf::FloatRect());
+                CHECK(sprite.getTextureRect() == sf::IntRect());
-            CHECK(sprite.getGlobalBounds() == sf::FloatRect());
+                CHECK(sprite.getColor() == sf::Color::White);
+                CHECK(sprite.getLocalBounds() == sf::FloatRect());
+                CHECK(sprite.getGlobalBounds() == sf::FloatRect());
+            }
+
+            SECTION("std::shared_ptr<const Texture>")
+            {
+                const sf::Sprite sprite(texturePtr);
+                CHECK(&sprite.getTexture() == texturePtr.get());
+                CHECK(sprite.getTextureRect() == sf::IntRect());
+                CHECK(sprite.getColor() == sf::Color::White);
+                CHECK(sprite.getLocalBounds() == sf::FloatRect());
+                CHECK(sprite.getGlobalBounds() == sf::FloatRect());
+            }
         }
 
         SECTION("Texture and rectangle constructor")
         {
-            const sf::Sprite sprite(texture, {{0, 0}, {40, 60}});
+            SECTION("const Texture&")
-            CHECK(&sprite.getTexture() == &texture);
+            {
-            CHECK(sprite.getTextureRect() == sf::IntRect({0, 0}, {40, 60}));
+                const sf::Sprite sprite(texture, {{0, 0}, {40, 60}});
-            CHECK(sprite.getColor() == sf::Color::White);
+                CHECK(&sprite.getTexture() == &texture);
-            CHECK(sprite.getLocalBounds() == sf::FloatRect({0, 0}, {40, 60}));
+                CHECK(sprite.getTextureRect() == sf::IntRect({0, 0}, {40, 60}));
-            CHECK(sprite.getGlobalBounds() == sf::FloatRect({0, 0}, {40, 60}));
+                CHECK(sprite.getColor() == sf::Color::White);
+                CHECK(sprite.getLocalBounds() == sf::FloatRect({0, 0}, {40, 60}));
+                CHECK(sprite.getGlobalBounds() == sf::FloatRect({0, 0}, {40, 60}));
+            }
+
+            SECTION("std::shared_ptr<const Texture>")
+            {
+                const sf::Sprite sprite(texturePtr, {{0, 0}, {40, 60}});
+                CHECK(&sprite.getTexture() == texturePtr.get());
+                CHECK(sprite.getTextureRect() == sf::IntRect({0, 0}, {40, 60}));
+                CHECK(sprite.getColor() == sf::Color::White);
+                CHECK(sprite.getLocalBounds() == sf::FloatRect({0, 0}, {40, 60}));
+                CHECK(sprite.getGlobalBounds() == sf::FloatRect({0, 0}, {40, 60}));
+            }
         }
     }
 
     SECTION("Set/get texture")
     {
-        sf::Sprite        sprite(texture);
+        SECTION("const Texture&")
-        const sf::Texture otherTexture;
+        {
-        sprite.setTexture(otherTexture);
+            sf::Sprite sprite(texturePtr);
-        CHECK(&sprite.getTexture() == &otherTexture);
+            sprite.setTexture(texture);
+            CHECK(&sprite.getTexture() == &texture);
+        }
+
+        SECTION("std::shared_ptr<const Texture>")
+        {
+            sf::Sprite sprite(texture);
+            sprite.setTexture(texturePtr);
+            CHECK(&sprite.getTexture() == texturePtr.get());
+        }
     }
 
     SECTION("Set/get texture rect")

test/Graphics/Text.test.cpp

@@ -19,7 +19,8 @@ TEST_CASE("[Graphics] sf::Text", runDisplayTests())
         STATIC_CHECK(std::is_nothrow_move_assignable_v<sf::Text>);
     }
 
-    sf::Font font;
+    const auto fontPtr = std::make_shared<const sf::Font>();
+    sf::Font   font;
     REQUIRE(font.loadFromFile("Graphics/tuffy.ttf"));
 
     SECTION("Construction")
@@ -78,19 +79,39 @@ TEST_CASE("[Graphics] sf::Text", runDisplayTests())
 
         SECTION("Font, string, and character size constructor")
         {
-            const sf::Text text(font, "abcdefghijklmnopqrstuvwxyz", 24);
+            SECTION("const Font&")
-            CHECK(text.getString() == "abcdefghijklmnopqrstuvwxyz");
+            {
-            CHECK(&text.getFont() == &font);
+                const sf::Text text(font, "abcdefghijklmnopqrstuvwxyz", 24);
-            CHECK(text.getCharacterSize() == 24);
+                CHECK(text.getString() == "abcdefghijklmnopqrstuvwxyz");
-            CHECK(text.getLetterSpacing() == 1.f);
+                CHECK(&text.getFont() == &font);
-            CHECK(text.getLineSpacing() == 1.f);
+                CHECK(text.getCharacterSize() == 24);
-            CHECK(text.getStyle() == sf::Text::Regular);
+                CHECK(text.getLetterSpacing() == 1.f);
-            CHECK(text.getFillColor() == sf::Color::White);
+                CHECK(text.getLineSpacing() == 1.f);
-            CHECK(text.getOutlineColor() == sf::Color::Black);
+                CHECK(text.getStyle() == sf::Text::Regular);
-            CHECK(text.getOutlineThickness() == 0);
+                CHECK(text.getFillColor() == sf::Color::White);
-            CHECK(text.findCharacterPos(0) == sf::Vector2f());
+                CHECK(text.getOutlineColor() == sf::Color::Black);
-            CHECK(text.getLocalBounds() == sf::FloatRect({1, 7}, {290, 22}));
+                CHECK(text.getOutlineThickness() == 0);
-            CHECK(text.getGlobalBounds() == sf::FloatRect({1, 7}, {290, 22}));
+                CHECK(text.findCharacterPos(0) == sf::Vector2f());
+                CHECK(text.getLocalBounds() == sf::FloatRect({1, 7}, {290, 22}));
+                CHECK(text.getGlobalBounds() == sf::FloatRect({1, 7}, {290, 22}));
+            }
+
+            SECTION("std::shared_ptr<const Font>")
+            {
+                const sf::Text text(fontPtr, "abcdefghijklmnopqrstuvwxyz", 24);
+                CHECK(text.getString() == "abcdefghijklmnopqrstuvwxyz");
+                CHECK(&text.getFont() == fontPtr.get());
+                CHECK(text.getCharacterSize() == 24);
+                CHECK(text.getLetterSpacing() == 1.f);
+                CHECK(text.getLineSpacing() == 1.f);
+                CHECK(text.getStyle() == sf::Text::Regular);
+                CHECK(text.getFillColor() == sf::Color::White);
+                CHECK(text.getOutlineColor() == sf::Color::Black);
+                CHECK(text.getOutlineThickness() == 0);
+                CHECK(text.findCharacterPos(0) == sf::Vector2f());
+                CHECK(text.getLocalBounds() == sf::FloatRect({0, 24}, {0, 0}));
+                CHECK(text.getGlobalBounds() == sf::FloatRect({0, 24}, {0, 0}));
+            }
         }
     }
 
@@ -103,10 +124,19 @@ TEST_CASE("[Graphics] sf::Text", runDisplayTests())
 
     SECTION("Set/get font")
     {
-        sf::Text       text(font);
+        SECTION("const Font&")
-        const sf::Font otherFont;
+        {
-        text.setFont(otherFont);
+            sf::Text text(fontPtr);
-        CHECK(&text.getFont() == &otherFont);
+            text.setFont(font);
+            CHECK(&text.getFont() == &font);
+        }
+
+        SECTION("std::shared_ptr<const Font>")
+        {
+            sf::Text text(font);
+            text.setFont(fontPtr);
+            CHECK(&text.getFont() == fontPtr.get());
+        }
     }
 
     SECTION("Set/get character size")