From 574737fbb56464d537c36c1926a538b1fff4bd58 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Thu, 11 Aug 2016 16:09:21 -0700 Subject: [PATCH] More GPU api fixes, protect Buffer::flush --- .../display-plugins/hmd/HmdDisplayPlugin.cpp | 3 - libraries/gpu-gl/src/gpu/gl/GLBackend.h | 1 - .../gpu-gl/src/gpu/gl41/GL41BackendBuffer.cpp | 87 ++++++++++--------- .../gpu-gl/src/gpu/gl45/GL45BackendBuffer.cpp | 63 +++++++------- libraries/gpu/src/gpu/Buffer.h | 13 ++- libraries/gpu/src/gpu/Forward.h | 3 + libraries/render-utils/src/GeometryCache.cpp | 3 - 7 files changed, 92 insertions(+), 81 deletions(-) diff --git a/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp index 90cf847a3b..c916fcafe2 100644 --- a/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp @@ -362,7 +362,6 @@ void HmdDisplayPlugin::OverlayRenderer::build() { vertices->append(sizeof(Vertex), (gpu::Byte*)&vertex); } } - vertices->flush(); // Compute number of indices needed static const int VERTEX_PER_TRANGLE = 3; @@ -389,7 +388,6 @@ void HmdDisplayPlugin::OverlayRenderer::build() { } } this->indices->append(indices); - this->indices->flush(); format = std::make_shared(); // 1 for everyone format->setAttribute(gpu::Stream::POSITION, gpu::Stream::POSITION, gpu::Element(gpu::VEC3, gpu::FLOAT, gpu::XYZ), 0); format->setAttribute(gpu::Stream::TEXCOORD, gpu::Stream::TEXCOORD, gpu::Element(gpu::VEC2, gpu::FLOAT, gpu::UV)); @@ -438,7 +436,6 @@ void HmdDisplayPlugin::OverlayRenderer::render(HmdDisplayPlugin& plugin) { for_each_eye([&](Eye eye){ uniforms.mvp = mvps[eye]; uniformBuffers[eye]->setSubData(0, uniforms); - uniformBuffers[eye]->flush(); }); plugin.render([&](gpu::Batch& batch) { batch.enableStereo(false); diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackend.h b/libraries/gpu-gl/src/gpu/gl/GLBackend.h index 4519d38eec..080c05829f 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackend.h +++ b/libraries/gpu-gl/src/gpu/gl/GLBackend.h @@ -342,7 +342,6 @@ protected: PipelineStageState() { _cameraCorrectionBuffer.edit() = CameraCorrection(); - _cameraCorrectionBuffer._buffer->flush(); } } _pipeline; diff --git a/libraries/gpu-gl/src/gpu/gl41/GL41BackendBuffer.cpp b/libraries/gpu-gl/src/gpu/gl41/GL41BackendBuffer.cpp index c7bc99d34e..d9d7328bc8 100644 --- a/libraries/gpu-gl/src/gpu/gl41/GL41BackendBuffer.cpp +++ b/libraries/gpu-gl/src/gpu/gl41/GL41BackendBuffer.cpp @@ -8,50 +8,55 @@ #include "GL41Backend.h" #include "../gl/GLBuffer.h" +namespace gpu { + namespace gl41 { + class GL41Buffer : public gl::GLBuffer { + using Parent = gpu::gl::GLBuffer; + static GLuint allocate() { + GLuint result; + glGenBuffers(1, &result); + return result; + } + + public: + GL41Buffer(const std::weak_ptr& backend, const Buffer& buffer, GL41Buffer* original) : Parent(backend, buffer, allocate()) { + glBindBuffer(GL_ARRAY_BUFFER, _buffer); + glBufferData(GL_ARRAY_BUFFER, _size, nullptr, GL_DYNAMIC_DRAW); + glBindBuffer(GL_ARRAY_BUFFER, 0); + + if (original && original->_size) { + glBindBuffer(GL_COPY_WRITE_BUFFER, _buffer); + glBindBuffer(GL_COPY_READ_BUFFER, original->_buffer); + glCopyBufferSubData(GL_COPY_READ_BUFFER, GL_COPY_WRITE_BUFFER, 0, 0, original->_size); + glBindBuffer(GL_COPY_WRITE_BUFFER, 0); + glBindBuffer(GL_COPY_READ_BUFFER, 0); + (void)CHECK_GL_ERROR(); + } + Backend::setGPUObject(buffer, this); + } + + void transfer() override { + glBindBuffer(GL_ARRAY_BUFFER, _buffer); + (void)CHECK_GL_ERROR(); + Size offset; + Size size; + Size currentPage { 0 }; + auto data = _gpuObject._renderSysmem.readData(); + while (_gpuObject._renderPages.getNextTransferBlock(offset, size, currentPage)) { + glBufferSubData(GL_ARRAY_BUFFER, offset, size, data + offset); + (void)CHECK_GL_ERROR(); + } + glBindBuffer(GL_ARRAY_BUFFER, 0); + (void)CHECK_GL_ERROR(); + _gpuObject._renderPages._flags &= ~PageManager::DIRTY; + } + }; + } +} + using namespace gpu; using namespace gpu::gl41; -class GL41Buffer : public gl::GLBuffer { - using Parent = gpu::gl::GLBuffer; - static GLuint allocate() { - GLuint result; - glGenBuffers(1, &result); - return result; - } - -public: - GL41Buffer(const std::weak_ptr& backend, const Buffer& buffer, GL41Buffer* original) : Parent(backend, buffer, allocate()) { - glBindBuffer(GL_ARRAY_BUFFER, _buffer); - glBufferData(GL_ARRAY_BUFFER, _size, nullptr, GL_DYNAMIC_DRAW); - glBindBuffer(GL_ARRAY_BUFFER, 0); - - if (original && original->_size) { - glBindBuffer(GL_COPY_WRITE_BUFFER, _buffer); - glBindBuffer(GL_COPY_READ_BUFFER, original->_buffer); - glCopyBufferSubData(GL_COPY_READ_BUFFER, GL_COPY_WRITE_BUFFER, 0, 0, original->_size); - glBindBuffer(GL_COPY_WRITE_BUFFER, 0); - glBindBuffer(GL_COPY_READ_BUFFER, 0); - (void)CHECK_GL_ERROR(); - } - Backend::setGPUObject(buffer, this); - } - - void transfer() override { - glBindBuffer(GL_ARRAY_BUFFER, _buffer); - (void)CHECK_GL_ERROR(); - Size offset; - Size size; - Size currentPage { 0 }; - auto data = _gpuObject._renderSysmem.readData(); - while (_gpuObject._renderPages.getNextTransferBlock(offset, size, currentPage)) { - glBufferSubData(GL_ARRAY_BUFFER, offset, size, data + offset); - (void)CHECK_GL_ERROR(); - } - glBindBuffer(GL_ARRAY_BUFFER, 0); - (void)CHECK_GL_ERROR(); - _gpuObject._renderPages._flags &= ~PageManager::DIRTY; - } -}; GLuint GL41Backend::getBufferID(const Buffer& buffer) { return GL41Buffer::getId(*this, buffer); diff --git a/libraries/gpu-gl/src/gpu/gl45/GL45BackendBuffer.cpp b/libraries/gpu-gl/src/gpu/gl45/GL45BackendBuffer.cpp index b9036651b2..c7c9ec1b2e 100644 --- a/libraries/gpu-gl/src/gpu/gl45/GL45BackendBuffer.cpp +++ b/libraries/gpu-gl/src/gpu/gl45/GL45BackendBuffer.cpp @@ -8,38 +8,43 @@ #include "GL45Backend.h" #include "../gl/GLBuffer.h" +namespace gpu { + namespace gl45 { + class GL45Buffer : public gl::GLBuffer { + using Parent = gpu::gl::GLBuffer; + static GLuint allocate() { + GLuint result; + glCreateBuffers(1, &result); + return result; + } + + public: + GL45Buffer(const std::weak_ptr& backend, const Buffer& buffer, GLBuffer* original) : Parent(backend, buffer, allocate()) { + glNamedBufferStorage(_buffer, _size == 0 ? 256 : _size, nullptr, GL_DYNAMIC_STORAGE_BIT); + if (original && original->_size) { + glCopyNamedBufferSubData(original->_buffer, _buffer, 0, 0, std::min(original->_size, _size)); + } + Backend::setGPUObject(buffer, this); + } + + void transfer() override { + Size offset; + Size size; + Size currentPage { 0 }; + auto data = _gpuObject._renderSysmem.readData(); + while (_gpuObject._renderPages.getNextTransferBlock(offset, size, currentPage)) { + glNamedBufferSubData(_buffer, (GLintptr)offset, (GLsizeiptr)size, data + offset); + } + (void)CHECK_GL_ERROR(); + _gpuObject._renderPages._flags &= ~PageManager::DIRTY; + } + }; + } +} + using namespace gpu; using namespace gpu::gl45; -class GL45Buffer : public gl::GLBuffer { - using Parent = gpu::gl::GLBuffer; - static GLuint allocate() { - GLuint result; - glCreateBuffers(1, &result); - return result; - } - -public: - GL45Buffer(const std::weak_ptr& backend, const Buffer& buffer, GLBuffer* original) : Parent(backend, buffer, allocate()) { - glNamedBufferStorage(_buffer, _size == 0 ? 256 : _size, nullptr, GL_DYNAMIC_STORAGE_BIT); - if (original && original->_size) { - glCopyNamedBufferSubData(original->_buffer, _buffer, 0, 0, std::min(original->_size, _size)); - } - Backend::setGPUObject(buffer, this); - } - - void transfer() override { - Size offset; - Size size; - Size currentPage { 0 }; - auto data = _gpuObject._renderSysmem.readData(); - while (_gpuObject._renderPages.getNextTransferBlock(offset, size, currentPage)) { - glNamedBufferSubData(_buffer, (GLintptr)offset, (GLsizeiptr)size, data + offset); - } - (void)CHECK_GL_ERROR(); - _gpuObject._renderPages._flags &= ~PageManager::DIRTY; - } -}; GLuint GL45Backend::getBufferID(const Buffer& buffer) { return GL45Buffer::getId(*this, buffer); diff --git a/libraries/gpu/src/gpu/Buffer.h b/libraries/gpu/src/gpu/Buffer.h index 83b38bfc09..da1a987bee 100644 --- a/libraries/gpu/src/gpu/Buffer.h +++ b/libraries/gpu/src/gpu/Buffer.h @@ -126,6 +126,7 @@ public: // Main thread operation to say that the buffer is ready to be used as a frame Update getUpdate() const; +protected: // For use by the render thread to avoid the intermediate step of getUpdate/applyUpdate void flush() const; @@ -136,8 +137,7 @@ public: mutable std::atomic _getUpdateCount { 0 }; mutable std::atomic _applyUpdateCount { 0 }; -//protected: -public: + void markDirty(Size offset, Size bytes); template @@ -153,10 +153,15 @@ public: Sysmem _sysmem; - // FIXME find a more generic way to do this. - friend class gl::GLBuffer; friend class BufferView; friend class Frame; + friend class Batch; + + // FIXME find a more generic way to do this. + friend class gl::GLBackend; + friend class gl::GLBuffer; + friend class gl41::GL41Buffer; + friend class gl45::GL45Buffer; }; using BufferUpdates = std::vector; diff --git a/libraries/gpu/src/gpu/Forward.h b/libraries/gpu/src/gpu/Forward.h index 624b3c4694..b3e2d6f8a8 100644 --- a/libraries/gpu/src/gpu/Forward.h +++ b/libraries/gpu/src/gpu/Forward.h @@ -118,15 +118,18 @@ namespace gpu { }; namespace gl { + class GLBackend; class GLBuffer; } namespace gl41 { class GL41Backend; + class GL41Buffer; } namespace gl45 { class GL45Backend; + class GL45Buffer; } } diff --git a/libraries/render-utils/src/GeometryCache.cpp b/libraries/render-utils/src/GeometryCache.cpp index 56b8108356..01cec78eae 100644 --- a/libraries/render-utils/src/GeometryCache.cpp +++ b/libraries/render-utils/src/GeometryCache.cpp @@ -378,9 +378,6 @@ void GeometryCache::buildShapes() { //Torus, //Cone, //Cylinder, - - _shapeIndices->flush(); - _shapeVertices->flush(); } gpu::Stream::FormatPointer& getSolidStreamFormat() {