Re-apply UBO binding workaround on program save.
The workaround which was previously defined to only apply on load
also seems to affect save on some AMD drivers.
BUG=angleproject:1637
BUG=angleproject:1897
Change-Id: Ia01a1420a484f3c2682ce97eaab18baccfb66a50
Reviewed-on: https://chromium-review.googlesource.com/558008
Reviewed-by: Yuly Novikov <ynovikov@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
diff --git a/src/libANGLE/MemoryProgramCache.cpp b/src/libANGLE/MemoryProgramCache.cpp
index 3dd1b94..8fa1c49 100644
--- a/src/libANGLE/MemoryProgramCache.cpp
+++ b/src/libANGLE/MemoryProgramCache.cpp
@@ -437,7 +437,7 @@
stream.writeInt(imageBinding.elementCount);
}
- program->getImplementation()->save(&stream);
+ program->getImplementation()->save(context, &stream);
ASSERT(binaryOut);
binaryOut->resize(stream.length());
diff --git a/src/libANGLE/renderer/ProgramImpl.h b/src/libANGLE/renderer/ProgramImpl.h
index a0fe6b9..d2b7118 100644
--- a/src/libANGLE/renderer/ProgramImpl.h
+++ b/src/libANGLE/renderer/ProgramImpl.h
@@ -41,7 +41,7 @@
virtual gl::LinkResult load(const gl::Context *context,
gl::InfoLog &infoLog,
gl::BinaryInputStream *stream) = 0;
- virtual void save(gl::BinaryOutputStream *stream) = 0;
+ virtual void save(const gl::Context *context, gl::BinaryOutputStream *stream) = 0;
virtual void setBinaryRetrievableHint(bool retrievable) = 0;
virtual void setSeparable(bool separable) = 0;
diff --git a/src/libANGLE/renderer/ProgramImpl_mock.h b/src/libANGLE/renderer/ProgramImpl_mock.h
index 92e5260..8883ca4 100644
--- a/src/libANGLE/renderer/ProgramImpl_mock.h
+++ b/src/libANGLE/renderer/ProgramImpl_mock.h
@@ -24,7 +24,7 @@
virtual ~MockProgramImpl() { destructor(); }
MOCK_METHOD3(load, gl::LinkResult(const gl::Context *, gl::InfoLog &, gl::BinaryInputStream *));
- MOCK_METHOD1(save, void(gl::BinaryOutputStream *));
+ MOCK_METHOD2(save, void(const gl::Context *, gl::BinaryOutputStream *));
MOCK_METHOD1(setBinaryRetrievableHint, void(bool));
MOCK_METHOD1(setSeparable, void(bool));
diff --git a/src/libANGLE/renderer/d3d/ProgramD3D.cpp b/src/libANGLE/renderer/d3d/ProgramD3D.cpp
index fe6f80f..c8cc0ca 100644
--- a/src/libANGLE/renderer/d3d/ProgramD3D.cpp
+++ b/src/libANGLE/renderer/d3d/ProgramD3D.cpp
@@ -959,7 +959,7 @@
return true;
}
-void ProgramD3D::save(gl::BinaryOutputStream *stream)
+void ProgramD3D::save(const gl::Context *context, gl::BinaryOutputStream *stream)
{
// Output the DeviceIdentifier before we output any shader code
// When we load the binary again later, we can validate the device identifier before trying to
diff --git a/src/libANGLE/renderer/d3d/ProgramD3D.h b/src/libANGLE/renderer/d3d/ProgramD3D.h
index 8c5c2a8..7f0a48b 100644
--- a/src/libANGLE/renderer/d3d/ProgramD3D.h
+++ b/src/libANGLE/renderer/d3d/ProgramD3D.h
@@ -161,7 +161,7 @@
gl::LinkResult load(const gl::Context *context,
gl::InfoLog &infoLog,
gl::BinaryInputStream *stream) override;
- void save(gl::BinaryOutputStream *stream) override;
+ void save(const gl::Context *context, gl::BinaryOutputStream *stream) override;
void setBinaryRetrievableHint(bool retrievable) override;
void setSeparable(bool separable) override;
diff --git a/src/libANGLE/renderer/gl/ProgramGL.cpp b/src/libANGLE/renderer/gl/ProgramGL.cpp
index ec26143..2ea74e2 100644
--- a/src/libANGLE/renderer/gl/ProgramGL.cpp
+++ b/src/libANGLE/renderer/gl/ProgramGL.cpp
@@ -71,22 +71,12 @@
}
postLink();
-
- // Re-apply UBO bindings to work around driver bugs.
- const WorkaroundsGL &workaroundsGL = GetImplAs<ContextGL>(context)->getWorkaroundsGL();
- if (workaroundsGL.reapplyUBOBindingsAfterLoadingBinaryProgram)
- {
- const auto &blocks = mState.getUniformBlocks();
- for (size_t blockIndex : mState.getActiveUniformBlockBindingsMask())
- {
- setUniformBlockBinding(static_cast<GLuint>(blockIndex), blocks[blockIndex].binding);
- }
- }
+ reapplyUBOBindingsIfNeeded(context);
return true;
}
-void ProgramGL::save(gl::BinaryOutputStream *stream)
+void ProgramGL::save(const gl::Context *context, gl::BinaryOutputStream *stream)
{
GLint binaryLength = 0;
mFunctions->getProgramiv(mProgramID, GL_PROGRAM_BINARY_LENGTH, &binaryLength);
@@ -99,6 +89,22 @@
stream->writeInt(binaryFormat);
stream->writeInt(binaryLength);
stream->writeBytes(&binary[0], binaryLength);
+
+ reapplyUBOBindingsIfNeeded(context);
+}
+
+void ProgramGL::reapplyUBOBindingsIfNeeded(const gl::Context *context)
+{
+ // Re-apply UBO bindings to work around driver bugs.
+ const WorkaroundsGL &workaroundsGL = GetImplAs<ContextGL>(context)->getWorkaroundsGL();
+ if (workaroundsGL.reapplyUBOBindingsAfterUsingBinaryProgram)
+ {
+ const auto &blocks = mState.getUniformBlocks();
+ for (size_t blockIndex : mState.getActiveUniformBlockBindingsMask())
+ {
+ setUniformBlockBinding(static_cast<GLuint>(blockIndex), blocks[blockIndex].binding);
+ }
+ }
}
void ProgramGL::setBinaryRetrievableHint(bool retrievable)
diff --git a/src/libANGLE/renderer/gl/ProgramGL.h b/src/libANGLE/renderer/gl/ProgramGL.h
index 7d79b2d..29ffb1d 100644
--- a/src/libANGLE/renderer/gl/ProgramGL.h
+++ b/src/libANGLE/renderer/gl/ProgramGL.h
@@ -34,7 +34,7 @@
gl::LinkResult load(const gl::Context *contextImpl,
gl::InfoLog &infoLog,
gl::BinaryInputStream *stream) override;
- void save(gl::BinaryOutputStream *stream) override;
+ void save(const gl::Context *context, gl::BinaryOutputStream *stream) override;
void setBinaryRetrievableHint(bool retrievable) override;
void setSeparable(bool separable) override;
@@ -82,6 +82,7 @@
void preLink();
bool checkLinkStatus(gl::InfoLog &infoLog);
void postLink();
+ void reapplyUBOBindingsIfNeeded(const gl::Context *context);
// Helper function, makes it simpler to type.
GLint uniLoc(GLint glLocation) const { return mUniformRealLocationMap[glLocation]; }
diff --git a/src/libANGLE/renderer/gl/WorkaroundsGL.h b/src/libANGLE/renderer/gl/WorkaroundsGL.h
index 7b456ef..21367ad 100644
--- a/src/libANGLE/renderer/gl/WorkaroundsGL.h
+++ b/src/libANGLE/renderer/gl/WorkaroundsGL.h
@@ -114,11 +114,11 @@
// Tracking bug: http://crbug.com/672380
bool emulateAtan2Float = false;
- // Some drivers seem to forget about UBO bindings when loading program binaries. Work around
- // this by re-applying the bindings after the program binary is loaded.
+ // Some drivers seem to forget about UBO bindings when using program binaries. Work around
+ // this by re-applying the bindings after the program binary is loaded or saved.
// This only seems to affect AMD OpenGL drivers, and some Android devices.
// http://anglebug.com/1637
- bool reapplyUBOBindingsAfterLoadingBinaryProgram = false;
+ bool reapplyUBOBindingsAfterUsingBinaryProgram = false;
// Some OpenGL drivers return 0 when we query MAX_VERTEX_ATTRIB_STRIDE in an OpenGL 4.4 or
// higher context.
diff --git a/src/libANGLE/renderer/gl/renderergl_utils.cpp b/src/libANGLE/renderer/gl/renderergl_utils.cpp
index 357405e..45dfb66 100644
--- a/src/libANGLE/renderer/gl/renderergl_utils.cpp
+++ b/src/libANGLE/renderer/gl/renderergl_utils.cpp
@@ -998,11 +998,11 @@
// 364 are known to be affected, at least up to 375.
workarounds->emulateAtan2Float = IsNvidia(vendor);
- workarounds->reapplyUBOBindingsAfterLoadingBinaryProgram = IsAMD(vendor);
+ workarounds->reapplyUBOBindingsAfterUsingBinaryProgram = IsAMD(vendor);
#if defined(ANGLE_PLATFORM_ANDROID)
// TODO(jmadill): Narrow workaround range for specific devices.
- workarounds->reapplyUBOBindingsAfterLoadingBinaryProgram = true;
+ workarounds->reapplyUBOBindingsAfterUsingBinaryProgram = true;
#endif
}
diff --git a/src/libANGLE/renderer/null/ProgramNULL.cpp b/src/libANGLE/renderer/null/ProgramNULL.cpp
index 52edc35..cee30dd 100644
--- a/src/libANGLE/renderer/null/ProgramNULL.cpp
+++ b/src/libANGLE/renderer/null/ProgramNULL.cpp
@@ -29,7 +29,7 @@
return true;
}
-void ProgramNULL::save(gl::BinaryOutputStream *stream)
+void ProgramNULL::save(const gl::Context *context, gl::BinaryOutputStream *stream)
{
}
diff --git a/src/libANGLE/renderer/null/ProgramNULL.h b/src/libANGLE/renderer/null/ProgramNULL.h
index 9b6fda3..363d334 100644
--- a/src/libANGLE/renderer/null/ProgramNULL.h
+++ b/src/libANGLE/renderer/null/ProgramNULL.h
@@ -24,7 +24,7 @@
gl::LinkResult load(const gl::Context *context,
gl::InfoLog &infoLog,
gl::BinaryInputStream *stream) override;
- void save(gl::BinaryOutputStream *stream) override;
+ void save(const gl::Context *context, gl::BinaryOutputStream *stream) override;
void setBinaryRetrievableHint(bool retrievable) override;
void setSeparable(bool separable) override;
diff --git a/src/libANGLE/renderer/vulkan/ProgramVk.cpp b/src/libANGLE/renderer/vulkan/ProgramVk.cpp
index 6a5b90c..36cbb29 100644
--- a/src/libANGLE/renderer/vulkan/ProgramVk.cpp
+++ b/src/libANGLE/renderer/vulkan/ProgramVk.cpp
@@ -43,7 +43,7 @@
return gl::InternalError();
}
-void ProgramVk::save(gl::BinaryOutputStream *stream)
+void ProgramVk::save(const gl::Context *context, gl::BinaryOutputStream *stream)
{
UNIMPLEMENTED();
}
diff --git a/src/libANGLE/renderer/vulkan/ProgramVk.h b/src/libANGLE/renderer/vulkan/ProgramVk.h
index eadbd74..c24cc3f 100644
--- a/src/libANGLE/renderer/vulkan/ProgramVk.h
+++ b/src/libANGLE/renderer/vulkan/ProgramVk.h
@@ -26,7 +26,7 @@
gl::LinkResult load(const gl::Context *context,
gl::InfoLog &infoLog,
gl::BinaryInputStream *stream) override;
- void save(gl::BinaryOutputStream *stream) override;
+ void save(const gl::Context *context, gl::BinaryOutputStream *stream) override;
void setBinaryRetrievableHint(bool retrievable) override;
void setSeparable(bool separable) override;
diff --git a/src/tests/gl_tests/UniformBufferTest.cpp b/src/tests/gl_tests/UniformBufferTest.cpp
index 2ad13f6..0d931b7 100644
--- a/src/tests/gl_tests/UniformBufferTest.cpp
+++ b/src/tests/gl_tests/UniformBufferTest.cpp
@@ -676,13 +676,6 @@
// Test uniform block bindings specified by layout in shader work properly.
TEST_P(UniformBufferTest31, UniformBufferBindings)
{
- // TODO(jmadill): Figure out why this fails on Linux AMD OpenGL
- if (IsLinux() && IsAMD() && IsDesktopOpenGL())
- {
- std::cout << "Test skipped on Linux OpenGL on AMD." << std::endl;
- return;
- }
-
const std::string &vertexShaderSource =
"#version 310 es\n"
"in vec4 position;\n"
@@ -748,13 +741,6 @@
// Test uniform blocks used as instanced array take next binding point for each subsequent element.
TEST_P(UniformBufferTest31, ConsecutiveBindingsForBlockArray)
{
- // TODO(jmadill): Figure out why this fails on Linux AMD OpenGL
- if (IsLinux() && IsAMD() && IsDesktopOpenGL())
- {
- std::cout << "Test skipped on Linux OpenGL on AMD." << std::endl;
- return;
- }
-
const std::string &vertexShaderSource =
"#version 310 es\n"
"in vec4 position;\n"