Merge pull request #845 from HifiExperiments/colorBleed

Clean up GeometryCache and remove _glColor4f
overte-org · Mar 15, 2024 · 3d5afbd · 3d5afbd
2 parents 38caf61 + be95d32
commit 3d5afbd
Show file tree

Hide file tree

Showing 17 changed files with 308 additions and 858 deletions.
diff --git a/libraries/entities-renderer/src/RenderableMaterialEntityItem.cpp b/libraries/entities-renderer/src/RenderableMaterialEntityItem.cpp
@@ -337,21 +337,26 @@ void MaterialEntityRenderer::doRender(RenderArgs* args) {
         }
 
         // Draw!
-        DependencyManager::get<GeometryCache>()->renderSphere(batch);
+        const uint32_t compactColor = 0xFFFFFFFF;
+        _colorBuffer->setData(sizeof(compactColor), (const gpu::Byte*) &compactColor);
+        DependencyManager::get<GeometryCache>()->renderShape(batch, GeometryCache::Shape::Sphere, _colorBuffer);
     } else {
         auto proceduralDrawMaterial = std::static_pointer_cast<graphics::ProceduralMaterial>(drawMaterial);
         glm::vec4 outColor = glm::vec4(drawMaterial->getAlbedo(), drawMaterial->getOpacity());
         outColor = proceduralDrawMaterial->getColor(outColor);
         proceduralDrawMaterial->prepare(batch, transform.getTranslation(), transform.getScale(),
                                         transform.getRotation(), _created, ProceduralProgramKey(outColor.a < 1.0f));
+
+        const uint32_t compactColor = GeometryCache::toCompactColor(glm::vec4(outColor));
+        _colorBuffer->setData(sizeof(compactColor), (const gpu::Byte*) &compactColor);
         if (render::ShapeKey(args->_globalShapeKey).isWireframe() || _primitiveMode == PrimitiveMode::LINES) {
-            DependencyManager::get<GeometryCache>()->renderWireSphere(batch, outColor);
+            DependencyManager::get<GeometryCache>()->renderWireShape(batch, GeometryCache::Shape::Sphere, _colorBuffer);
         } else {
-            DependencyManager::get<GeometryCache>()->renderSphere(batch, outColor);
+            DependencyManager::get<GeometryCache>()->renderShape(batch, GeometryCache::Shape::Sphere, _colorBuffer);
         }
     }
 
-    args->_details._trianglesRendered += (int)DependencyManager::get<GeometryCache>()->getSphereTriangleCount();
+    args->_details._trianglesRendered += (int)DependencyManager::get<GeometryCache>()->getShapeTriangleCount(GeometryCache::Shape::Sphere);
 }
 
 void MaterialEntityRenderer::setCurrentMaterialName(const std::string& currentMaterialName) {

diff --git a/libraries/entities-renderer/src/RenderableMaterialEntityItem.h b/libraries/entities-renderer/src/RenderableMaterialEntityItem.h
@@ -67,6 +67,7 @@ class MaterialEntityRenderer : public TypedEntityRenderer<MaterialEntityItem> {
     std::shared_ptr<NetworkMaterial> _appliedMaterial;
     std::string _currentMaterialName;
 
+    gpu::BufferPointer _colorBuffer { std::make_shared<gpu::Buffer>() };
 };
 
 } } 

diff --git a/libraries/entities-renderer/src/RenderableShapeEntityItem.cpp b/libraries/entities-renderer/src/RenderableShapeEntityItem.cpp
@@ -131,10 +131,12 @@ void ShapeEntityRenderer::doRender(RenderArgs* args) {
             procedural->prepare(batch, transform.getTranslation(), transform.getScale(), transform.getRotation(), _created, ProceduralProgramKey(outColor.a < 1.0f));
         });
 
