From 56a1d57a94f66f3ead577da058974b4f8f7752b5 Mon Sep 17 00:00:00 2001 From: samcake Date: Tue, 29 Nov 2016 11:57:50 -0800 Subject: [PATCH 1/3] removing separate vertex format from the gl45backend to test if it avoids the crash --- .../gpu-gl/src/gpu/gl45/GL45BackendInput.cpp | 106 ++++++++++++++++++ .../src/gpu/gl45/GL45BackendTransform.cpp | 14 +++ 2 files changed, 120 insertions(+) diff --git a/libraries/gpu-gl/src/gpu/gl45/GL45BackendInput.cpp b/libraries/gpu-gl/src/gpu/gl45/GL45BackendInput.cpp index 01bd2d7bce..97f98ecf09 100644 --- a/libraries/gpu-gl/src/gpu/gl45/GL45BackendInput.cpp +++ b/libraries/gpu-gl/src/gpu/gl45/GL45BackendInput.cpp @@ -17,16 +17,26 @@ using namespace gpu::gl45; void GL45Backend::resetInputStage() { Parent::resetInputStage(); +#ifdef GPU_USE_SEPARATE_VERTEX_FORMAT glBindBuffer(GL_ARRAY_BUFFER, 0); for (uint32_t i = 0; i < _input._attributeActivation.size(); i++) { glDisableVertexAttribArray(i); + glVertexAttribFormat(i, 4, GL_FLOAT, GL_FALSE, 0); } for (uint32_t i = 0; i < _input._attribBindingBuffers.size(); i++) { glBindVertexBuffer(i, 0, 0, 0); } +#else + glBindBuffer(GL_ARRAY_BUFFER, 0); + for (uint32_t i = 0; i < _input._attributeActivation.size(); i++) { + glDisableVertexAttribArray(i); + glVertexAttribPointer(i, 4, GL_FLOAT, GL_FALSE, 0, 0); + } +#endif } void GL45Backend::updateInput() { +#ifdef GPU_USE_SEPARATE_VERTEX_FORMAT if (_input._invalidFormat) { InputStageState::ActivationCache newActivation; @@ -116,4 +126,100 @@ void GL45Backend::updateInput() { _input._invalidBuffers.reset(); (void)CHECK_GL_ERROR(); } +#else + if (_input._invalidFormat || _input._invalidBuffers.any()) { + + if (_input._invalidFormat) { + InputStageState::ActivationCache newActivation; + + _stats._ISNumFormatChanges++; + + // Check expected activation + if (_input._format) { + for (auto& it : _input._format->getAttributes()) { + const Stream::Attribute& attrib = (it).second; + uint8_t locationCount = attrib._element.getLocationCount(); + for (int i = 0; i < locationCount; ++i) { + newActivation.set(attrib._slot + i); + } + } + } + + // Manage Activation what was and what is expected now + for (unsigned int i = 0; i < newActivation.size(); i++) { + bool newState = newActivation[i]; + if (newState != _input._attributeActivation[i]) { + + if (newState) { + glEnableVertexAttribArray(i); + } else { + glDisableVertexAttribArray(i); + } + (void)CHECK_GL_ERROR(); + + _input._attributeActivation.flip(i); + } + } + } + + // now we need to bind the buffers and assign the attrib pointers + if (_input._format) { + const Buffers& buffers = _input._buffers; + const Offsets& offsets = _input._bufferOffsets; + const Offsets& strides = _input._bufferStrides; + + const Stream::Format::AttributeMap& attributes = _input._format->getAttributes(); + auto& inputChannels = _input._format->getChannels(); + _stats._ISNumInputBufferChanges++; + + GLuint boundVBO = 0; + for (auto& channelIt : inputChannels) { + const Stream::Format::ChannelMap::value_type::second_type& channel = (channelIt).second; + if ((channelIt).first < buffers.size()) { + int bufferNum = (channelIt).first; + + if (_input._invalidBuffers.test(bufferNum) || _input._invalidFormat) { + // GLuint vbo = gpu::GL41Backend::getBufferID((*buffers[bufferNum])); + GLuint vbo = _input._bufferVBOs[bufferNum]; + if (boundVBO != vbo) { + glBindBuffer(GL_ARRAY_BUFFER, vbo); + (void)CHECK_GL_ERROR(); + boundVBO = vbo; + } + _input._invalidBuffers[bufferNum] = false; + + for (unsigned int i = 0; i < channel._slots.size(); i++) { + const Stream::Attribute& attrib = attributes.at(channel._slots[i]); + GLuint slot = attrib._slot; + GLuint count = attrib._element.getLocationScalarCount(); + uint8_t locationCount = attrib._element.getLocationCount(); + GLenum type = gl::ELEMENT_TYPE_TO_GL[attrib._element.getType()]; + // GLenum perLocationStride = strides[bufferNum]; + GLenum perLocationStride = attrib._element.getLocationSize(); + GLuint stride = (GLuint)strides[bufferNum]; + GLuint pointer = (GLuint)(attrib._offset + offsets[bufferNum]); + GLboolean isNormalized = attrib._element.isNormalized(); + + for (size_t locNum = 0; locNum < locationCount; ++locNum) { + glVertexAttribPointer(slot + (GLuint)locNum, count, type, isNormalized, stride, + reinterpret_cast(pointer + perLocationStride * (GLuint)locNum)); +#ifdef GPU_STEREO_DRAWCALL_INSTANCED + glVertexAttribDivisor(slot + (GLuint)locNum, attrib._frequency * (isStereo() ? 2 : 1)); +#else + glVertexAttribDivisor(slot + (GLuint)locNum, attrib._frequency); +#endif + } + + // TODO: Support properly the IAttrib version + + (void)CHECK_GL_ERROR(); + } + } + } + } + } + // everything format related should be in sync now + _input._invalidFormat = false; + } +#endif } diff --git a/libraries/gpu-gl/src/gpu/gl45/GL45BackendTransform.cpp b/libraries/gpu-gl/src/gpu/gl45/GL45BackendTransform.cpp index 57862eb983..f5f869add5 100644 --- a/libraries/gpu-gl/src/gpu/gl45/GL45BackendTransform.cpp +++ b/libraries/gpu-gl/src/gpu/gl45/GL45BackendTransform.cpp @@ -80,6 +80,7 @@ void GL45Backend::updateTransform(const Batch& batch) { } glVertexAttribI2i(gpu::Stream::DRAW_CALL_INFO, drawCallInfo.index, drawCallInfo.unused); } else { +#ifdef GPU_USE_SEPARATE_VERTEX_FORMAT if (!_transform._enabledDrawcallInfoBuffer) { glEnableVertexAttribArray(gpu::Stream::DRAW_CALL_INFO); // Make sure attrib array is enabled glVertexAttribIFormat(gpu::Stream::DRAW_CALL_INFO, 2, GL_UNSIGNED_SHORT, 0); @@ -95,6 +96,19 @@ void GL45Backend::updateTransform(const Batch& batch) { // so we must provide a stride. // This is in contrast to VertexAttrib*Pointer, where a zero signifies tightly-packed elements. glBindVertexBuffer(gpu::Stream::DRAW_CALL_INFO, _transform._drawCallInfoBuffer, (GLintptr)_transform._drawCallInfoOffsets[batch._currentNamedCall], 2 * sizeof(GLushort)); +#else + if (!_transform._enabledDrawcallInfoBuffer) { + glEnableVertexAttribArray(gpu::Stream::DRAW_CALL_INFO); // Make sure attrib array is enabled + glBindBuffer(GL_ARRAY_BUFFER, _transform._drawCallInfoBuffer); +#ifdef GPU_STEREO_DRAWCALL_INSTANCED + glVertexAttribDivisor(gpu::Stream::DRAW_CALL_INFO, (isStereo() ? 2 : 1)); +#else + glVertexAttribDivisor(gpu::Stream::DRAW_CALL_INFO, 1); +#endif + _transform._enabledDrawcallInfoBuffer = true; + } + glVertexAttribIPointer(gpu::Stream::DRAW_CALL_INFO, 2, GL_UNSIGNED_SHORT, 0, _transform._drawCallInfoOffsets[batch._currentNamedCall]); +#endif } (void)CHECK_GL_ERROR(); From b0900c5d09d2f560f80412be70ad50d4e4da2371 Mon Sep 17 00:00:00 2001 From: samcake Date: Fri, 2 Dec 2016 17:58:15 -0800 Subject: [PATCH 2/3] Revert "removing separate vertex format from the gl45backend to test if it avoids the crash" This reverts commit 56a1d57a94f66f3ead577da058974b4f8f7752b5. --- .../gpu-gl/src/gpu/gl45/GL45BackendInput.cpp | 106 ------------------ .../src/gpu/gl45/GL45BackendTransform.cpp | 14 --- 2 files changed, 120 deletions(-) diff --git a/libraries/gpu-gl/src/gpu/gl45/GL45BackendInput.cpp b/libraries/gpu-gl/src/gpu/gl45/GL45BackendInput.cpp index 97f98ecf09..01bd2d7bce 100644 --- a/libraries/gpu-gl/src/gpu/gl45/GL45BackendInput.cpp +++ b/libraries/gpu-gl/src/gpu/gl45/GL45BackendInput.cpp @@ -17,26 +17,16 @@ using namespace gpu::gl45; void GL45Backend::resetInputStage() { Parent::resetInputStage(); -#ifdef GPU_USE_SEPARATE_VERTEX_FORMAT glBindBuffer(GL_ARRAY_BUFFER, 0); for (uint32_t i = 0; i < _input._attributeActivation.size(); i++) { glDisableVertexAttribArray(i); - glVertexAttribFormat(i, 4, GL_FLOAT, GL_FALSE, 0); } for (uint32_t i = 0; i < _input._attribBindingBuffers.size(); i++) { glBindVertexBuffer(i, 0, 0, 0); } -#else - glBindBuffer(GL_ARRAY_BUFFER, 0); - for (uint32_t i = 0; i < _input._attributeActivation.size(); i++) { - glDisableVertexAttribArray(i); - glVertexAttribPointer(i, 4, GL_FLOAT, GL_FALSE, 0, 0); - } -#endif } void GL45Backend::updateInput() { -#ifdef GPU_USE_SEPARATE_VERTEX_FORMAT if (_input._invalidFormat) { InputStageState::ActivationCache newActivation; @@ -126,100 +116,4 @@ void GL45Backend::updateInput() { _input._invalidBuffers.reset(); (void)CHECK_GL_ERROR(); } -#else - if (_input._invalidFormat || _input._invalidBuffers.any()) { - - if (_input._invalidFormat) { - InputStageState::ActivationCache newActivation; - - _stats._ISNumFormatChanges++; - - // Check expected activation - if (_input._format) { - for (auto& it : _input._format->getAttributes()) { - const Stream::Attribute& attrib = (it).second; - uint8_t locationCount = attrib._element.getLocationCount(); - for (int i = 0; i < locationCount; ++i) { - newActivation.set(attrib._slot + i); - } - } - } - - // Manage Activation what was and what is expected now - for (unsigned int i = 0; i < newActivation.size(); i++) { - bool newState = newActivation[i]; - if (newState != _input._attributeActivation[i]) { - - if (newState) { - glEnableVertexAttribArray(i); - } else { - glDisableVertexAttribArray(i); - } - (void)CHECK_GL_ERROR(); - - _input._attributeActivation.flip(i); - } - } - } - - // now we need to bind the buffers and assign the attrib pointers - if (_input._format) { - const Buffers& buffers = _input._buffers; - const Offsets& offsets = _input._bufferOffsets; - const Offsets& strides = _input._bufferStrides; - - const Stream::Format::AttributeMap& attributes = _input._format->getAttributes(); - auto& inputChannels = _input._format->getChannels(); - _stats._ISNumInputBufferChanges++; - - GLuint boundVBO = 0; - for (auto& channelIt : inputChannels) { - const Stream::Format::ChannelMap::value_type::second_type& channel = (channelIt).second; - if ((channelIt).first < buffers.size()) { - int bufferNum = (channelIt).first; - - if (_input._invalidBuffers.test(bufferNum) || _input._invalidFormat) { - // GLuint vbo = gpu::GL41Backend::getBufferID((*buffers[bufferNum])); - GLuint vbo = _input._bufferVBOs[bufferNum]; - if (boundVBO != vbo) { - glBindBuffer(GL_ARRAY_BUFFER, vbo); - (void)CHECK_GL_ERROR(); - boundVBO = vbo; - } - _input._invalidBuffers[bufferNum] = false; - - for (unsigned int i = 0; i < channel._slots.size(); i++) { - const Stream::Attribute& attrib = attributes.at(channel._slots[i]); - GLuint slot = attrib._slot; - GLuint count = attrib._element.getLocationScalarCount(); - uint8_t locationCount = attrib._element.getLocationCount(); - GLenum type = gl::ELEMENT_TYPE_TO_GL[attrib._element.getType()]; - // GLenum perLocationStride = strides[bufferNum]; - GLenum perLocationStride = attrib._element.getLocationSize(); - GLuint stride = (GLuint)strides[bufferNum]; - GLuint pointer = (GLuint)(attrib._offset + offsets[bufferNum]); - GLboolean isNormalized = attrib._element.isNormalized(); - - for (size_t locNum = 0; locNum < locationCount; ++locNum) { - glVertexAttribPointer(slot + (GLuint)locNum, count, type, isNormalized, stride, - reinterpret_cast(pointer + perLocationStride * (GLuint)locNum)); -#ifdef GPU_STEREO_DRAWCALL_INSTANCED - glVertexAttribDivisor(slot + (GLuint)locNum, attrib._frequency * (isStereo() ? 2 : 1)); -#else - glVertexAttribDivisor(slot + (GLuint)locNum, attrib._frequency); -#endif - } - - // TODO: Support properly the IAttrib version - - (void)CHECK_GL_ERROR(); - } - } - } - } - } - // everything format related should be in sync now - _input._invalidFormat = false; - } -#endif } diff --git a/libraries/gpu-gl/src/gpu/gl45/GL45BackendTransform.cpp b/libraries/gpu-gl/src/gpu/gl45/GL45BackendTransform.cpp index f5f869add5..57862eb983 100644 --- a/libraries/gpu-gl/src/gpu/gl45/GL45BackendTransform.cpp +++ b/libraries/gpu-gl/src/gpu/gl45/GL45BackendTransform.cpp @@ -80,7 +80,6 @@ void GL45Backend::updateTransform(const Batch& batch) { } glVertexAttribI2i(gpu::Stream::DRAW_CALL_INFO, drawCallInfo.index, drawCallInfo.unused); } else { -#ifdef GPU_USE_SEPARATE_VERTEX_FORMAT if (!_transform._enabledDrawcallInfoBuffer) { glEnableVertexAttribArray(gpu::Stream::DRAW_CALL_INFO); // Make sure attrib array is enabled glVertexAttribIFormat(gpu::Stream::DRAW_CALL_INFO, 2, GL_UNSIGNED_SHORT, 0); @@ -96,19 +95,6 @@ void GL45Backend::updateTransform(const Batch& batch) { // so we must provide a stride. // This is in contrast to VertexAttrib*Pointer, where a zero signifies tightly-packed elements. glBindVertexBuffer(gpu::Stream::DRAW_CALL_INFO, _transform._drawCallInfoBuffer, (GLintptr)_transform._drawCallInfoOffsets[batch._currentNamedCall], 2 * sizeof(GLushort)); -#else - if (!_transform._enabledDrawcallInfoBuffer) { - glEnableVertexAttribArray(gpu::Stream::DRAW_CALL_INFO); // Make sure attrib array is enabled - glBindBuffer(GL_ARRAY_BUFFER, _transform._drawCallInfoBuffer); -#ifdef GPU_STEREO_DRAWCALL_INSTANCED - glVertexAttribDivisor(gpu::Stream::DRAW_CALL_INFO, (isStereo() ? 2 : 1)); -#else - glVertexAttribDivisor(gpu::Stream::DRAW_CALL_INFO, 1); -#endif - _transform._enabledDrawcallInfoBuffer = true; - } - glVertexAttribIPointer(gpu::Stream::DRAW_CALL_INFO, 2, GL_UNSIGNED_SHORT, 0, _transform._drawCallInfoOffsets[batch._currentNamedCall]); -#endif } (void)CHECK_GL_ERROR(); From d851278acddef7bf52224b6092045582476a43a2 Mon Sep 17 00:00:00 2001 From: samcake Date: Thu, 8 Dec 2016 15:04:57 -0800 Subject: [PATCH 3/3] FIx the highlight issue, the gloss 2 is actually roughnees to the power of 4 --- libraries/render-utils/src/local_lights_shading.slf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/render-utils/src/local_lights_shading.slf b/libraries/render-utils/src/local_lights_shading.slf index d3542fcdfa..0b884d8bc0 100644 --- a/libraries/render-utils/src/local_lights_shading.slf +++ b/libraries/render-utils/src/local_lights_shading.slf @@ -85,7 +85,7 @@ void main(void) { vec3 fragEyeDir = normalize(fragEyeVector.xyz); // COmpute the rougness into gloss2 once: - float fragGloss2 = pow(frag.roughness + 0.001, 2.0); + float fragGloss2 = pow(frag.roughness + 0.001, 4.0); bool withScattering = (frag.scattering * isScatteringEnabled() > 0.0); int numLightTouching = 0;