From 1123ba2b0a5bebd92c87ea336655b9a8472b4b39 Mon Sep 17 00:00:00 2001 From: Sam Gateau Date: Wed, 22 Aug 2018 22:48:42 -0700 Subject: [PATCH] exploring optimization in Backend for bffer bindings --- libraries/gpu-gl-common/src/gpu/gl/GLBackend.h | 1 + .../gpu-gl-common/src/gpu/gl/GLBackendInput.cpp | 17 ++++++++++++----- .../src/gpu/gl/GLBackendPipeline.cpp | 2 ++ libraries/gpu-gl-common/src/gpu/gl/GLBuffer.h | 14 +++++++++++++- libraries/gpu-gl/src/gpu/gl41/GL41Backend.h | 1 + .../gpu-gl/src/gpu/gl41/GL41BackendBuffer.cpp | 4 ++++ libraries/gpu-gl/src/gpu/gl45/GL45Backend.h | 1 + .../gpu-gl/src/gpu/gl45/GL45BackendBuffer.cpp | 5 +++++ libraries/gpu/src/gpu/Batch.h | 4 ++-- 9 files changed, 41 insertions(+), 8 deletions(-) diff --git a/libraries/gpu-gl-common/src/gpu/gl/GLBackend.h b/libraries/gpu-gl-common/src/gpu/gl/GLBackend.h index cadcec7a56..2fa2df5bfa 100644 --- a/libraries/gpu-gl-common/src/gpu/gl/GLBackend.h +++ b/libraries/gpu-gl-common/src/gpu/gl/GLBackend.h @@ -239,6 +239,7 @@ public: virtual GLuint getFramebufferID(const FramebufferPointer& framebuffer) = 0; virtual GLuint getTextureID(const TexturePointer& texture) final; virtual GLuint getBufferID(const Buffer& buffer) = 0; + virtual GLuint getBufferIDUnsafe(const Buffer& buffer) = 0; virtual GLuint getQueryID(const QueryPointer& query) = 0; virtual GLFramebuffer* syncGPUObject(const Framebuffer& framebuffer) = 0; diff --git a/libraries/gpu-gl-common/src/gpu/gl/GLBackendInput.cpp b/libraries/gpu-gl-common/src/gpu/gl/GLBackendInput.cpp index 77e1f90f66..6ce25bc56c 100644 --- a/libraries/gpu-gl-common/src/gpu/gl/GLBackendInput.cpp +++ b/libraries/gpu-gl-common/src/gpu/gl/GLBackendInput.cpp @@ -11,6 +11,7 @@ #include "GLBackend.h" #include "GLShared.h" #include "GLInputFormat.h" +#include "GLBuffer.h" using namespace gpu; using namespace gpu::gl; @@ -39,14 +40,20 @@ void GLBackend::do_setInputBuffer(const Batch& batch, size_t paramOffset) { BufferPointer buffer = batch._buffers.get(batch._params[paramOffset + 2]._uint); uint32 channel = batch._params[paramOffset + 3]._uint; - if (channel < getNumInputBuffers()) { + // if (channel < getNumInputBuffers()) { bool isModified = false; if (_input._buffers[channel] != buffer) { _input._buffers[channel] = buffer; GLuint vbo = 0; if (buffer) { - vbo = getBufferID((*buffer)); + // vbo = getBufferID((*buffer)); + // vbo = getBufferIDUnsafe((*buffer)); + auto* object = Backend::getGPUObject((*buffer)); + + if (object) { + vbo = object->_buffer; + } } _input._bufferVBOs[channel] = vbo; @@ -66,7 +73,7 @@ void GLBackend::do_setInputBuffer(const Batch& batch, size_t paramOffset) { if (isModified) { _input._invalidBuffers.set(channel); } - } + // } } void GLBackend::initInput() { @@ -128,7 +135,7 @@ void GLBackend::do_setIndexBuffer(const Batch& batch, size_t paramOffset) { if (indexBuffer != _input._indexBuffer) { _input._indexBuffer = indexBuffer; if (indexBuffer) { - glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, getBufferID(*indexBuffer)); + glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, getBufferIDUnsafe(*indexBuffer)); } else { // FIXME do we really need this? Is there ever a draw call where we care that the element buffer is null? glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0); @@ -145,7 +152,7 @@ void GLBackend::do_setIndirectBuffer(const Batch& batch, size_t paramOffset) { if (buffer != _input._indirectBuffer) { _input._indirectBuffer = buffer; if (buffer) { - glBindBuffer(GL_DRAW_INDIRECT_BUFFER, getBufferID(*buffer)); + glBindBuffer(GL_DRAW_INDIRECT_BUFFER, getBufferIDUnsafe(*buffer)); } else { // FIXME do we really need this? Is there ever a draw call where we care that the element buffer is null? glBindBuffer(GL_DRAW_INDIRECT_BUFFER, 0); diff --git a/libraries/gpu-gl-common/src/gpu/gl/GLBackendPipeline.cpp b/libraries/gpu-gl-common/src/gpu/gl/GLBackendPipeline.cpp index d3d2bc0938..fd67202863 100644 --- a/libraries/gpu-gl-common/src/gpu/gl/GLBackendPipeline.cpp +++ b/libraries/gpu-gl-common/src/gpu/gl/GLBackendPipeline.cpp @@ -172,6 +172,8 @@ void GLBackend::bindUniformBuffer(uint32_t slot, const BufferPointer& buffer, GL // Sync BufferObject auto* object = syncGPUObject(*bufferState.buffer); + // auto glBO = getBufferIDUnsafe(*buffer); + if (object) { glBindBufferRange(GL_UNIFORM_BUFFER, slot, object->_buffer, bufferState.offset, bufferState.size); diff --git a/libraries/gpu-gl-common/src/gpu/gl/GLBuffer.h b/libraries/gpu-gl-common/src/gpu/gl/GLBuffer.h index 182014e764..8efbe03b90 100644 --- a/libraries/gpu-gl-common/src/gpu/gl/GLBuffer.h +++ b/libraries/gpu-gl-common/src/gpu/gl/GLBuffer.h @@ -49,13 +49,25 @@ public: } } + template + static GLuint getIdUnsafe(GLBackend& backend, const Buffer& buffer) { + GLBufferType* object = Backend::getGPUObject(buffer); + + if (object) { + return object->_buffer; + } + else { + return 0; + } + } + const GLuint& _buffer { _id }; const GLuint _size; const Stamp _stamp; ~GLBuffer(); - virtual void transfer() = 0; + virtual void transfer() {}; protected: GLBuffer(const std::weak_ptr& backend, const Buffer& buffer, GLuint id); diff --git a/libraries/gpu-gl/src/gpu/gl41/GL41Backend.h b/libraries/gpu-gl/src/gpu/gl41/GL41Backend.h index c6fbc43ae5..a09b3e9297 100644 --- a/libraries/gpu-gl/src/gpu/gl41/GL41Backend.h +++ b/libraries/gpu-gl/src/gpu/gl41/GL41Backend.h @@ -134,6 +134,7 @@ protected: GLFramebuffer* syncGPUObject(const Framebuffer& framebuffer) override; GLuint getBufferID(const Buffer& buffer) override; + GLuint getBufferIDUnsafe(const Buffer& buffer) override; GLuint getResourceBufferID(const Buffer& buffer); GLBuffer* syncGPUObject(const Buffer& buffer) override; diff --git a/libraries/gpu-gl/src/gpu/gl41/GL41BackendBuffer.cpp b/libraries/gpu-gl/src/gpu/gl41/GL41BackendBuffer.cpp index 62ade673b4..97ac2739eb 100644 --- a/libraries/gpu-gl/src/gpu/gl41/GL41BackendBuffer.cpp +++ b/libraries/gpu-gl/src/gpu/gl41/GL41BackendBuffer.cpp @@ -83,6 +83,10 @@ GLuint GL41Backend::getBufferID(const Buffer& buffer) { return GL41Buffer::getId(*this, buffer); } +GLuint GL41Backend::getBufferIDUnsafe(const Buffer& buffer) { + return GL41Backend::getBufferID(buffer); +} + GLuint GL41Backend::getResourceBufferID(const Buffer& buffer) { auto* object = GL41Buffer::sync(*this, buffer); if (object) { diff --git a/libraries/gpu-gl/src/gpu/gl45/GL45Backend.h b/libraries/gpu-gl/src/gpu/gl45/GL45Backend.h index 658bea2a3e..5da4506773 100644 --- a/libraries/gpu-gl/src/gpu/gl45/GL45Backend.h +++ b/libraries/gpu-gl/src/gpu/gl45/GL45Backend.h @@ -235,6 +235,7 @@ protected: GLFramebuffer* syncGPUObject(const Framebuffer& framebuffer) override; GLuint getBufferID(const Buffer& buffer) override; + GLuint getBufferIDUnsafe(const Buffer& buffer) override; GLBuffer* syncGPUObject(const Buffer& buffer) override; GLTexture* syncGPUObject(const TexturePointer& texture) override; diff --git a/libraries/gpu-gl/src/gpu/gl45/GL45BackendBuffer.cpp b/libraries/gpu-gl/src/gpu/gl45/GL45BackendBuffer.cpp index 2afbea3876..0d26f8f412 100644 --- a/libraries/gpu-gl/src/gpu/gl45/GL45BackendBuffer.cpp +++ b/libraries/gpu-gl/src/gpu/gl45/GL45BackendBuffer.cpp @@ -51,6 +51,11 @@ GLuint GL45Backend::getBufferID(const Buffer& buffer) { return GL45Buffer::getId(*this, buffer); } +GLuint GL45Backend::getBufferIDUnsafe(const Buffer& buffer) { + //return GL45Buffer::getId(*this, buffer); + return GL45Buffer::getIdUnsafe(*this, buffer); +} + GLBuffer* GL45Backend::syncGPUObject(const Buffer& buffer) { return GL45Buffer::sync(*this, buffer); } diff --git a/libraries/gpu/src/gpu/Batch.h b/libraries/gpu/src/gpu/Batch.h index bcbfe0616d..2bead507b8 100644 --- a/libraries/gpu/src/gpu/Batch.h +++ b/libraries/gpu/src/gpu/Batch.h @@ -417,10 +417,10 @@ public: } const Data& get(uint32 offset) const { - if (offset >= _items.size()) { + /* if (offset >= _items.size()) { static const Data EMPTY; return EMPTY; - } + }*/ return (_items.data() + offset)->_data; }