Commit 12b6a82e by Tim Van Patten Committed by Commit Bot

No-Op draws when no active VS and/or FS is present

Re-land CL with WebGL fixes: This required some extra pointer checking during validation to handle the fact that a Program and/or ProgramExecutable may not be present when attempting to draw. This isn't an error, just undefined behavior, which we (eventually) treat as a no-op. According to the OpenGL ES 3.1 spec: 7.3. PROGRAM OBJECTS If there is no active program for the vertex or fragment shader stages, the results of vertex and fragment shader execution will respectively be undefined. However, this is not an error. To handle this, if no VS or FS is present in the active Program/PPO, we will no-op the draw command. Bug: angleproject:3570 Test: KHR-GLES31.core.sepshaderobjs.StateInteraction Change-Id: I70d688bf344a78cf3b4fd66c995ae03ce4b9b807 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2185156Reviewed-by: 's avatarCourtney Goeltzenleuchter <courtneygo@google.com> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Tim Van Patten <timvp@google.com>
parent e25af112
...@@ -1124,6 +1124,7 @@ void Context::bindProgramPipeline(ProgramPipelineID pipelineHandle) ...@@ -1124,6 +1124,7 @@ void Context::bindProgramPipeline(ProgramPipelineID pipelineHandle)
ProgramPipeline *pipeline = mState.mProgramPipelineManager->checkProgramPipelineAllocation( ProgramPipeline *pipeline = mState.mProgramPipelineManager->checkProgramPipelineAllocation(
mImplementation.get(), pipelineHandle); mImplementation.get(), pipelineHandle);
ANGLE_CONTEXT_TRY(mState.setProgramPipelineBinding(this, pipeline)); ANGLE_CONTEXT_TRY(mState.setProgramPipelineBinding(this, pipeline));
mStateCache.onProgramExecutableChange(this);
} }
void Context::beginQuery(QueryType target, QueryID query) void Context::beginQuery(QueryType target, QueryID query)
...@@ -8492,7 +8493,8 @@ StateCache::StateCache() ...@@ -8492,7 +8493,8 @@ StateCache::StateCache()
mCachedInstancedVertexElementLimit(0), mCachedInstancedVertexElementLimit(0),
mCachedBasicDrawStatesError(kInvalidPointer), mCachedBasicDrawStatesError(kInvalidPointer),
mCachedBasicDrawElementsError(kInvalidPointer), mCachedBasicDrawElementsError(kInvalidPointer),
mCachedTransformFeedbackActiveUnpaused(false) mCachedTransformFeedbackActiveUnpaused(false),
mCachedCanDraw(false)
{} {}
StateCache::~StateCache() = default; StateCache::~StateCache() = default;
...@@ -8513,6 +8515,7 @@ void StateCache::initialize(Context *context) ...@@ -8513,6 +8515,7 @@ void StateCache::initialize(Context *context)
updateBasicDrawStatesError(); updateBasicDrawStatesError();
updateBasicDrawElementsError(); updateBasicDrawElementsError();
updateVertexAttribTypesValidation(context); updateVertexAttribTypesValidation(context);
updateCanDraw(context);
} }
void StateCache::updateActiveAttribsMask(Context *context) void StateCache::updateActiveAttribsMask(Context *context)
...@@ -8624,6 +8627,7 @@ void StateCache::onProgramExecutableChange(Context *context) ...@@ -8624,6 +8627,7 @@ void StateCache::onProgramExecutableChange(Context *context)
updateBasicDrawStatesError(); updateBasicDrawStatesError();
updateValidDrawModes(context); updateValidDrawModes(context);
updateActiveShaderStorageBufferIndices(context); updateActiveShaderStorageBufferIndices(context);
updateCanDraw(context);
} }
void StateCache::onVertexArrayFormatChange(Context *context) void StateCache::onVertexArrayFormatChange(Context *context)
...@@ -8885,4 +8889,11 @@ void StateCache::updateActiveShaderStorageBufferIndices(Context *context) ...@@ -8885,4 +8889,11 @@ void StateCache::updateActiveShaderStorageBufferIndices(Context *context)
} }
} }
} }
void StateCache::updateCanDraw(Context *context)
{
mCachedCanDraw = (context->isGLES1() ||
(context->getState().getProgramExecutable() &&
context->getState().getProgramExecutable()->hasVertexAndFragmentShader()));
}
} // namespace gl } // namespace gl
...@@ -257,6 +257,10 @@ class StateCache final : angle::NonCopyable ...@@ -257,6 +257,10 @@ class StateCache final : angle::NonCopyable
return mCachedActiveShaderStorageBufferIndices; return mCachedActiveShaderStorageBufferIndices;
} }
// Places that can trigger updateCanDraw:
// 1. onProgramExecutableChange.
bool getCanDraw() const { return mCachedCanDraw; }
// State change notifications. // State change notifications.
void onVertexArrayBindingChange(Context *context); void onVertexArrayBindingChange(Context *context);
void onProgramExecutableChange(Context *context); void onProgramExecutableChange(Context *context);
...@@ -290,6 +294,7 @@ class StateCache final : angle::NonCopyable ...@@ -290,6 +294,7 @@ class StateCache final : angle::NonCopyable
void updateTransformFeedbackActiveUnpaused(Context *context); void updateTransformFeedbackActiveUnpaused(Context *context);
void updateVertexAttribTypesValidation(Context *context); void updateVertexAttribTypesValidation(Context *context);
void updateActiveShaderStorageBufferIndices(Context *context); void updateActiveShaderStorageBufferIndices(Context *context);
void updateCanDraw(Context *context);
void setValidDrawModes(bool pointsOK, bool linesOK, bool trisOK, bool lineAdjOK, bool triAdjOK); void setValidDrawModes(bool pointsOK, bool linesOK, bool trisOK, bool lineAdjOK, bool triAdjOK);
...@@ -324,6 +329,8 @@ class StateCache final : angle::NonCopyable ...@@ -324,6 +329,8 @@ class StateCache final : angle::NonCopyable
VertexAttribTypeCase, VertexAttribTypeCase,
angle::EnumSize<VertexAttribType>() + 1> angle::EnumSize<VertexAttribType>() + 1>
mCachedIntegerVertexAttribTypesValidation; mCachedIntegerVertexAttribTypesValidation;
bool mCachedCanDraw;
}; };
using VertexArrayMap = ResourceMap<VertexArray, VertexArrayID>; using VertexArrayMap = ResourceMap<VertexArray, VertexArrayID>;
......
...@@ -66,6 +66,11 @@ ANGLE_INLINE void MarkShaderStorageBufferUsage(const Context *context) ...@@ -66,6 +66,11 @@ ANGLE_INLINE void MarkShaderStorageBufferUsage(const Context *context)
// have a valid primitive for this mode (0 for points, 0-1 for lines, 0-2 for tris). // have a valid primitive for this mode (0 for points, 0-1 for lines, 0-2 for tris).
ANGLE_INLINE bool Context::noopDraw(PrimitiveMode mode, GLsizei count) ANGLE_INLINE bool Context::noopDraw(PrimitiveMode mode, GLsizei count)
{ {
if (!mStateCache.getCanDraw())
{
return true;
}
return count < kMinimumPrimitiveCounts[mode]; return count < kMinimumPrimitiveCounts[mode];
} }
......
...@@ -5464,6 +5464,7 @@ angle::Result Program::deserialize(const Context *context, ...@@ -5464,6 +5464,7 @@ angle::Result Program::deserialize(const Context *context,
mState.mExecutable.load(&stream); mState.mExecutable.load(&stream);
postResolveLink(context); postResolveLink(context);
mState.mExecutable.updateCanDrawWith();
return angle::Result::Continue; return angle::Result::Continue;
} }
......
...@@ -24,7 +24,8 @@ ProgramExecutable::ProgramExecutable() ...@@ -24,7 +24,8 @@ ProgramExecutable::ProgramExecutable()
mAttributesMask(0), mAttributesMask(0),
mActiveSamplersMask(0), mActiveSamplersMask(0),
mActiveSamplerRefCounts{}, mActiveSamplerRefCounts{},
mActiveImagesMask(0) mActiveImagesMask(0),
mCanDrawWith(false)
{ {
mActiveSamplerTypes.fill(TextureType::InvalidEnum); mActiveSamplerTypes.fill(TextureType::InvalidEnum);
mActiveSamplerFormats.fill(SamplerFormat::InvalidEnum); mActiveSamplerFormats.fill(SamplerFormat::InvalidEnum);
...@@ -458,4 +459,10 @@ bool ProgramExecutable::linkValidateGlobalNames(InfoLog &infoLog) const ...@@ -458,4 +459,10 @@ bool ProgramExecutable::linkValidateGlobalNames(InfoLog &infoLog) const
return true; return true;
} }
void ProgramExecutable::updateCanDrawWith()
{
mCanDrawWith =
(hasLinkedShaderStage(ShaderType::Vertex) && hasLinkedShaderStage(ShaderType::Fragment));
}
} // namespace gl } // namespace gl
...@@ -94,6 +94,8 @@ class ProgramExecutable ...@@ -94,6 +94,8 @@ class ProgramExecutable
{ {
mLinkedGraphicsShaderStages.set(shaderType); mLinkedGraphicsShaderStages.set(shaderType);
} }
updateCanDrawWith();
} }
bool hasLinkedShaderStage(ShaderType shaderType) const bool hasLinkedShaderStage(ShaderType shaderType) const
{ {
...@@ -165,6 +167,9 @@ class ProgramExecutable ...@@ -165,6 +167,9 @@ class ProgramExecutable
void setIsCompute(bool isComputeIn); void setIsCompute(bool isComputeIn);
void updateCanDrawWith();
bool hasVertexAndFragmentShader() const { return mCanDrawWith; }
private: private:
// TODO(timvp): http://anglebug.com/3570: Investigate removing these friend // TODO(timvp): http://anglebug.com/3570: Investigate removing these friend
// class declarations and accessing the necessary members with getters/setters. // class declarations and accessing the necessary members with getters/setters.
...@@ -204,6 +209,8 @@ class ProgramExecutable ...@@ -204,6 +209,8 @@ class ProgramExecutable
// Cached mask of active images. // Cached mask of active images.
ActiveTextureMask mActiveImagesMask; ActiveTextureMask mActiveImagesMask;
ActiveTextureArray<ShaderBitSet> mActiveImageShaderBits; ActiveTextureArray<ShaderBitSet> mActiveImageShaderBits;
bool mCanDrawWith;
}; };
} // namespace gl } // namespace gl
......
...@@ -287,6 +287,8 @@ void ProgramPipeline::updateLinkedShaderStages() ...@@ -287,6 +287,8 @@ void ProgramPipeline::updateLinkedShaderStages()
mState.mExecutable.setLinkedShaderStages(shaderType); mState.mExecutable.setLinkedShaderStages(shaderType);
} }
} }
mState.mExecutable.updateCanDrawWith();
} }
void ProgramPipeline::updateExecutableAttributes() void ProgramPipeline::updateExecutableAttributes()
......
...@@ -723,7 +723,8 @@ class State : angle::NonCopyable ...@@ -723,7 +723,8 @@ class State : angle::NonCopyable
ANGLE_INLINE bool validateSamplerFormats() const ANGLE_INLINE bool validateSamplerFormats() const
{ {
return (mTexturesIncompatibleWithSamplers & mExecutable->getActiveSamplersMask()).none(); return (!mExecutable ||
(mTexturesIncompatibleWithSamplers & mExecutable->getActiveSamplersMask()).none());
} }
ProvokingVertexConvention getProvokingVertex() const { return mProvokingVertex; } ProvokingVertexConvention getProvokingVertex() const { return mProvokingVertex; }
......
...@@ -459,9 +459,10 @@ bool ValidateVertexShaderAttributeTypeMatch(const Context *context) ...@@ -459,9 +459,10 @@ bool ValidateVertexShaderAttributeTypeMatch(const Context *context)
vaoAttribTypeBits = (vaoAttribEnabledMask & vaoAttribTypeBits); vaoAttribTypeBits = (vaoAttribEnabledMask & vaoAttribTypeBits);
vaoAttribTypeBits |= (~vaoAttribEnabledMask & stateCurrentValuesTypeBits); vaoAttribTypeBits |= (~vaoAttribEnabledMask & stateCurrentValuesTypeBits);
return ValidateComponentTypeMasks( return program &&
program->getExecutable().getAttributesTypeMask().to_ulong(), vaoAttribTypeBits, ValidateComponentTypeMasks(
program->getExecutable().getAttributesMask().to_ulong(), 0xFFFF); program->getExecutable().getAttributesTypeMask().to_ulong(), vaoAttribTypeBits,
program->getExecutable().getAttributesMask().to_ulong(), 0xFFFF);
} }
bool IsCompatibleDrawModeWithGeometryShader(PrimitiveMode drawMode, bool IsCompatibleDrawModeWithGeometryShader(PrimitiveMode drawMode,
...@@ -2916,16 +2917,6 @@ const char *ValidateDrawStates(const Context *context) ...@@ -2916,16 +2917,6 @@ const char *ValidateDrawStates(const Context *context)
if (program) if (program)
{ {
// In OpenGL ES spec for UseProgram at section 7.3, trying to render without
// vertex shader stage or fragment shader stage is a undefined behaviour.
// But ANGLE should clearly generate an INVALID_OPERATION error instead of
// produce undefined result.
if (!program->getExecutable().hasLinkedShaderStage(ShaderType::Vertex) ||
!program->getExecutable().hasLinkedShaderStage(ShaderType::Fragment))
{
return kNoActiveGraphicsShaderStage;
}
if (!program->validateSamplers(nullptr, context->getCaps())) if (!program->validateSamplers(nullptr, context->getCaps()))
{ {
return kTextureTypeConflict; return kTextureTypeConflict;
...@@ -2939,16 +2930,6 @@ const char *ValidateDrawStates(const Context *context) ...@@ -2939,16 +2930,6 @@ const char *ValidateDrawStates(const Context *context)
} }
else if (programPipeline) else if (programPipeline)
{ {
// In OpenGL ES spec for UseProgram at section 7.3, trying to render without
// vertex shader stage or fragment shader stage is a undefined behaviour.
// But ANGLE should clearly generate an INVALID_OPERATION error instead of
// produce undefined result.
if (!programPipeline->getExecutable().hasLinkedShaderStage(ShaderType::Vertex) ||
!programPipeline->getExecutable().hasLinkedShaderStage(ShaderType::Fragment))
{
return kNoActiveGraphicsShaderStage;
}
const char *errorMsg = ValidateProgramPipelineAttachedPrograms(programPipeline); const char *errorMsg = ValidateProgramPipelineAttachedPrograms(programPipeline);
if (errorMsg) if (errorMsg)
{ {
...@@ -2966,10 +2947,6 @@ const char *ValidateDrawStates(const Context *context) ...@@ -2966,10 +2947,6 @@ const char *ValidateDrawStates(const Context *context)
return errorMsg; return errorMsg;
} }
} }
else
{
return kProgramNotBound;
}
// Do some additional WebGL-specific validation // Do some additional WebGL-specific validation
if (extensions.webglCompatibility) if (extensions.webglCompatibility)
......
...@@ -296,7 +296,7 @@ void main() ...@@ -296,7 +296,7 @@ void main()
glUseProgram(program); glUseProgram(program);
glDrawArrays(GL_POINTS, 0, 2); glDrawArrays(GL_POINTS, 0, 2);
EXPECT_GL_ERROR(GL_INVALID_OPERATION); EXPECT_GL_NO_ERROR();
} }
// Attach a vertex and fragment shader and link, but dispatch compute. // Attach a vertex and fragment shader and link, but dispatch compute.
......
...@@ -37,8 +37,7 @@ class LinkAndRelinkTestES31 : public ANGLETest ...@@ -37,8 +37,7 @@ class LinkAndRelinkTestES31 : public ANGLETest
// or vice versa. // or vice versa.
// When program link fails and no valid rendering program is installed in the GL // When program link fails and no valid rendering program is installed in the GL
// state before the link, it should report an error for UseProgram and // state before the link, it should report an error for UseProgram
// DrawArrays/DrawElements.
TEST_P(LinkAndRelinkTest, RenderingProgramFailsWithoutProgramInstalled) TEST_P(LinkAndRelinkTest, RenderingProgramFailsWithoutProgramInstalled)
{ {
glUseProgram(0); glUseProgram(0);
...@@ -53,7 +52,7 @@ TEST_P(LinkAndRelinkTest, RenderingProgramFailsWithoutProgramInstalled) ...@@ -53,7 +52,7 @@ TEST_P(LinkAndRelinkTest, RenderingProgramFailsWithoutProgramInstalled)
EXPECT_GL_ERROR(GL_INVALID_OPERATION); EXPECT_GL_ERROR(GL_INVALID_OPERATION);
glDrawArrays(GL_POINTS, 0, 1); glDrawArrays(GL_POINTS, 0, 1);
EXPECT_GL_ERROR(GL_INVALID_OPERATION); EXPECT_GL_NO_ERROR();
} }
// When program link or relink fails and a valid rendering program is installed // When program link or relink fails and a valid rendering program is installed
...@@ -219,7 +218,6 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithoutProgramInstalled) ...@@ -219,7 +218,6 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithoutProgramInstalled)
// When program link or relink fails and a valid compute program is installed in // When program link or relink fails and a valid compute program is installed in
// the GL state before the link, using the failed program via UseProgram should // the GL state before the link, using the failed program via UseProgram should
// report an error, but dispatching compute should succeed. // report an error, but dispatching compute should succeed.
// However, starting rendering always fails.
TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled) TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled)
{ {
// Install a compute program in the GL state via UseProgram, then dispatch // Install a compute program in the GL state via UseProgram, then dispatch
...@@ -252,7 +250,7 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled) ...@@ -252,7 +250,7 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled)
EXPECT_GL_NO_ERROR(); EXPECT_GL_NO_ERROR();
glDrawArrays(GL_POINTS, 0, 1); glDrawArrays(GL_POINTS, 0, 1);
EXPECT_GL_ERROR(GL_INVALID_OPERATION); EXPECT_GL_NO_ERROR();
// Link failure, and a valid program has been installed in the GL state. // Link failure, and a valid program has been installed in the GL state.
GLuint programNull = glCreateProgram(); GLuint programNull = glCreateProgram();
...@@ -266,7 +264,7 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled) ...@@ -266,7 +264,7 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled)
EXPECT_GL_NO_ERROR(); EXPECT_GL_NO_ERROR();
glDrawArrays(GL_POINTS, 0, 1); glDrawArrays(GL_POINTS, 0, 1);
EXPECT_GL_ERROR(GL_INVALID_OPERATION); EXPECT_GL_NO_ERROR();
// Using the unsuccessfully linked program should report an error. // Using the unsuccessfully linked program should report an error.
glUseProgram(programNull); glUseProgram(programNull);
...@@ -280,7 +278,7 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled) ...@@ -280,7 +278,7 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled)
EXPECT_GL_NO_ERROR(); EXPECT_GL_NO_ERROR();
glDrawArrays(GL_POINTS, 0, 1); glDrawArrays(GL_POINTS, 0, 1);
EXPECT_GL_ERROR(GL_INVALID_OPERATION); EXPECT_GL_NO_ERROR();
// We try to relink the installed program, but make it fail. // We try to relink the installed program, but make it fail.
...@@ -296,7 +294,7 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled) ...@@ -296,7 +294,7 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled)
EXPECT_GL_NO_ERROR(); EXPECT_GL_NO_ERROR();
glDrawArrays(GL_POINTS, 0, 1); glDrawArrays(GL_POINTS, 0, 1);
EXPECT_GL_ERROR(GL_INVALID_OPERATION); EXPECT_GL_NO_ERROR();
// Using the unsuccessfully relinked program should report an error. // Using the unsuccessfully relinked program should report an error.
glUseProgram(program); glUseProgram(program);
...@@ -310,11 +308,11 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled) ...@@ -310,11 +308,11 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled)
EXPECT_GL_NO_ERROR(); EXPECT_GL_NO_ERROR();
glDrawArrays(GL_POINTS, 0, 1); glDrawArrays(GL_POINTS, 0, 1);
EXPECT_GL_ERROR(GL_INVALID_OPERATION); EXPECT_GL_NO_ERROR();
} }
// If you compile and link a compute program successfully and use the program, // If you compile and link a compute program successfully and use the program,
// then dispatching compute can succeed, while starting rendering will fail. // then dispatching compute and rendering can succeed (with undefined behavior).
// If you relink the compute program to a rendering program when it is in use, // If you relink the compute program to a rendering program when it is in use,
// then dispatching compute will fail, but starting rendering can succeed. // then dispatching compute will fail, but starting rendering can succeed.
TEST_P(LinkAndRelinkTestES31, RelinkProgramSucceedsFromComputeToRendering) TEST_P(LinkAndRelinkTestES31, RelinkProgramSucceedsFromComputeToRendering)
...@@ -347,7 +345,7 @@ void main() ...@@ -347,7 +345,7 @@ void main()
EXPECT_GL_NO_ERROR(); EXPECT_GL_NO_ERROR();
glDrawArrays(GL_POINTS, 0, 1); glDrawArrays(GL_POINTS, 0, 1);
EXPECT_GL_ERROR(GL_INVALID_OPERATION); EXPECT_GL_NO_ERROR();
constexpr char kVS[] = "void main() {}"; constexpr char kVS[] = "void main() {}";
constexpr char kFS[] = "void main() {}"; constexpr char kFS[] = "void main() {}";
...@@ -441,7 +439,7 @@ void main() ...@@ -441,7 +439,7 @@ void main()
EXPECT_GL_NO_ERROR(); EXPECT_GL_NO_ERROR();
glDrawArrays(GL_POINTS, 0, 1); glDrawArrays(GL_POINTS, 0, 1);
EXPECT_GL_ERROR(GL_INVALID_OPERATION); EXPECT_GL_NO_ERROR();
} }
ANGLE_INSTANTIATE_TEST_ES2_AND_ES3(LinkAndRelinkTest); ANGLE_INSTANTIATE_TEST_ES2_AND_ES3(LinkAndRelinkTest);
......
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