From bd666406f4635168b0dc1f4ce68810a6b580e13d Mon Sep 17 00:00:00 2001
From: Olivier Prat <olivier@zvork.fr>
Date: Tue, 19 Dec 2017 14:17:17 +0100
Subject: [PATCH] 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
---
.../src/RenderablePolyLineEntityItem.cpp | 1 +
.../src/RenderableShapeEntityItem.cpp | 5 ++--
.../src/RenderableWebEntityItem.cpp | 1 -
libraries/gpu-gl/src/gpu/gl/GLBackend.cpp | 4 +++
libraries/gpu-gl/src/gpu/gl/GLBackend.h | 1 +
.../gpu-gl/src/gpu/gl41/GL41BackendInput.cpp | 13 ++++++++
.../gpu-gl/src/gpu/gl45/GL45BackendInput.cpp | 14 +++++++++
libraries/render-utils/src/GeometryCache.cpp | 30 +++++++++++++++++++
libraries/render-utils/src/GeometryCache.h | 6 ++++
.../render-utils/src/MeshPartPayload.cpp | 10 -------
tests/gpu-test/src/TestShapes.cpp | 9 +++---
11 files changed, 75 insertions(+), 19 deletions(-)
diff --git a/libraries/entities-renderer/src/RenderablePolyLineEntityItem.cpp b/libraries/entities-renderer/src/RenderablePolyLineEntityItem.cpp
index fbf85fa8c2..21764dff7f 100644
--- a/libraries/entities-renderer/src/RenderablePolyLineEntityItem.cpp
+++ b/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 {
diff --git a/libraries/entities-renderer/src/RenderableShapeEntityItem.cpp b/libraries/entities-renderer/src/RenderableShapeEntityItem.cpp
index 3524709395..b00ad0259e 100644
--- a/libraries/entities-renderer/src/RenderableShapeEntityItem.cpp
+++ b/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
diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp
index cdd260e73c..b253e73b87 100644
--- a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp
+++ b/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);
diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp b/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp
index 6fb0d7b152..93077e240b 100644
--- a/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp
+++ b/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();
}
diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackend.h b/libraries/gpu-gl/src/gpu/gl/GLBackend.h
index 1908db614d..5558d3ada1 100644
--- a/libraries/gpu-gl/src/gpu/gl/GLBackend.h
+++ b/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;
diff --git a/libraries/gpu-gl/src/gpu/gl41/GL41BackendInput.cpp b/libraries/gpu-gl/src/gpu/gl41/GL41BackendInput.cpp
index e8ebcbe05c..42bd56e6e4 100644
--- a/libraries/gpu-gl/src/gpu/gl41/GL41BackendInput.cpp
+++ b/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;
diff --git a/libraries/gpu-gl/src/gpu/gl45/GL45BackendInput.cpp b/libraries/gpu-gl/src/gpu/gl45/GL45BackendInput.cpp
index ece62e15f1..4a43fc988c 100644
--- a/libraries/gpu-gl/src/gpu/gl45/GL45BackendInput.cpp
+++ b/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
diff --git a/libraries/render-utils/src/GeometryCache.cpp b/libraries/render-utils/src/GeometryCache.cpp
index 76c354bdf8..2616d08600 100644
--- a/libraries/render-utils/src/GeometryCache.cpp
+++ b/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,
diff --git a/libraries/render-utils/src/GeometryCache.h b/libraries/render-utils/src/GeometryCache.h
index cd8c43f1df..0585cc9e55 100644
--- a/libraries/render-utils/src/GeometryCache.h
+++ b/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,
diff --git a/libraries/render-utils/src/MeshPartPayload.cpp b/libraries/render-utils/src/MeshPartPayload.cpp
index 1ea3e1a705..cf895a73fa 100644
--- a/libraries/render-utils/src/MeshPartPayload.cpp
+++ b/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 {
diff --git a/tests/gpu-test/src/TestShapes.cpp b/tests/gpu-test/src/TestShapes.cpp
index 253d89cf61..67a348c002 100644
--- a/tests/gpu-test/src/TestShapes.cpp
+++ b/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));
}