+        const uint32_t compactColor = GeometryCache::toCompactColor(glm::vec4(outColor));
+        _colorBuffer->setData(sizeof(compactColor), (const gpu::Byte*) &compactColor);
         if (wireframe) {
-            geometryCache->renderWireShape(batch, geometryShape, outColor);
+            geometryCache->renderWireShape(batch, geometryShape, _colorBuffer);
         } else {
-            geometryCache->renderShape(batch, geometryShape, outColor);
+            geometryCache->renderShape(batch, geometryShape, _colorBuffer);
         }
     } else if (pipelineType == Pipeline::SIMPLE) {
         // FIXME, support instanced multi-shape rendering using multidraw indirect
@@ -149,18 +151,22 @@ void ShapeEntityRenderer::doRender(RenderArgs* args) {
                 geometryCache->renderSolidShapeInstance(args, batch, geometryShape, outColor, pipeline);
             }
         } else {
+            const uint32_t compactColor = GeometryCache::toCompactColor(glm::vec4(outColor));
+            _colorBuffer->setData(sizeof(compactColor), (const gpu::Byte*) &compactColor);
             if (wireframe) {
-                geometryCache->renderWireShape(batch, geometryShape, outColor);
+                geometryCache->renderWireShape(batch, geometryShape, _colorBuffer);
             } else {
-                geometryCache->renderShape(batch, geometryShape, outColor);
+                geometryCache->renderShape(batch, geometryShape, _colorBuffer);
             }
         }
     } else {
         if (RenderPipelines::bindMaterials(materials, batch, args->_renderMode, args->_enableTexturing)) {
             args->_details._materialSwitches++;
         }
 
-        geometryCache->renderShape(batch, geometryShape);
+        const uint32_t compactColor = GeometryCache::toCompactColor(glm::vec4(outColor));
+        _colorBuffer->setData(sizeof(compactColor), (const gpu::Byte*) &compactColor);
+        geometryCache->renderShape(batch, geometryShape, _colorBuffer);
     }
 
     const auto triCount = geometryCache->getShapeTriangleCount(geometryShape);

diff --git a/libraries/entities-renderer/src/RenderableShapeEntityItem.h b/libraries/entities-renderer/src/RenderableShapeEntityItem.h
@@ -42,6 +42,8 @@ class ShapeEntityRenderer : public TypedEntityRenderer<ShapeEntityItem> {
     std::shared_ptr<graphics::ProceduralMaterial> _material { std::make_shared<graphics::ProceduralMaterial>() };
     glm::vec3 _color { NAN };
     float _alpha { NAN };
+
+    gpu::BufferPointer _colorBuffer { std::make_shared<gpu::Buffer>() };
 };
 
 } } 

diff --git a/libraries/gpu-gl-common/src/gpu/gl/GLBackend.cpp b/libraries/gpu-gl-common/src/gpu/gl/GLBackend.cpp
@@ -100,8 +100,6 @@ GLBackend::CommandCall GLBackend::_commandCalls[Batch::NUM_COMMANDS] =
     (&::gpu::gl::GLBackend::do_glUniformMatrix3fv),
     (&::gpu::gl::GLBackend::do_glUniformMatrix4fv),
 
-    (&::gpu::gl::GLBackend::do_glColor4f),
-
     (&::gpu::gl::GLBackend::do_pushProfileRange),
     (&::gpu::gl::GLBackend::do_popProfileRange),
 };
@@ -838,22 +836,6 @@ void GLBackend::do_glUniformMatrix4fv(const Batch& batch, size_t paramOffset) {
     (void)CHECK_GL_ERROR();
 }
 
-void GLBackend::do_glColor4f(const Batch& batch, size_t paramOffset) {
-
-    glm::vec4 newColor(
-        batch._params[paramOffset + 3]._float,
-        batch._params[paramOffset + 2]._float,
-        batch._params[paramOffset + 1]._float,
-        batch._params[paramOffset + 0]._float);
-
-    if (_input._colorAttribute != newColor) {
-        _input._colorAttribute = newColor;
-        glVertexAttrib4fv(gpu::Stream::COLOR, &_input._colorAttribute.r);
-        _input._hasColorAttribute = true;
-    }
-    (void)CHECK_GL_ERROR();
-}
-
 void GLBackend::releaseBuffer(GLuint id, Size size) const {
     Lock lock(_trashMutex);
     _currentFrameTrash.buffersTrash.push_back({ id, size });

diff --git a/libraries/gpu-gl-common/src/gpu/gl/GLBackend.h b/libraries/gpu-gl-common/src/gpu/gl/GLBackend.h
@@ -241,8 +241,6 @@ class GLBackend : public Backend, public std::enable_shared_from_this<GLBackend>
     virtual void do_glUniformMatrix3fv(const Batch& batch, size_t paramOffset) final;
     virtual void do_glUniformMatrix4fv(const Batch& batch, size_t paramOffset) final;
 
-    virtual void do_glColor4f(const Batch& batch, size_t paramOffset) final;
-
     // The State setters called by the GLState::Commands when a new state is assigned
     virtual void do_setStateFillMode(int32 mode) final;
     virtual void do_setStateCullMode(int32 mode) final;
@@ -350,8 +348,6 @@ class GLBackend : public Backend, public std::enable_shared_from_this<GLBackend>
     struct InputStageState {
         bool _invalidFormat { true };
         bool _lastUpdateStereoState { false };
-        bool _hasColorAttribute { false };
-        bool _hadColorAttribute { false };
         FormatReference _format { GPU_REFERENCE_INIT_VALUE };
         std::string _formatKey;
 
@@ -368,8 +364,6 @@ class GLBackend : public Backend, public std::enable_shared_from_this<GLBackend>
         std::array<Offset, MAX_NUM_INPUT_BUFFERS> _bufferStrides;
         std::array<GLuint, MAX_NUM_INPUT_BUFFERS> _bufferVBOs;
 
-        glm::vec4 _colorAttribute { 1.0f };
-
         BufferReference _indexBuffer;
         Offset _indexBufferOffset { 0 };
         Type _indexBufferType { UINT32 };

diff --git a/libraries/gpu-gl-common/src/gpu/gl/GLBackendInput.cpp b/libraries/gpu-gl-common/src/gpu/gl/GLBackendInput.cpp
@@ -103,9 +103,6 @@ void GLBackend::resetInputStage() {
     reset(_input._format);
     _input._formatKey.clear();
     _input._invalidFormat = false;
-    _input._hasColorAttribute = false;
-    _input._hadColorAttribute = false;
-    _input._colorAttribute = vec4(1.0f);
     _input._attributeActivation.reset();
 
     for (uint32_t i = 0; i < _input._buffers.size(); i++) {
@@ -163,8 +160,6 @@ void GLBackend::updateInput() {
 #endif
     _input._lastUpdateStereoState = isStereoNow;
 
-    bool hasColorAttribute = _input._hasColorAttribute;
-
     if (_input._invalidFormat) {
         InputStageState::ActivationCache newActivation;
 
@@ -194,8 +189,6 @@ void GLBackend::updateInput() {
 
                     GLenum perLocationSize = attrib._element.getLocationSize();
 
-                    hasColorAttribute |= slot == Stream::COLOR;
-
                     for (GLuint locNum = 0; locNum < locationCount; ++locNum) {
                         GLuint attriNum = (GLuint)(slot + locNum);
                         newActivation.set(attriNum);
@@ -226,12 +219,6 @@ void GLBackend::updateInput() {
                 glVertexBindingDivisor(bufferChannelNum, frequency);
 #endif
             }
-
-            if (!hasColorAttribute && _input._hadColorAttribute) {
-                // The previous input stage had a color attribute but this one doesn't, so reset the color to pure white.
-                _input._colorAttribute = glm::vec4(1.0f);
-                glVertexAttrib4fv(Stream::COLOR, &_input._colorAttribute.r);
-            }
         }
 
         // Manage Activation what was and what is expected now
@@ -253,9 +240,6 @@ void GLBackend::updateInput() {
         _stats._ISNumFormatChanges++;
     }
 
-    _input._hadColorAttribute = hasColorAttribute;
-    _input._hasColorAttribute = false;
-
     if (_input._invalidBuffers.any()) {
         auto vbo = _input._bufferVBOs.data();
         auto offset = _input._bufferOffsets.data();

diff --git a/libraries/gpu-gl/src/gpu/gl41/GL41BackendInput.cpp b/libraries/gpu-gl/src/gpu/gl41/GL41BackendInput.cpp
@@ -33,8 +33,6 @@ void GL41Backend::updateInput() {
 #endif
     _input._lastUpdateStereoState = isStereoNow;
 
-    bool hasColorAttribute = _input._hasColorAttribute;
-
     if (_input._invalidFormat || _input._invalidBuffers.any()) {
 
         auto format = acquire(_input._format);
@@ -110,8 +108,6 @@ void GL41Backend::updateInput() {
                             uintptr_t pointer = (uintptr_t)(attrib._offset + offsets[bufferNum]);
                             GLboolean isNormalized = attrib._element.isNormalized();
 
-                            hasColorAttribute |= slot == Stream::COLOR;
-
                             for (size_t locNum = 0; locNum < locationCount; ++locNum) {
                                 if (attrib._element.isInteger()) {
                                     glVertexAttribIPointer(slot + (GLuint)locNum, count, type, stride,
@@ -131,17 +127,8 @@ void GL41Backend::updateInput() {
                     }
                 }
             }
-
-            if (!hasColorAttribute && _input._hadColorAttribute) {
-                // The previous input stage had a color attribute but this one doesn't, so reset the color to pure white.
-                _input._colorAttribute = glm::vec4(1.0f);
-                glVertexAttrib4fv(Stream::COLOR, &_input._colorAttribute.r);
-            }
         }
         // everything format related should be in sync now
         _input._invalidFormat = false;
     }
-
-    _input._hadColorAttribute = hasColorAttribute;
-    _input._hasColorAttribute = false;
 }
diff --git a/libraries/gpu-gl/src/gpu/gl45/GL45BackendInput.cpp b/libraries/gpu-gl/src/gpu/gl45/GL45BackendInput.cpp
@@ -35,8 +35,6 @@ void GL45Backend::updateInput() {
 #endif
     _input._lastUpdateStereoState = isStereoNow;
 
-    bool hasColorAttribute = _input._hasColorAttribute;
-
     if (_input._invalidFormat) {
         InputStageState::ActivationCache newActivation;
 
@@ -66,8 +64,6 @@ void GL45Backend::updateInput() {
 
                     GLenum perLocationSize = attrib._element.getLocationSize();
 
-                    hasColorAttribute |= slot == Stream::COLOR;
-
                     for (GLuint locNum = 0; locNum < locationCount; ++locNum) {
                         GLuint attriNum = (GLuint)(slot + locNum);
                         newActivation.set(attriNum);
@@ -98,12 +94,6 @@ void GL45Backend::updateInput() {
                 glVertexBindingDivisor(bufferChannelNum, frequency);
 #endif
             }
-
-            if (!hasColorAttribute && _input._hadColorAttribute) {
-                // The previous input stage had a color attribute but this one doesn't, so reset the color to pure white.
-                _input._colorAttribute = glm::vec4(1.0f);
-                glVertexAttrib4fv(Stream::COLOR, &_input._colorAttribute.r);
-            }
         }
 
         // Manage Activation what was and what is expected now
@@ -125,9 +115,6 @@ void GL45Backend::updateInput() {
         _stats._ISNumFormatChanges++;
     }
 
-    _input._hadColorAttribute = hasColorAttribute;
-    _input._hasColorAttribute = false;
-
     if (_input._invalidBuffers.any()) {
         auto vbo = _input._bufferVBOs.data();
         auto offset = _input._bufferOffsets.data();

diff --git a/libraries/gpu/src/gpu/Batch.cpp b/libraries/gpu/src/gpu/Batch.cpp
@@ -687,15 +687,6 @@ void Batch::_glUniformMatrix4fv(int32 location, int count, uint8 transpose, cons
     _params.emplace_back(location);
 }
 
-void Batch::_glColor4f(float red, float green, float blue, float alpha) {
-    ADD_COMMAND(glColor4f);
-
-    _params.emplace_back(alpha);
-    _params.emplace_back(blue);
-    _params.emplace_back(green);
-    _params.emplace_back(red);
-}
-
 void Batch::finishFrame(BufferUpdates& updates) {
     PROFILE_RANGE(render_gpu, __FUNCTION__);
 

diff --git a/libraries/gpu/src/gpu/Batch.h b/libraries/gpu/src/gpu/Batch.h
@@ -287,8 +287,6 @@ class Batch {
         _glUniformMatrix3fv(location, 1, false, glm::value_ptr(v));
     }
 
-    void _glColor4f(float red, float green, float blue, float alpha);
-
     // Maybe useful but shoudln't be public. Please convince me otherwise
     // Well porting to gles i need it...
     void runLambda(std::function<void()> f);
@@ -363,8 +361,6 @@ class Batch {
         COMMAND_glUniformMatrix3fv,
         COMMAND_glUniformMatrix4fv,
 
-        COMMAND_glColor4f,
-
         COMMAND_pushProfileRange,
         COMMAND_popProfileRange,
 

diff --git a/libraries/gpu/src/gpu/FrameIOKeys.h b/libraries/gpu/src/gpu/FrameIOKeys.h
@@ -201,8 +201,6 @@ constexpr const char*  COMMAND_NAMES[] = {
     "glUniformMatrix3fv",
     "glUniformMatrix4fv",
 
-    "glColor4f",
-
     "pushProfileRange",
     "popProfileRange",
 };

diff --git a/libraries/graphics/src/graphics/Geometry.cpp b/libraries/graphics/src/graphics/Geometry.cpp
@@ -19,14 +19,17 @@ Mesh::Mesh() :
     _vertexBuffer(gpu::Element(gpu::VEC3, gpu::FLOAT, gpu::XYZ)),
     _indexBuffer(gpu::Element(gpu::SCALAR, gpu::UINT32, gpu::INDEX)),
     _partBuffer(gpu::Element(gpu::VEC4, gpu::UINT32, gpu::PART)) {
+    const uint32_t compactColor = 0xFFFFFFFF;
+    _colorBuffer->setData(sizeof(compactColor), (const gpu::Byte*) &compactColor);
 }
 
 Mesh::Mesh(const Mesh& mesh) :
     _vertexFormat(mesh._vertexFormat),
     _vertexBuffer(mesh._vertexBuffer),
     _attributeBuffers(mesh._attributeBuffers),
     _indexBuffer(mesh._indexBuffer),
-    _partBuffer(mesh._partBuffer) {
+    _partBuffer(mesh._partBuffer),
+    _colorBuffer(mesh._colorBuffer) {
 }
 
 Mesh::~Mesh() {
@@ -39,6 +42,13 @@ void Mesh::setVertexFormatAndStream(const gpu::Stream::FormatPointer& vf, const
     auto attrib = _vertexFormat->getAttribute(gpu::Stream::POSITION);
     _vertexBuffer = BufferView(vbs->getBuffers()[attrib._channel], vbs->getOffsets()[attrib._channel], vbs->getBuffers()[attrib._channel]->getSize(),
         (gpu::uint16) vbs->getStrides()[attrib._channel], attrib._element);
+
+    // We require meshes to have a color attribute.  If they don't, we default to white.
+    if (!_vertexFormat->hasAttribute(gpu::Stream::COLOR)) {
+        int channelNum = _vertexStream.getNumBuffers();
+        _vertexFormat->setAttribute(gpu::Stream::COLOR, channelNum, gpu::Element(gpu::VEC4, gpu::NUINT8, gpu::RGBA), 0, gpu::Stream::PER_INSTANCE);
+        _vertexStream.addBuffer(_colorBuffer, 0, _vertexFormat->getChannels().at(channelNum)._stride);
+    }
 }
 
 void Mesh::setVertexBuffer(const BufferView& buffer) {
@@ -98,6 +108,12 @@ void Mesh::evalVertexStream() {
         _vertexStream.addBuffer(view._buffer, view._offset, stride);
         channelNum++;
     }
+
+    // We require meshes to have a color attribute.  If they don't, we default to white.
+    if (!_vertexFormat->hasAttribute(gpu::Stream::COLOR)) {
+        _vertexFormat->setAttribute(gpu::Stream::COLOR, channelNum, gpu::Element(gpu::VEC4, gpu::NUINT8, gpu::RGBA), 0, gpu::Stream::PER_INSTANCE);
+        _vertexStream.addBuffer(_colorBuffer, 0, _vertexFormat->getChannels().at(channelNum)._stride);
+    }
 }
 
 void Mesh::setIndexBuffer(const BufferView& buffer) {

diff --git a/libraries/graphics/src/graphics/Geometry.h b/libraries/graphics/src/graphics/Geometry.h
@@ -15,6 +15,7 @@
 
 #include <AABox.h>
 
+#include <gpu/Forward.h>
 #include <gpu/Resource.h>
 #include <gpu/Stream.h>
 
@@ -28,7 +29,6 @@ typedef glm::vec3 Vec3;
 class Mesh;
 using MeshPointer = std::shared_ptr< Mesh >;
 
-
 class Mesh {
 public:
     const static Index PRIMITIVE_RESTART_INDEX = -1;
@@ -142,6 +142,8 @@ class Mesh {
     std::string modelName;
     std::string displayName;
 
+    gpu::BufferPointer getColorBuffer() const { return _colorBuffer; }
+
 protected:
 
     gpu::Stream::FormatPointer _vertexFormat;
@@ -154,6 +156,8 @@ class Mesh {
 
     BufferView _partBuffer;
 
+    gpu::BufferPointer _colorBuffer { std::make_shared<gpu::Buffer>() };
+
     void evalVertexFormat();
     void evalVertexStream();