Removed glColor reset hack in MeshPartPayload and replaced it by a reset of the color attribute to white in the execution of the setInputFormat command of the various GLBackends

libraries/entities-renderer/src/RenderablePolyLineEntityItem.cpp

@@ -303,6 +303,7 @@ void PolyLineEntityRenderer::doRender(RenderArgs* args) {
     batch.setInputBuffer(0, _verticesBuffer, 0, sizeof(Vertex));
 
 #ifndef POLYLINE_ENTITY_USE_FADE_EFFECT
+    // glColor4f must be called after setInputFormat if it must be taken into account
     if (_isFading) {
         batch._glColor4f(1.0f, 1.0f, 1.0f, Interpolate::calculateFadeRatio(_fadeStartTime));
     } else {

libraries/entities-renderer/src/RenderableShapeEntityItem.cpp

@@ -137,11 +137,10 @@ void ShapeEntityRenderer::doRender(RenderArgs* args) {
     });
 
     if (proceduralRender) {
-        batch._glColor4f(outColor.r, outColor.g, outColor.b, outColor.a);
         if (render::ShapeKey(args->_globalShapeKey).isWireframe()) {
-            geometryCache->renderWireShape(batch, geometryShape);
+            geometryCache->renderWireShape(batch, geometryShape, outColor);
         } else {
-            geometryCache->renderShape(batch, geometryShape);
+            geometryCache->renderShape(batch, geometryShape, outColor);
         }
     } else {
         // FIXME, support instanced multi-shape rendering using multidraw indirect

libraries/entities-renderer/src/RenderableWebEntityItem.cpp

@@ -188,7 +188,6 @@ void WebEntityRenderer::doRender(RenderArgs* args) {
     });
     batch.setResourceTexture(0, _texture);
     float fadeRatio = _isFading ? Interpolate::calculateFadeRatio(_fadeStartTime) : 1.0f;
-    batch._glColor4f(1.0f, 1.0f, 1.0f, fadeRatio);
 
     DependencyManager::get<GeometryCache>()->bindWebBrowserProgram(batch, fadeRatio < OPAQUE_ALPHA_THRESHOLD);
     DependencyManager::get<GeometryCache>()->renderQuad(batch, topLeft, bottomRight, texMin, texMax, glm::vec4(1.0f, 1.0f, 1.0f, fadeRatio), _geometryId);

libraries/gpu-gl/src/gpu/gl/GLBackend.cpp

@@ -605,6 +605,10 @@ void GLBackend::do_glColor4f(const Batch& batch, size_t paramOffset) {
     if (_input._colorAttribute != newColor) {
         _input._colorAttribute = newColor;
         glVertexAttrib4fv(gpu::Stream::COLOR, &_input._colorAttribute.r);
+        // Color has been changed and is not white. To prevent colors from bleeding
+        // between different objects, we need to set the _hadColorAttribute flag
+        // as if a previous render call had potential colors
+        _input._hadColorAttribute = (newColor != glm::vec4(1.0f, 1.0f, 1.0f, 1.0f));
     }
     (void)CHECK_GL_ERROR();
 }

libraries/gpu-gl/src/gpu/gl/GLBackend.h

@@ -253,6 +253,7 @@ protected:
 
     struct InputStageState {
         bool _invalidFormat { true };
+        bool _hadColorAttribute{ true };
         Stream::FormatPointer _format;
         std::string _formatKey;
 

libraries/gpu-gl/src/gpu/gl41/GL41BackendInput.cpp

@@ -62,6 +62,8 @@ void GL41Backend::updateInput() {
 
         // now we need to bind the buffers and assign the attrib pointers
         if (_input._format) {
+            bool hasColorAttribute{ false };
+
             const Buffers& buffers = _input._buffers;
             const Offsets& offsets = _input._bufferOffsets;
             const Offsets& strides = _input._bufferStrides;
@@ -98,6 +100,8 @@ void GL41Backend::updateInput() {
                             uintptr_t pointer = (uintptr_t)(attrib._offset + offsets[bufferNum]);
                             GLboolean isNormalized = attrib._element.isNormalized();
 
+                            hasColorAttribute = hasColorAttribute || (slot == Stream::COLOR);
+
                             for (size_t locNum = 0; locNum < locationCount; ++locNum) {
                                 if (attrib._element.isInteger()) {
                                     glVertexAttribIPointer(slot + (GLuint)locNum, count, type, stride,
@@ -117,6 +121,15 @@ void GL41Backend::updateInput() {
                     }
                 }
             }
+
+            if (_input._hadColorAttribute && !hasColorAttribute) {
+                // The previous input stage had a color attribute but this one doesn't so reset
+                // color to pure white.
+                const auto white = glm::vec4(1.0f, 1.0f, 1.0f, 1.0f);
+                glVertexAttrib4fv(Stream::COLOR, &white.r);
+                _input._colorAttribute = white;
+            }
+            _input._hadColorAttribute = hasColorAttribute;
         }
         // everything format related should be in sync now
         _input._invalidFormat = false;

libraries/gpu-gl/src/gpu/gl45/GL45BackendInput.cpp

@@ -32,6 +32,8 @@ void GL45Backend::updateInput() {
 
         // Assign the vertex format required
         if (_input._format) {
+            bool hasColorAttribute{ false };
+
             _input._attribBindingBuffers.reset();
 
             const Stream::Format::AttributeMap& attributes = _input._format->getAttributes();
@@ -54,6 +56,9 @@ void GL45Backend::updateInput() {
                     GLboolean isNormalized = attrib._element.isNormalized();
 
                     GLenum perLocationSize = attrib._element.getLocationSize();
+
+                    hasColorAttribute = hasColorAttribute || (slot == Stream::COLOR);
+
                     for (GLuint locNum = 0; locNum < locationCount; ++locNum) {
                         GLuint attriNum = (GLuint)(slot + locNum);
                         newActivation.set(attriNum);
@@ -84,6 +89,15 @@ void GL45Backend::updateInput() {
                 glVertexBindingDivisor(bufferChannelNum, frequency);
 #endif
             }
+
+            if (_input._hadColorAttribute && !hasColorAttribute) {
+                // The previous input stage had a color attribute but this one doesn't so reset
+                // color to pure white.
+                const auto white = glm::vec4(1.0f, 1.0f, 1.0f, 1.0f);
+                glVertexAttrib4fv(Stream::COLOR, &white.r);
+                _input._colorAttribute = white;
+            }
+            _input._hadColorAttribute = hasColorAttribute;
         }
 
         // Manage Activation what was and what is expected now

libraries/render-utils/src/GeometryCache.cpp

@@ -760,6 +760,20 @@ void GeometryCache::renderWireShape(gpu::Batch& batch, Shape shape) {
     _shapes[shape].drawWire(batch);
 }
 
+void GeometryCache::renderShape(gpu::Batch& batch, Shape shape, const glm::vec4& color) {
+    batch.setInputFormat(getSolidStreamFormat());
+    // Color must be set after input format
+    batch._glColor4f(color.r, color.g, color.b, color.a);
+    _shapes[shape].draw(batch);
+}
+
+void GeometryCache::renderWireShape(gpu::Batch& batch, Shape shape, const glm::vec4& color) {
+    batch.setInputFormat(getSolidStreamFormat());
+    // Color must be set after input format
+    batch._glColor4f(color.r, color.g, color.b, color.a);
+    _shapes[shape].drawWire(batch);
+}
+
 void setupBatchInstance(gpu::Batch& batch, gpu::BufferPointer colorBuffer) {
     gpu::BufferView colorView(colorBuffer, COLOR_ELEMENT);
     batch.setInputBuffer(gpu::Stream::COLOR, colorView);
@@ -811,6 +825,14 @@ void GeometryCache::renderWireCube(gpu::Batch& batch) {
     renderWireShape(batch, Cube);
 }
 
+void GeometryCache::renderCube(gpu::Batch& batch, const glm::vec4& color) {
+    renderShape(batch, Cube, color);
+}
+
+void GeometryCache::renderWireCube(gpu::Batch& batch, const glm::vec4& color) {
+    renderWireShape(batch, Cube, color);
+}
+
 void GeometryCache::renderSphere(gpu::Batch& batch) {
     renderShape(batch, Sphere);
 }
@@ -819,6 +841,14 @@ void GeometryCache::renderWireSphere(gpu::Batch& batch) {
     renderWireShape(batch, Sphere);
 }
 
+void GeometryCache::renderSphere(gpu::Batch& batch, const glm::vec4& color) {
+    renderShape(batch, Sphere, color);
+}
+
+void GeometryCache::renderWireSphere(gpu::Batch& batch, const glm::vec4& color) {
+    renderWireShape(batch, Sphere, color);
+}
+
 void GeometryCache::renderGrid(gpu::Batch& batch, const glm::vec2& minCorner, const glm::vec2& maxCorner,
     int majorRows, int majorCols, float majorEdge,
     int minorRows, int minorCols, float minorEdge,

libraries/render-utils/src/GeometryCache.h

@@ -251,14 +251,20 @@ public:
     // Dynamic geometry
     void renderShape(gpu::Batch& batch, Shape shape);
     void renderWireShape(gpu::Batch& batch, Shape shape);
+    void renderShape(gpu::Batch& batch, Shape shape, const glm::vec4& color);
+    void renderWireShape(gpu::Batch& batch, Shape shape, const glm::vec4& color);
     size_t getShapeTriangleCount(Shape shape);
 
     void renderCube(gpu::Batch& batch);
     void renderWireCube(gpu::Batch& batch);
+    void renderCube(gpu::Batch& batch, const glm::vec4& color);
+    void renderWireCube(gpu::Batch& batch, const glm::vec4& color);
     size_t getCubeTriangleCount();
 
     void renderSphere(gpu::Batch& batch);
     void renderWireSphere(gpu::Batch& batch);
+    void renderSphere(gpu::Batch& batch, const glm::vec4& color);
+    void renderWireSphere(gpu::Batch& batch, const glm::vec4& color);
     size_t getSphereTriangleCount();
 
     void renderGrid(gpu::Batch& batch, const glm::vec2& minCorner, const glm::vec2& maxCorner,

libraries/render-utils/src/MeshPartPayload.cpp

@@ -122,11 +122,6 @@ void MeshPartPayload::bindMesh(gpu::Batch& batch) {
     batch.setInputFormat((_drawMesh->getVertexFormat()));
 
     batch.setInputStream(0, _drawMesh->getVertexStream());
-
-    // TODO: Get rid of that extra call
-    if (!_hasColorAttrib) {
-        batch._glColor4f(1.0f, 1.0f, 1.0f, 1.0f);
-    }
 }
 
 void MeshPartPayload::bindMaterial(gpu::Batch& batch, const ShapePipeline::LocationsPointer locations, bool enableTextures) const {
@@ -526,11 +521,6 @@ void ModelMeshPartPayload::bindMesh(gpu::Batch& batch) {
             batch.setInputStream(0, _drawMesh->getVertexStream());
         }
     }
-
-    // TODO: Get rid of that extra call
-    if (!_hasColorAttrib) {
-        batch._glColor4f(1.0f, 1.0f, 1.0f, 1.0f);
-    }
 }
 
 void ModelMeshPartPayload::bindTransform(gpu::Batch& batch, const ShapePipeline::LocationsPointer locations, RenderArgs::RenderMode renderMode) const {

tests/gpu-test/src/TestShapes.cpp

@@ -29,19 +29,18 @@ void TestShapes::renderTest(size_t testId, RenderArgs* args) {
     float seconds = secTimestampNow() - startSecs;
     seconds /= 4.0f;
     batch.setModelTransform(Transform());
-    batch._glColor4f(0.8f, 0.25f, 0.25f, 1.0f);
+    const auto color = glm::vec4(0.8f, 0.25f, 0.25f, 1.0f);
 
     bool wire = (seconds - floorf(seconds) > 0.5f);
     int shapeIndex = ((int)seconds) % TYPE_COUNT;
     if (wire) {
-        geometryCache->renderWireShape(batch, SHAPE[shapeIndex]);
+        geometryCache->renderWireShape(batch, SHAPE[shapeIndex], color);
     } else {
-        geometryCache->renderShape(batch, SHAPE[shapeIndex]);
+        geometryCache->renderShape(batch, SHAPE[shapeIndex], color);
     }
 
     batch.setModelTransform(Transform().setScale(1.01f));
-    batch._glColor4f(1, 1, 1, 1);
-    geometryCache->renderWireCube(batch);
+    geometryCache->renderWireCube(batch, glm::vec4(1,1,1,1));
 }