Commit e0cff190 by Geoff Lang Committed by Commit Bot

Validate that fragment shader output matches the draw buffer type.

TEST=conformance2/rendering/fs-color-type-mismatch-color-buffer-type BUG=angleproject:1688 Change-Id: I17848baf40b6d32b5adc1458fe2369b850164da3 Reviewed-on: https://chromium-review.googlesource.com/518246 Commit-Queue: Geoff Lang <geofflang@chromium.org> Reviewed-by: 's avatarFrank Henigman <fjhenigman@chromium.org>
parent 311d9999
...@@ -642,6 +642,26 @@ const FramebufferAttachment *Framebuffer::getDrawBuffer(size_t drawBuffer) const ...@@ -642,6 +642,26 @@ const FramebufferAttachment *Framebuffer::getDrawBuffer(size_t drawBuffer) const
return mState.getDrawBuffer(drawBuffer); return mState.getDrawBuffer(drawBuffer);
} }
GLenum Framebuffer::getDrawbufferWriteType(size_t drawBuffer) const
{
const FramebufferAttachment *attachment = mState.getDrawBuffer(drawBuffer);
if (attachment == nullptr)
{
return GL_NONE;
}
GLenum componentType = attachment->getFormat().info->componentType;
switch (componentType)
{
case GL_INT:
case GL_UNSIGNED_INT:
return componentType;
default:
return GL_FLOAT;
}
}
bool Framebuffer::hasEnabledDrawBuffer() const bool Framebuffer::hasEnabledDrawBuffer() const
{ {
for (size_t drawbufferIdx = 0; drawbufferIdx < mState.mDrawBufferStates.size(); ++drawbufferIdx) for (size_t drawbufferIdx = 0; drawbufferIdx < mState.mDrawBufferStates.size(); ++drawbufferIdx)
......
...@@ -167,6 +167,7 @@ class Framebuffer final : public LabeledObject, public OnAttachmentDirtyReceiver ...@@ -167,6 +167,7 @@ class Framebuffer final : public LabeledObject, public OnAttachmentDirtyReceiver
const std::vector<GLenum> &getDrawBufferStates() const; const std::vector<GLenum> &getDrawBufferStates() const;
void setDrawBuffers(size_t count, const GLenum *buffers); void setDrawBuffers(size_t count, const GLenum *buffers);
const FramebufferAttachment *getDrawBuffer(size_t drawBuffer) const; const FramebufferAttachment *getDrawBuffer(size_t drawBuffer) const;
GLenum getDrawbufferWriteType(size_t drawBuffer) const;
bool hasEnabledDrawBuffer() const; bool hasEnabledDrawBuffer() const;
GLenum getReadBufferState() const; GLenum getReadBufferState() const;
......
...@@ -770,6 +770,7 @@ void Program::unlink() ...@@ -770,6 +770,7 @@ void Program::unlink()
mState.mUniformBlocks.clear(); mState.mUniformBlocks.clear();
mState.mOutputVariables.clear(); mState.mOutputVariables.clear();
mState.mOutputLocations.clear(); mState.mOutputLocations.clear();
mState.mOutputVariableTypes.clear();
mState.mComputeShaderLocalSize.fill(1); mState.mComputeShaderLocalSize.fill(1);
mState.mSamplerBindings.clear(); mState.mSamplerBindings.clear();
...@@ -939,6 +940,12 @@ Error Program::loadBinary(const Context *context, ...@@ -939,6 +940,12 @@ Error Program::loadBinary(const Context *context,
mState.mOutputLocations[locationIndex] = locationData; mState.mOutputLocations[locationIndex] = locationData;
} }
unsigned int outputTypeCount = stream.readInt<unsigned int>();
for (unsigned int outputIndex = 0; outputIndex < outputTypeCount; ++outputIndex)
{
mState.mOutputVariableTypes.push_back(stream.readInt<GLenum>());
}
stream.readInt(&mState.mSamplerUniformRange.start); stream.readInt(&mState.mSamplerUniformRange.start);
stream.readInt(&mState.mSamplerUniformRange.end); stream.readInt(&mState.mSamplerUniformRange.end);
...@@ -1072,6 +1079,12 @@ Error Program::saveBinary(const Context *context, ...@@ -1072,6 +1079,12 @@ Error Program::saveBinary(const Context *context,
stream.writeString(outputPair.second.name); stream.writeString(outputPair.second.name);
} }
stream.writeInt(mState.mOutputVariableTypes.size());
for (const auto &outputVariableType : mState.mOutputVariableTypes)
{
stream.writeInt(outputVariableType);
}
stream.writeInt(mState.mSamplerUniformRange.start); stream.writeInt(mState.mSamplerUniformRange.start);
stream.writeInt(mState.mSamplerUniformRange.end); stream.writeInt(mState.mSamplerUniformRange.end);
...@@ -2665,6 +2678,32 @@ void Program::linkOutputVariables() ...@@ -2665,6 +2678,32 @@ void Program::linkOutputVariables()
const Shader *fragmentShader = mState.mAttachedFragmentShader; const Shader *fragmentShader = mState.mAttachedFragmentShader;
ASSERT(fragmentShader != nullptr); ASSERT(fragmentShader != nullptr);
ASSERT(mState.mOutputVariableTypes.empty());
// Gather output variable types
for (const auto &outputVariable : fragmentShader->getActiveOutputVariables())
{
if (outputVariable.isBuiltIn() && outputVariable.name != "gl_FragColor" &&
outputVariable.name != "gl_FragData")
{
continue;
}
unsigned int baseLocation =
(outputVariable.location == -1 ? 0u
: static_cast<unsigned int>(outputVariable.location));
for (unsigned int elementIndex = 0; elementIndex < outputVariable.elementCount();
elementIndex++)
{
const unsigned int location = baseLocation + elementIndex;
if (location >= mState.mOutputVariableTypes.size())
{
mState.mOutputVariableTypes.resize(location + 1, GL_NONE);
}
mState.mOutputVariableTypes[location] = VariableComponentType(outputVariable.type);
}
}
// Skip this step for GLES2 shaders. // Skip this step for GLES2 shaders.
if (fragmentShader->getShaderVersion() == 100) if (fragmentShader->getShaderVersion() == 100)
return; return;
......
...@@ -279,6 +279,9 @@ class ProgramState final : angle::NonCopyable ...@@ -279,6 +279,9 @@ class ProgramState final : angle::NonCopyable
// TODO(jmadill): use unordered/hash map when available // TODO(jmadill): use unordered/hash map when available
std::map<int, VariableLocation> mOutputLocations; std::map<int, VariableLocation> mOutputLocations;
// Fragment output variable base types: FLOAT, INT, or UINT. Ordered by location.
std::vector<GLenum> mOutputVariableTypes;
bool mBinaryRetrieveableHint; bool mBinaryRetrieveableHint;
bool mSeparable; bool mSeparable;
}; };
...@@ -354,6 +357,10 @@ class Program final : angle::NonCopyable, public LabeledObject ...@@ -354,6 +357,10 @@ class Program final : angle::NonCopyable, public LabeledObject
GLint getFragDataLocation(const std::string &name) const; GLint getFragDataLocation(const std::string &name) const;
size_t getOutputResourceCount() const; size_t getOutputResourceCount() const;
const std::vector<GLenum> &getOutputVariableTypes() const
{
return mState.mOutputVariableTypes;
}
void getActiveUniform(GLuint index, void getActiveUniform(GLuint index,
GLsizei bufsize, GLsizei bufsize,
......
...@@ -718,6 +718,28 @@ bool ValidateUniformMatrixValue(ValidationContext *context, GLenum valueType, GL ...@@ -718,6 +718,28 @@ bool ValidateUniformMatrixValue(ValidationContext *context, GLenum valueType, GL
return false; return false;
} }
bool ValidateFragmentShaderColorBufferTypeMatch(ValidationContext *context)
{
const Program *program = context->getGLState().getProgram();
const Framebuffer *framebuffer = context->getGLState().getDrawFramebuffer();
const auto &programOutputTypes = program->getOutputVariableTypes();
for (size_t drawBufferIdx = 0; drawBufferIdx < programOutputTypes.size(); drawBufferIdx++)
{
GLenum outputType = programOutputTypes[drawBufferIdx];
GLenum inputType = framebuffer->getDrawbufferWriteType(drawBufferIdx);
if (outputType != GL_NONE && inputType != GL_NONE && inputType != outputType)
{
context->handleError(Error(GL_INVALID_OPERATION,
"Fragment shader output type does not match the bound "
"framebuffer attachment type."));
return false;
}
}
return true;
}
} // anonymous namespace } // anonymous namespace
bool ValidTextureTarget(const ValidationContext *context, GLenum target) bool ValidTextureTarget(const ValidationContext *context, GLenum target)
...@@ -2713,9 +2735,10 @@ bool ValidateDrawBase(ValidationContext *context, GLenum mode, GLsizei count) ...@@ -2713,9 +2735,10 @@ bool ValidateDrawBase(ValidationContext *context, GLenum mode, GLsizei count)
} }
} }
// Detect rendering feedback loops for WebGL. // Do some additonal WebGL-specific validation
if (context->getExtensions().webglCompatibility) if (context->getExtensions().webglCompatibility)
{ {
// Detect rendering feedback loops for WebGL.
if (framebuffer->formsRenderingFeedbackLoopWith(state)) if (framebuffer->formsRenderingFeedbackLoopWith(state))
{ {
context->handleError( context->handleError(
...@@ -2723,6 +2746,12 @@ bool ValidateDrawBase(ValidationContext *context, GLenum mode, GLsizei count) ...@@ -2723,6 +2746,12 @@ bool ValidateDrawBase(ValidationContext *context, GLenum mode, GLsizei count)
"Rendering feedback loop formed between Framebuffer and active Texture.")); "Rendering feedback loop formed between Framebuffer and active Texture."));
return false; return false;
} }
// Detect that the color buffer types match the fragment shader output types
if (!ValidateFragmentShaderColorBufferTypeMatch(context))
{
return false;
}
} }
// No-op if zero count // No-op if zero count
......
...@@ -2333,6 +2333,98 @@ TEST_P(WebGL2CompatibilityTest, BlitFramebufferSameImage) ...@@ -2333,6 +2333,98 @@ TEST_P(WebGL2CompatibilityTest, BlitFramebufferSameImage)
ASSERT_GL_ERROR(GL_INVALID_OPERATION); ASSERT_GL_ERROR(GL_INVALID_OPERATION);
} }
// Verify that errors are generated when the fragment shader output doesn't match the bound color
// buffer types
TEST_P(WebGL2CompatibilityTest, FragmentShaderColorBufferTypeMissmatch)
{
const std::string vertexShader =
"#version 300 es\n"
"void main() {\n"
" gl_Position = vec4(0, 0, 0, 1);\n"
"}\n";
const std::string fragmentShader =
"#version 300 es\n"
"precision mediump float;\n"
"layout(location = 0) out vec4 floatOutput;\n"
"layout(location = 1) out uvec4 uintOutput;\n"
"layout(location = 2) out ivec4 intOutput;\n"
"void main() {\n"
" floatOutput = vec4(0, 0, 0, 1);\n"
" uintOutput = uvec4(0, 0, 0, 1);\n"
" intOutput = ivec4(0, 0, 0, 1);\n"
"}\n";
ANGLE_GL_PROGRAM(program, vertexShader, fragmentShader);
glUseProgram(program.get());
GLuint floatLocation = glGetFragDataLocation(program, "floatOutput");
GLuint uintLocation = glGetFragDataLocation(program, "uintOutput");
GLuint intLocation = glGetFragDataLocation(program, "intOutput");
GLFramebuffer fbo;
glBindFramebuffer(GL_FRAMEBUFFER, fbo);
GLRenderbuffer floatRenderbuffer;
glBindRenderbuffer(GL_RENDERBUFFER, floatRenderbuffer);
glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8, 1, 1);
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0 + floatLocation, GL_RENDERBUFFER,
floatRenderbuffer);
GLRenderbuffer uintRenderbuffer;
glBindRenderbuffer(GL_RENDERBUFFER, uintRenderbuffer);
glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8UI, 1, 1);
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0 + uintLocation, GL_RENDERBUFFER,
uintRenderbuffer);
GLRenderbuffer intRenderbuffer;
glBindRenderbuffer(GL_RENDERBUFFER, intRenderbuffer);
glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8I, 1, 1);
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0 + intLocation, GL_RENDERBUFFER,
intRenderbuffer);
ASSERT_GL_NO_ERROR();
GLint maxDrawBuffers = 0;
glGetIntegerv(GL_MAX_DRAW_BUFFERS, &maxDrawBuffers);
std::vector<GLenum> drawBuffers(static_cast<size_t>(maxDrawBuffers), GL_NONE);
drawBuffers[floatLocation] = GL_COLOR_ATTACHMENT0 + floatLocation;
drawBuffers[uintLocation] = GL_COLOR_ATTACHMENT0 + uintLocation;
drawBuffers[intLocation] = GL_COLOR_ATTACHMENT0 + intLocation;
glDrawBuffers(maxDrawBuffers, drawBuffers.data());
// Check that the correct case generates no errors
glDrawArrays(GL_TRIANGLES, 0, 6);
EXPECT_GL_NO_ERROR();
// Unbind some buffers and verify that there are still no errors
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0 + uintLocation, GL_RENDERBUFFER,
0);
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0 + intLocation, GL_RENDERBUFFER,
0);
glDrawArrays(GL_TRIANGLES, 0, 6);
EXPECT_GL_NO_ERROR();
// Swap the int and uint buffers to and verify that an error is generated
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0 + uintLocation, GL_RENDERBUFFER,
intRenderbuffer);
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0 + intLocation, GL_RENDERBUFFER,
uintRenderbuffer);
glDrawArrays(GL_TRIANGLES, 0, 6);
EXPECT_GL_ERROR(GL_INVALID_OPERATION);
// Swap the float and uint buffers to and verify that an error is generated
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0 + uintLocation, GL_RENDERBUFFER,
floatRenderbuffer);
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0 + floatLocation, GL_RENDERBUFFER,
uintRenderbuffer);
glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0 + intLocation, GL_RENDERBUFFER,
intRenderbuffer);
glDrawArrays(GL_TRIANGLES, 0, 6);
EXPECT_GL_ERROR(GL_INVALID_OPERATION);
}
// Use this to select which configurations (e.g. which renderer, which GLES major version) these // Use this to select which configurations (e.g. which renderer, which GLES major version) these
// tests should be run against. // tests should be run against.
ANGLE_INSTANTIATE_TEST(WebGLCompatibilityTest, ANGLE_INSTANTIATE_TEST(WebGLCompatibilityTest,
......
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