From 8da027c56f383abfb140d90437b0de09ad7195ef Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Wed, 13 Jan 2016 12:15:57 -0800 Subject: [PATCH] Encapsulate GPUObject in a safer way --- libraries/gpu/src/gpu/Context.h | 66 +++-------------------------- libraries/gpu/src/gpu/Format.h | 19 ++++++++- libraries/gpu/src/gpu/Framebuffer.h | 8 +--- libraries/gpu/src/gpu/Pipeline.h | 8 +--- libraries/gpu/src/gpu/Query.h | 10 +---- libraries/gpu/src/gpu/Resource.cpp | 13 ++---- libraries/gpu/src/gpu/Resource.h | 9 +--- libraries/gpu/src/gpu/Shader.h | 8 +--- libraries/gpu/src/gpu/State.h | 8 +--- libraries/gpu/src/gpu/Texture.h | 9 +--- 10 files changed, 34 insertions(+), 124 deletions(-) diff --git a/libraries/gpu/src/gpu/Context.h b/libraries/gpu/src/gpu/Context.h index ad21d71427..260aff72d7 100644 --- a/libraries/gpu/src/gpu/Context.h +++ b/libraries/gpu/src/gpu/Context.h @@ -97,68 +97,16 @@ public: TransformCamera getEyeCamera(int eye, const StereoState& stereo) const; }; - template< typename T > - static void setGPUObject(const Buffer& buffer, T* object) { - buffer.setGPUObject(object); + + template + static void setGPUObject(const GPUObjectWrapper& wrapper, T* object) { + wrapper.setGPUObject(object); } - template< typename T > - static T* getGPUObject(const Buffer& buffer) { - return reinterpret_cast(buffer.getGPUObject()); + template + static T* getGPUObject(const GPUObjectWrapper& wrapper) { + return reinterpret_cast(wrapper.getGPUObject()); } - template< typename T > - static void setGPUObject(const Texture& texture, T* object) { - texture.setGPUObject(object); - } - template< typename T > - static T* getGPUObject(const Texture& texture) { - return reinterpret_cast(texture.getGPUObject()); - } - - template< typename T > - static void setGPUObject(const Shader& shader, T* object) { - shader.setGPUObject(object); - } - template< typename T > - static T* getGPUObject(const Shader& shader) { - return reinterpret_cast(shader.getGPUObject()); - } - - template< typename T > - static void setGPUObject(const Pipeline& pipeline, T* object) { - pipeline.setGPUObject(object); - } - template< typename T > - static T* getGPUObject(const Pipeline& pipeline) { - return reinterpret_cast(pipeline.getGPUObject()); - } - - template< typename T > - static void setGPUObject(const State& state, T* object) { - state.setGPUObject(object); - } - template< typename T > - static T* getGPUObject(const State& state) { - return reinterpret_cast(state.getGPUObject()); - } - - template< typename T > - static void setGPUObject(const Framebuffer& framebuffer, T* object) { - framebuffer.setGPUObject(object); - } - template< typename T > - static T* getGPUObject(const Framebuffer& framebuffer) { - return reinterpret_cast(framebuffer.getGPUObject()); - } - - template< typename T > - static void setGPUObject(const Query& query, T* object) { - query.setGPUObject(object); - } - template< typename T > - static T* getGPUObject(const Query& query) { - return reinterpret_cast(query.getGPUObject()); - } protected: StereoState _stereo; diff --git a/libraries/gpu/src/gpu/Format.h b/libraries/gpu/src/gpu/Format.h index 41a95e2578..1ae579d9f5 100644 --- a/libraries/gpu/src/gpu/Format.h +++ b/libraries/gpu/src/gpu/Format.h @@ -16,10 +16,25 @@ namespace gpu { +class Backend; + class GPUObject { public: - GPUObject() {} - virtual ~GPUObject() {} + virtual ~GPUObject() = default; +}; + +class GPUObjectWrapper { +public: + virtual ~GPUObjectWrapper() { delete _gpuObject; } + +private: + // This shouldn't be used by anything else than the Backend class with the proper casting. + // TODO: Consider using std::unique_ptr to get rid of dtor and ensure correct destruction of GPU objects + mutable GPUObject* _gpuObject { nullptr }; + void setGPUObject(GPUObject* gpuObject) const { _gpuObject = gpuObject; } + GPUObject* getGPUObject() const { return _gpuObject; } + + friend class Backend; }; typedef int Stamp; diff --git a/libraries/gpu/src/gpu/Framebuffer.h b/libraries/gpu/src/gpu/Framebuffer.h index 807047a56e..5fbc2e1487 100755 --- a/libraries/gpu/src/gpu/Framebuffer.h +++ b/libraries/gpu/src/gpu/Framebuffer.h @@ -64,7 +64,7 @@ protected: typedef std::shared_ptr SwapchainPointer; -class Framebuffer { +class Framebuffer : public GPUObjectWrapper { public: enum BufferMask { BUFFER_COLOR0 = 1, @@ -153,12 +153,6 @@ protected: // Non exposed Framebuffer(const Framebuffer& framebuffer) = delete; Framebuffer() {} - - // This shouldn't be used by anything else than the Backend class with the proper casting. - mutable GPUObject* _gpuObject = NULL; - void setGPUObject(GPUObject* gpuObject) const { _gpuObject = gpuObject; } - GPUObject* getGPUObject() const { return _gpuObject; } - friend class Backend; }; typedef std::shared_ptr FramebufferPointer; diff --git a/libraries/gpu/src/gpu/Pipeline.h b/libraries/gpu/src/gpu/Pipeline.h index adc65a0c66..f062873869 100755 --- a/libraries/gpu/src/gpu/Pipeline.h +++ b/libraries/gpu/src/gpu/Pipeline.h @@ -20,7 +20,7 @@ namespace gpu { -class Pipeline { +class Pipeline : public GPUObjectWrapper { public: using Pointer = std::shared_ptr< Pipeline >; @@ -38,12 +38,6 @@ protected: Pipeline(); Pipeline(const Pipeline& pipeline); // deep copy of the sysmem shader Pipeline& operator=(const Pipeline& pipeline); // deep copy of the sysmem texture - - // This shouldn't be used by anything else than the Backend class with the proper casting. - mutable GPUObject* _gpuObject = nullptr; - void setGPUObject(GPUObject* gpuObject) const { _gpuObject = gpuObject; } - GPUObject* getGPUObject() const { return _gpuObject; } - friend class Backend; }; typedef Pipeline::Pointer PipelinePointer; diff --git a/libraries/gpu/src/gpu/Query.h b/libraries/gpu/src/gpu/Query.h index d9c3185d9e..82f17c71ae 100644 --- a/libraries/gpu/src/gpu/Query.h +++ b/libraries/gpu/src/gpu/Query.h @@ -19,7 +19,7 @@ namespace gpu { - class Query { + class Query : public GPUObjectWrapper { public: Query(); ~Query(); @@ -27,14 +27,6 @@ namespace gpu { uint32 queryResult; double getElapsedTime(); - - protected: - - // This shouldn't be used by anything else than the Backend class with the proper casting. - mutable GPUObject* _gpuObject = NULL; - void setGPUObject(GPUObject* gpuObject) const { _gpuObject = gpuObject; } - GPUObject* getGPUObject() const { return _gpuObject; } - friend class Backend; }; typedef std::shared_ptr QueryPointer; diff --git a/libraries/gpu/src/gpu/Resource.cpp b/libraries/gpu/src/gpu/Resource.cpp index c162b38b93..1788f6ece5 100644 --- a/libraries/gpu/src/gpu/Resource.cpp +++ b/libraries/gpu/src/gpu/Resource.cpp @@ -170,20 +170,17 @@ Resource::Size Resource::Sysmem::append(Size size, const Byte* bytes) { Buffer::Buffer() : Resource(), - _sysmem(new Sysmem()), - _gpuObject(NULL) { + _sysmem(new Sysmem()) { } Buffer::Buffer(Size size, const Byte* bytes) : Resource(), - _sysmem(new Sysmem(size, bytes)), - _gpuObject(NULL) { + _sysmem(new Sysmem(size, bytes)) { } Buffer::Buffer(const Buffer& buf) : Resource(), - _sysmem(new Sysmem(buf.getSysmem())), - _gpuObject(NULL) { + _sysmem(new Sysmem(buf.getSysmem())) { } Buffer& Buffer::operator=(const Buffer& buf) { @@ -196,10 +193,6 @@ Buffer::~Buffer() { delete _sysmem; _sysmem = NULL; } - if (_gpuObject) { - delete _gpuObject; - _gpuObject = NULL; - } } Buffer::Size Buffer::resize(Size size) { diff --git a/libraries/gpu/src/gpu/Resource.h b/libraries/gpu/src/gpu/Resource.h index 794ee680f4..817deaae86 100644 --- a/libraries/gpu/src/gpu/Resource.h +++ b/libraries/gpu/src/gpu/Resource.h @@ -108,7 +108,7 @@ protected: }; -class Buffer : public Resource { +class Buffer : public Resource, public GPUObjectWrapper { public: Buffer(); @@ -156,13 +156,6 @@ public: protected: Sysmem* _sysmem = NULL; - - - // This shouldn't be used by anything else than the Backend class with the proper casting. - mutable GPUObject* _gpuObject = NULL; - void setGPUObject(GPUObject* gpuObject) const { _gpuObject = gpuObject; } - GPUObject* getGPUObject() const { return _gpuObject; } - friend class Backend; }; typedef std::shared_ptr BufferPointer; diff --git a/libraries/gpu/src/gpu/Shader.h b/libraries/gpu/src/gpu/Shader.h index b737a42e12..908e57e06b 100755 --- a/libraries/gpu/src/gpu/Shader.h +++ b/libraries/gpu/src/gpu/Shader.h @@ -20,7 +20,7 @@ namespace gpu { -class Shader { +class Shader : public GPUObjectWrapper { public: typedef std::shared_ptr< Shader > Pointer; @@ -178,12 +178,6 @@ protected: // The type of the shader, the master key Type _type; - - // This shouldn't be used by anything else than the Backend class with the proper casting. - mutable GPUObject* _gpuObject = NULL; - void setGPUObject(GPUObject* gpuObject) const { _gpuObject = gpuObject; } - GPUObject* getGPUObject() const { return _gpuObject; } - friend class Backend; }; typedef Shader::Pointer ShaderPointer; diff --git a/libraries/gpu/src/gpu/State.h b/libraries/gpu/src/gpu/State.h index 7740506bce..e468b01171 100755 --- a/libraries/gpu/src/gpu/State.h +++ b/libraries/gpu/src/gpu/State.h @@ -36,7 +36,7 @@ namespace gpu { class GPUObject; -class State { +class State : public GPUObjectWrapper { public: State(); virtual ~State(); @@ -392,12 +392,6 @@ protected: Data _values; Signature _signature{0}; Stamp _stamp{0}; - - // This shouldn't be used by anything else than the Backend class with the proper casting. - mutable GPUObject* _gpuObject = nullptr; - void setGPUObject(GPUObject* gpuObject) const { _gpuObject = gpuObject; } - GPUObject* getGPUObject() const { return _gpuObject; } - friend class Backend; }; typedef std::shared_ptr< State > StatePointer; diff --git a/libraries/gpu/src/gpu/Texture.h b/libraries/gpu/src/gpu/Texture.h index 6e8eb10380..e68b65a15e 100755 --- a/libraries/gpu/src/gpu/Texture.h +++ b/libraries/gpu/src/gpu/Texture.h @@ -136,7 +136,7 @@ protected: Desc _desc; }; -class Texture : public Resource { +class Texture : public Resource, public GPUObjectWrapper { public: class Pixels { @@ -386,13 +386,6 @@ protected: static Texture* create(Type type, const Element& texelFormat, uint16 width, uint16 height, uint16 depth, uint16 numSamples, uint16 numSlices, const Sampler& sampler); Size resize(Type type, const Element& texelFormat, uint16 width, uint16 height, uint16 depth, uint16 numSamples, uint16 numSlices); - - // This shouldn't be used by anything else than the Backend class with the proper casting. - mutable GPUObject* _gpuObject = NULL; - void setGPUObject(GPUObject* gpuObject) const { _gpuObject = gpuObject; } - GPUObject* getGPUObject() const { return _gpuObject; } - - friend class Backend; }; typedef std::shared_ptr TexturePointer;