Commit 1ab55d96 by Courtney Goeltzenleuchter Committed by Commit Bot

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

This reverts commit a4b506f7. Reason for revert: WebGL crash https://bugs.chromium.org/p/angleproject/issues/detail?id=4616 Original change's description: > No-Op draws when no active VS and/or FS is present > > 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: If19e9fbb1bc09fa0d490832079bb9f514eab6035 > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2136386 > Reviewed-by: Jamie Madill <jmadill@chromium.org> > Commit-Queue: Tim Van Patten <timvp@google.com> TBR=timvp@google.com,jmadill@chromium.org,cclao@google.com Change-Id: Ia24c4156ff7779b69c1f3f705f1a91cbb1c9684c No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: angleproject:3570 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2184849Reviewed-by: 's avatarCourtney Goeltzenleuchter <courtneygo@google.com> Commit-Queue: Courtney Goeltzenleuchter <courtneygo@google.com>
parent b759a623
...@@ -1124,7 +1124,6 @@ void Context::bindProgramPipeline(ProgramPipelineID pipelineHandle) ...@@ -1124,7 +1124,6 @@ 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)
...@@ -8493,8 +8492,7 @@ StateCache::StateCache() ...@@ -8493,8 +8492,7 @@ 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;
...@@ -8515,7 +8513,6 @@ void StateCache::initialize(Context *context) ...@@ -8515,7 +8513,6 @@ 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)
...@@ -8627,7 +8624,6 @@ void StateCache::onProgramExecutableChange(Context *context) ...@@ -8627,7 +8624,6 @@ 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)
...@@ -8889,11 +8885,4 @@ void StateCache::updateActiveShaderStorageBufferIndices(Context *context) ...@@ -8889,11 +8885,4 @@ 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,10 +257,6 @@ class StateCache final : angle::NonCopyable ...@@ -257,10 +257,6 @@ 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);
...@@ -294,7 +290,6 @@ class StateCache final : angle::NonCopyable ...@@ -294,7 +290,6 @@ 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);
...@@ -329,8 +324,6 @@ class StateCache final : angle::NonCopyable ...@@ -329,8 +324,6 @@ 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,11 +66,6 @@ ANGLE_INLINE void MarkShaderStorageBufferUsage(const Context *context) ...@@ -66,11 +66,6 @@ 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,7 +5464,6 @@ angle::Result Program::deserialize(const Context *context, ...@@ -5464,7 +5464,6 @@ 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,8 +24,7 @@ ProgramExecutable::ProgramExecutable() ...@@ -24,8 +24,7 @@ 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);
...@@ -459,10 +458,4 @@ bool ProgramExecutable::linkValidateGlobalNames(InfoLog &infoLog) const ...@@ -459,10 +458,4 @@ 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,8 +94,6 @@ class ProgramExecutable ...@@ -94,8 +94,6 @@ class ProgramExecutable
{ {
mLinkedGraphicsShaderStages.set(shaderType); mLinkedGraphicsShaderStages.set(shaderType);
} }
updateCanDrawWith();
} }
bool hasLinkedShaderStage(ShaderType shaderType) const bool hasLinkedShaderStage(ShaderType shaderType) const
{ {
...@@ -167,9 +165,6 @@ class ProgramExecutable ...@@ -167,9 +165,6 @@ 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.
...@@ -209,8 +204,6 @@ class ProgramExecutable ...@@ -209,8 +204,6 @@ 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,8 +287,6 @@ void ProgramPipeline::updateLinkedShaderStages() ...@@ -287,8 +287,6 @@ void ProgramPipeline::updateLinkedShaderStages()
mState.mExecutable.setLinkedShaderStages(shaderType); mState.mExecutable.setLinkedShaderStages(shaderType);
} }
} }
mState.mExecutable.updateCanDrawWith();
} }
void ProgramPipeline::updateExecutableAttributes() void ProgramPipeline::updateExecutableAttributes()
......
...@@ -2916,6 +2916,16 @@ const char *ValidateDrawStates(const Context *context) ...@@ -2916,6 +2916,16 @@ 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;
...@@ -2929,6 +2939,16 @@ const char *ValidateDrawStates(const Context *context) ...@@ -2929,6 +2939,16 @@ 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)
{ {
...@@ -2946,6 +2966,10 @@ const char *ValidateDrawStates(const Context *context) ...@@ -2946,6 +2966,10 @@ 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_NO_ERROR(); EXPECT_GL_ERROR(GL_INVALID_OPERATION);
} }
// Attach a vertex and fragment shader and link, but dispatch compute. // Attach a vertex and fragment shader and link, but dispatch compute.
......
...@@ -37,7 +37,8 @@ class LinkAndRelinkTestES31 : public ANGLETest ...@@ -37,7 +37,8 @@ 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 // state before the link, it should report an error for UseProgram and
// DrawArrays/DrawElements.
TEST_P(LinkAndRelinkTest, RenderingProgramFailsWithoutProgramInstalled) TEST_P(LinkAndRelinkTest, RenderingProgramFailsWithoutProgramInstalled)
{ {
glUseProgram(0); glUseProgram(0);
...@@ -52,7 +53,7 @@ TEST_P(LinkAndRelinkTest, RenderingProgramFailsWithoutProgramInstalled) ...@@ -52,7 +53,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_NO_ERROR(); EXPECT_GL_ERROR(GL_INVALID_OPERATION);
} }
// 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
...@@ -218,6 +219,7 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithoutProgramInstalled) ...@@ -218,6 +219,7 @@ 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
...@@ -250,7 +252,7 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled) ...@@ -250,7 +252,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_NO_ERROR(); EXPECT_GL_ERROR(GL_INVALID_OPERATION);
// 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();
...@@ -264,7 +266,7 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled) ...@@ -264,7 +266,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_NO_ERROR(); EXPECT_GL_ERROR(GL_INVALID_OPERATION);
// Using the unsuccessfully linked program should report an error. // Using the unsuccessfully linked program should report an error.
glUseProgram(programNull); glUseProgram(programNull);
...@@ -278,7 +280,7 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled) ...@@ -278,7 +280,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_NO_ERROR(); EXPECT_GL_ERROR(GL_INVALID_OPERATION);
// We try to relink the installed program, but make it fail. // We try to relink the installed program, but make it fail.
...@@ -294,7 +296,7 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled) ...@@ -294,7 +296,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_NO_ERROR(); EXPECT_GL_ERROR(GL_INVALID_OPERATION);
// Using the unsuccessfully relinked program should report an error. // Using the unsuccessfully relinked program should report an error.
glUseProgram(program); glUseProgram(program);
...@@ -308,11 +310,11 @@ TEST_P(LinkAndRelinkTestES31, ComputeProgramFailsWithProgramInstalled) ...@@ -308,11 +310,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_NO_ERROR(); EXPECT_GL_ERROR(GL_INVALID_OPERATION);
} }
// 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 and rendering can succeed (with undefined behavior). // then dispatching compute can succeed, while starting rendering will fail.
// 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)
...@@ -345,7 +347,7 @@ void main() ...@@ -345,7 +347,7 @@ void main()
EXPECT_GL_NO_ERROR(); EXPECT_GL_NO_ERROR();
glDrawArrays(GL_POINTS, 0, 1); glDrawArrays(GL_POINTS, 0, 1);
EXPECT_GL_NO_ERROR(); EXPECT_GL_ERROR(GL_INVALID_OPERATION);
constexpr char kVS[] = "void main() {}"; constexpr char kVS[] = "void main() {}";
constexpr char kFS[] = "void main() {}"; constexpr char kFS[] = "void main() {}";
...@@ -439,7 +441,7 @@ void main() ...@@ -439,7 +441,7 @@ void main()
EXPECT_GL_NO_ERROR(); EXPECT_GL_NO_ERROR();
glDrawArrays(GL_POINTS, 0, 1); glDrawArrays(GL_POINTS, 0, 1);
EXPECT_GL_NO_ERROR(); EXPECT_GL_ERROR(GL_INVALID_OPERATION);
} }
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