From 332d11be41eebcd7c7cc32b1579afa4e6ea7aea6 Mon Sep 17 00:00:00 2001 From: Chris Thrasher Date: Tue, 6 Jun 2023 16:12:44 -0600 Subject: [PATCH] Add move semantics to `sf::RenderTarget` and `sf::RenderTexture` --- include/SFML/Graphics/RenderTarget.hpp | 24 +++++++++++++++++++++--- include/SFML/Graphics/RenderTexture.hpp | 24 ++++++++++++++++++++++++ src/SFML/Graphics/RenderTarget.cpp | 8 -------- src/SFML/Graphics/RenderTexture.cpp | 8 ++++++++ test/Graphics/RenderTarget.test.cpp | 6 +++++- test/Graphics/RenderTexture.test.cpp | 4 ++-- 6 files changed, 60 insertions(+), 14 deletions(-) diff --git a/include/SFML/Graphics/RenderTarget.hpp b/include/SFML/Graphics/RenderTarget.hpp index 4191ab143..d39ca9b5e 100644 --- a/include/SFML/Graphics/RenderTarget.hpp +++ b/include/SFML/Graphics/RenderTarget.hpp @@ -57,7 +57,7 @@ public: /// \brief Destructor /// //////////////////////////////////////////////////////////// - virtual ~RenderTarget(); + virtual ~RenderTarget() = default; //////////////////////////////////////////////////////////// /// \brief Deleted copy constructor @@ -71,6 +71,18 @@ public: //////////////////////////////////////////////////////////// RenderTarget& operator=(const RenderTarget&) = delete; + //////////////////////////////////////////////////////////// + /// \brief Move constructor + /// + //////////////////////////////////////////////////////////// + RenderTarget(RenderTarget&&) noexcept = default; + + //////////////////////////////////////////////////////////// + /// \brief Move assignment + /// + //////////////////////////////////////////////////////////// + RenderTarget& operator=(RenderTarget&&) noexcept = default; + //////////////////////////////////////////////////////////// /// \brief Clear the entire target with a single color /// @@ -396,7 +408,7 @@ protected: /// \brief Default constructor /// //////////////////////////////////////////////////////////// - RenderTarget(); + RenderTarget() = default; //////////////////////////////////////////////////////////// /// \brief Performs the common initialization step after creation @@ -496,7 +508,7 @@ private: //////////////////////////////////////////////////////////// View m_defaultView; //!< Default view View m_view; //!< Current view - StatesCache m_cache; //!< Render states cache + StatesCache m_cache{}; //!< Render states cache std::uint64_t m_id{}; //!< Unique number that identifies the RenderTarget }; @@ -525,6 +537,12 @@ private: /// OpenGL states are not messed up by calling the /// pushGLStates/popGLStates functions. /// +/// While render targets are moveable, it is not valid to move them +/// between threads. This will cause your program to crash. The +/// problem boils down to OpenGL being limited with regard to how it +/// works in multithreaded environments. Please ensure you only move +/// render targets within the same thread. +/// /// \see sf::RenderWindow, sf::RenderTexture, sf::View /// //////////////////////////////////////////////////////////// diff --git a/include/SFML/Graphics/RenderTexture.hpp b/include/SFML/Graphics/RenderTexture.hpp index ab0f93235..e0750cd18 100644 --- a/include/SFML/Graphics/RenderTexture.hpp +++ b/include/SFML/Graphics/RenderTexture.hpp @@ -68,6 +68,30 @@ public: //////////////////////////////////////////////////////////// ~RenderTexture() override; + //////////////////////////////////////////////////////////// + /// \brief Deleted copy constructor + /// + //////////////////////////////////////////////////////////// + RenderTexture(const RenderTexture&) = delete; + + //////////////////////////////////////////////////////////// + /// \brief Deleted copy assignment + /// + //////////////////////////////////////////////////////////// + RenderTexture& operator=(const RenderTexture&) = delete; + + //////////////////////////////////////////////////////////// + /// \brief Move constructor + /// + //////////////////////////////////////////////////////////// + RenderTexture(RenderTexture&&) noexcept; + + //////////////////////////////////////////////////////////// + /// \brief Move assignment operator + /// + //////////////////////////////////////////////////////////// + RenderTexture& operator=(RenderTexture&&) noexcept; + //////////////////////////////////////////////////////////// /// \brief Create the render-texture /// diff --git a/src/SFML/Graphics/RenderTarget.cpp b/src/SFML/Graphics/RenderTarget.cpp index 726f6e3bb..36c1f765c 100644 --- a/src/SFML/Graphics/RenderTarget.cpp +++ b/src/SFML/Graphics/RenderTarget.cpp @@ -153,14 +153,6 @@ std::uint32_t equationToGlConstant(sf::BlendMode::Equation blendEquation) namespace sf { -//////////////////////////////////////////////////////////// -RenderTarget::RenderTarget() = default; - - -//////////////////////////////////////////////////////////// -RenderTarget::~RenderTarget() = default; - - //////////////////////////////////////////////////////////// void RenderTarget::clear(const Color& color) { diff --git a/src/SFML/Graphics/RenderTexture.cpp b/src/SFML/Graphics/RenderTexture.cpp index 65c67185c..416752838 100644 --- a/src/SFML/Graphics/RenderTexture.cpp +++ b/src/SFML/Graphics/RenderTexture.cpp @@ -45,6 +45,14 @@ RenderTexture::RenderTexture() = default; RenderTexture::~RenderTexture() = default; +//////////////////////////////////////////////////////////// +RenderTexture::RenderTexture(RenderTexture&&) noexcept = default; + + +//////////////////////////////////////////////////////////// +RenderTexture& RenderTexture::operator=(RenderTexture&&) noexcept = default; + + //////////////////////////////////////////////////////////// bool RenderTexture::create(const Vector2u& size, const ContextSettings& settings) { diff --git a/test/Graphics/RenderTarget.test.cpp b/test/Graphics/RenderTarget.test.cpp index 2fabdb8df..c2c3943cb 100644 --- a/test/Graphics/RenderTarget.test.cpp +++ b/test/Graphics/RenderTarget.test.cpp @@ -8,6 +8,10 @@ class RenderTarget : public sf::RenderTarget { +public: + RenderTarget() = default; + +private: sf::Vector2u getSize() const override { return {640, 480}; @@ -22,7 +26,7 @@ TEST_CASE("[Graphics] sf::RenderTarget") STATIC_CHECK(!std::is_copy_constructible_v); STATIC_CHECK(!std::is_copy_assignable_v); STATIC_CHECK(!std::is_nothrow_move_constructible_v); - STATIC_CHECK(!std::is_nothrow_move_assignable_v); + STATIC_CHECK(std::is_nothrow_move_assignable_v); } SECTION("Construction") diff --git a/test/Graphics/RenderTexture.test.cpp b/test/Graphics/RenderTexture.test.cpp index 50d7b7917..bce664f72 100644 --- a/test/Graphics/RenderTexture.test.cpp +++ b/test/Graphics/RenderTexture.test.cpp @@ -11,8 +11,8 @@ TEST_CASE("[Graphics] sf::RenderTexture", runDisplayTests()) { STATIC_CHECK(!std::is_copy_constructible_v); STATIC_CHECK(!std::is_copy_assignable_v); - STATIC_CHECK(!std::is_nothrow_move_constructible_v); - STATIC_CHECK(!std::is_nothrow_move_assignable_v); + STATIC_CHECK(std::is_nothrow_move_constructible_v); + STATIC_CHECK(std::is_nothrow_move_assignable_v); } SECTION("Construction")