Commit b3b177d7 by Dzmitry Malyshau Committed by Commit Bot

Update driver constants on program change. Comes with a new SamplerMetadataUpdateOnSetProgram test.

This is a fix for a graphics problem we've been seeing for a while with WebRender+Angle on Nvidia/Windows. The sampler metadata doesn't get updated properly for some of the draw calls, since it's not invalidated on program change (this is what the CL is fixing). Extra entries get filled with garbage data because the constant buffer is updated with `MAP_WRITE_DISCARD`, and only those samplers are updated that the current program has. This may generally occur undetected, if not for our `textureSize` calls that appear to go the NV-specific Angle workaround path that ignores our `baseLevel = 0` and instead picks the one from the driver constants (which contains garbage), leading to either zeroes returned or even crashing the driver sometimes... BUG=angleproject:2399 Change-Id: Ie2bef32184e2305c7255299933b899eb3fffb7ab Reviewed-on: https://chromium-review.googlesource.com/949412Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Commit-Queue: Geoff Lang <geofflang@chromium.org>
parent 391bda23
...@@ -78,6 +78,7 @@ Klarälvdalens Datakonsult AB ...@@ -78,6 +78,7 @@ Klarälvdalens Datakonsult AB
Milian Wolff Milian Wolff
Mozilla Corp. Mozilla Corp.
Dzmitry Malyshau
Ehsan Akhgari Ehsan Akhgari
Edwin Flores Edwin Flores
Jeff Gilbert Jeff Gilbert
......
...@@ -252,9 +252,9 @@ ShaderConstants11::ShaderConstants11() ...@@ -252,9 +252,9 @@ ShaderConstants11::ShaderConstants11()
: mVertexDirty(true), : mVertexDirty(true),
mPixelDirty(true), mPixelDirty(true),
mComputeDirty(true), mComputeDirty(true),
mSamplerMetadataVSDirty(true), mNumActiveVSSamplers(0),
mSamplerMetadataPSDirty(true), mNumActivePSSamplers(0),
mSamplerMetadataCSDirty(true) mNumActiveCSSamplers(0)
{ {
} }
...@@ -287,12 +287,12 @@ size_t ShaderConstants11::getRequiredBufferSize(gl::ShaderType shaderType) const ...@@ -287,12 +287,12 @@ size_t ShaderConstants11::getRequiredBufferSize(gl::ShaderType shaderType) const
void ShaderConstants11::markDirty() void ShaderConstants11::markDirty()
{ {
mVertexDirty = true; mVertexDirty = true;
mPixelDirty = true; mPixelDirty = true;
mComputeDirty = true; mComputeDirty = true;
mSamplerMetadataVSDirty = true; mNumActiveVSSamplers = 0;
mSamplerMetadataPSDirty = true; mNumActivePSSamplers = 0;
mSamplerMetadataCSDirty = true; mNumActiveCSSamplers = 0;
} }
bool ShaderConstants11::updateSamplerMetadata(SamplerMetadata *data, const gl::Texture &texture) bool ShaderConstants11::updateSamplerMetadata(SamplerMetadata *data, const gl::Texture &texture)
...@@ -463,19 +463,19 @@ void ShaderConstants11::onSamplerChange(gl::ShaderType shaderType, ...@@ -463,19 +463,19 @@ void ShaderConstants11::onSamplerChange(gl::ShaderType shaderType,
case gl::SHADER_VERTEX: case gl::SHADER_VERTEX:
if (updateSamplerMetadata(&mSamplerMetadataVS[samplerIndex], texture)) if (updateSamplerMetadata(&mSamplerMetadataVS[samplerIndex], texture))
{ {
mSamplerMetadataVSDirty = true; mNumActiveVSSamplers = 0;
} }
break; break;
case gl::SHADER_FRAGMENT: case gl::SHADER_FRAGMENT:
if (updateSamplerMetadata(&mSamplerMetadataPS[samplerIndex], texture)) if (updateSamplerMetadata(&mSamplerMetadataPS[samplerIndex], texture))
{ {
mSamplerMetadataPSDirty = true; mNumActivePSSamplers = 0;
} }
break; break;
case gl::SHADER_COMPUTE: case gl::SHADER_COMPUTE:
if (updateSamplerMetadata(&mSamplerMetadataCS[samplerIndex], texture)) if (updateSamplerMetadata(&mSamplerMetadataCS[samplerIndex], texture))
{ {
mSamplerMetadataCSDirty = true; mNumActiveCSSamplers = 0;
} }
break; break;
default: default:
...@@ -494,31 +494,35 @@ gl::Error ShaderConstants11::updateBuffer(Renderer11 *renderer, ...@@ -494,31 +494,35 @@ gl::Error ShaderConstants11::updateBuffer(Renderer11 *renderer,
const uint8_t *data = nullptr; const uint8_t *data = nullptr;
const uint8_t *samplerData = nullptr; const uint8_t *samplerData = nullptr;
// Re-upload the sampler meta-data if the current program uses more samplers
// than we previously uploaded.
int numSamplers = programD3D.getUsedSamplerRange(shaderType);
switch (shaderType) switch (shaderType)
{ {
case gl::SHADER_VERTEX: case gl::SHADER_VERTEX:
dirty = mVertexDirty || mSamplerMetadataVSDirty; dirty = mVertexDirty || (mNumActiveVSSamplers < numSamplers);
dataSize = sizeof(Vertex); dataSize = sizeof(Vertex);
data = reinterpret_cast<const uint8_t *>(&mVertex); data = reinterpret_cast<const uint8_t *>(&mVertex);
samplerData = reinterpret_cast<const uint8_t *>(mSamplerMetadataVS.data()); samplerData = reinterpret_cast<const uint8_t *>(mSamplerMetadataVS.data());
mVertexDirty = false; mVertexDirty = false;
mSamplerMetadataVSDirty = false; mNumActiveVSSamplers = numSamplers;
break; break;
case gl::SHADER_FRAGMENT: case gl::SHADER_FRAGMENT:
dirty = mPixelDirty || mSamplerMetadataPSDirty; dirty = mPixelDirty || (mNumActivePSSamplers < numSamplers);
dataSize = sizeof(Pixel); dataSize = sizeof(Pixel);
data = reinterpret_cast<const uint8_t *>(&mPixel); data = reinterpret_cast<const uint8_t *>(&mPixel);
samplerData = reinterpret_cast<const uint8_t *>(mSamplerMetadataPS.data()); samplerData = reinterpret_cast<const uint8_t *>(mSamplerMetadataPS.data());
mPixelDirty = false; mPixelDirty = false;
mSamplerMetadataPSDirty = false; mNumActivePSSamplers = numSamplers;
break; break;
case gl::SHADER_COMPUTE: case gl::SHADER_COMPUTE:
dirty = mComputeDirty || mSamplerMetadataCSDirty; dirty = mComputeDirty || (mNumActiveCSSamplers < numSamplers);
dataSize = sizeof(Compute); dataSize = sizeof(Compute);
data = reinterpret_cast<const uint8_t *>(&mCompute); data = reinterpret_cast<const uint8_t *>(&mCompute);
samplerData = reinterpret_cast<const uint8_t *>(mSamplerMetadataCS.data()); samplerData = reinterpret_cast<const uint8_t *>(mSamplerMetadataCS.data());
mComputeDirty = false; mComputeDirty = false;
mSamplerMetadataCSDirty = false; mNumActiveCSSamplers = numSamplers;
break; break;
default: default:
UNREACHABLE(); UNREACHABLE();
...@@ -537,10 +541,10 @@ gl::Error ShaderConstants11::updateBuffer(Renderer11 *renderer, ...@@ -537,10 +541,10 @@ gl::Error ShaderConstants11::updateBuffer(Renderer11 *renderer,
ANGLE_TRY( ANGLE_TRY(
renderer->mapResource(driverConstantBuffer.get(), 0, D3D11_MAP_WRITE_DISCARD, 0, &mapping)); renderer->mapResource(driverConstantBuffer.get(), 0, D3D11_MAP_WRITE_DISCARD, 0, &mapping));
size_t samplerDataBytes = sizeof(SamplerMetadata) * programD3D.getUsedSamplerRange(shaderType);
memcpy(mapping.pData, data, dataSize); memcpy(mapping.pData, data, dataSize);
memcpy(reinterpret_cast<uint8_t *>(mapping.pData) + dataSize, samplerData, samplerDataBytes); memcpy(reinterpret_cast<uint8_t *>(mapping.pData) + dataSize,
samplerData,
sizeof(SamplerMetadata) * numSamplers);
renderer->getDeviceContext()->Unmap(driverConstantBuffer.get(), 0); renderer->getDeviceContext()->Unmap(driverConstantBuffer.get(), 0);
...@@ -1014,6 +1018,7 @@ void StateManager11::syncState(const gl::Context *context, const gl::State::Dirt ...@@ -1014,6 +1018,7 @@ void StateManager11::syncState(const gl::Context *context, const gl::State::Dirt
invalidateTexturesAndSamplers(); invalidateTexturesAndSamplers();
invalidateProgramUniforms(); invalidateProgramUniforms();
invalidateProgramUniformBuffers(); invalidateProgramUniformBuffers();
invalidateDriverUniforms();
gl::VertexArray *vao = state.getVertexArray(); gl::VertexArray *vao = state.getVertexArray();
if (mIsMultiviewEnabled && vao != nullptr) if (mIsMultiviewEnabled && vao != nullptr)
{ {
......
...@@ -134,11 +134,11 @@ class ShaderConstants11 : angle::NonCopyable ...@@ -134,11 +134,11 @@ class ShaderConstants11 : angle::NonCopyable
bool mComputeDirty; bool mComputeDirty;
std::vector<SamplerMetadata> mSamplerMetadataVS; std::vector<SamplerMetadata> mSamplerMetadataVS;
bool mSamplerMetadataVSDirty; int mNumActiveVSSamplers;
std::vector<SamplerMetadata> mSamplerMetadataPS; std::vector<SamplerMetadata> mSamplerMetadataPS;
bool mSamplerMetadataPSDirty; int mNumActivePSSamplers;
std::vector<SamplerMetadata> mSamplerMetadataCS; std::vector<SamplerMetadata> mSamplerMetadataCS;
bool mSamplerMetadataCSDirty; int mNumActiveCSSamplers;
}; };
class DrawCallVertexParams final : angle::NonCopyable class DrawCallVertexParams final : angle::NonCopyable
......
...@@ -716,6 +716,91 @@ TEST_P(StateChangeTestES3, VertexArrayObjectAndDisabledAttributes) ...@@ -716,6 +716,91 @@ TEST_P(StateChangeTestES3, VertexArrayObjectAndDisabledAttributes)
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::black); EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::black);
} }
const char kSamplerMetadataVertexShader0[] = R"(#version 300 es
precision mediump float;
out vec4 color;
uniform sampler2D texture;
void main()
{
vec2 size = vec2(textureSize(texture, 0));
color = size.x != 0.0 ? vec4(0.0, 1.0, 0.0, 1.0) : vec4(1.0, 0.0, 0.0, 0.0);
vec2 pos = vec2(0.0);
switch (gl_VertexID) {
case 0: pos = vec2(-1.0, -1.0); break;
case 1: pos = vec2(3.0, -1.0); break;
case 2: pos = vec2(-1.0, 3.0); break;
};
gl_Position = vec4(pos, 0.0, 1.0);
})";
const char kSamplerMetadataVertexShader1[] = R"(#version 300 es
precision mediump float;
out vec4 color;
uniform sampler2D texture1;
uniform sampler2D texture2;
void main()
{
vec2 size1 = vec2(textureSize(texture1, 0));
vec2 size2 = vec2(textureSize(texture2, 0));
color = size1.x * size2.x != 0.0 ? vec4(0.0, 1.0, 0.0, 1.0) : vec4(1.0, 0.0, 0.0, 0.0);
vec2 pos = vec2(0.0);
switch (gl_VertexID) {
case 0: pos = vec2(-1.0, -1.0); break;
case 1: pos = vec2(3.0, -1.0); break;
case 2: pos = vec2(-1.0, 3.0); break;
};
gl_Position = vec4(pos, 0.0, 1.0);
})";
const char kSamplerMetadataFragmentShader[] = R"(#version 300 es
precision mediump float;
in vec4 color;
out vec4 result;
void main()
{
result = color;
})";
// Tests that changing an active program invalidates the sampler metadata properly.
TEST_P(StateChangeTestES3, SamplerMetadataUpdateOnSetProgram)
{
GLVertexArray vertexArray;
glBindVertexArray(vertexArray);
// Create a simple framebuffer.
GLTexture texture1, texture2;
glActiveTexture(GL_TEXTURE0);
glBindTexture(GL_TEXTURE_2D, texture1);
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 2, 2, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
glActiveTexture(GL_TEXTURE1);
glBindTexture(GL_TEXTURE_2D, texture2);
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 3, 3, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr);
// Create 2 shader programs differing only in the number of active samplers.
ANGLE_GL_PROGRAM(program1, kSamplerMetadataVertexShader0, kSamplerMetadataFragmentShader);
glUseProgram(program1);
glUniform1i(glGetUniformLocation(program1, "texture"), 0);
ANGLE_GL_PROGRAM(program2, kSamplerMetadataVertexShader1, kSamplerMetadataFragmentShader);
glUseProgram(program2);
glUniform1i(glGetUniformLocation(program2, "texture1"), 0);
glUniform1i(glGetUniformLocation(program2, "texture2"), 0);
// Draw a solid green color to the framebuffer.
glUseProgram(program1);
glDrawArrays(GL_TRIANGLES, 0, 3);
// Test that our first program is good.
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
// Bind a different program that uses more samplers.
// Draw another quad that depends on the sampler metadata.
glUseProgram(program2);
glDrawArrays(GL_TRIANGLES, 0, 3);
// Flush via ReadPixels and check that it's still green.
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
ASSERT_GL_NO_ERROR();
}
// Simple state change tests, primarily focused on basic object lifetime and dependency management // Simple state change tests, primarily focused on basic object lifetime and dependency management
// with back-ends that don't support that automatically (i.e. Vulkan). // with back-ends that don't support that automatically (i.e. Vulkan).
class SimpleStateChangeTest : public ANGLETest class SimpleStateChangeTest : public ANGLETest
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment