Commit b42162fb by Jamie Madill Committed by Commit Bot

Optimize ValidateDrawStates.

Apparently returning a small struct was slow enough to make a 2-3% difference in benchmark scores. Very visible on the MSVC sampling profiler. Bug: angleproject:2747 Change-Id: I459a127f3f2a0fc3a08db15c37257a67f63f20ce Reviewed-on: https://chromium-review.googlesource.com/1181465Reviewed-by: 's avatarFrank Henigman <fjhenigman@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org>
parent eef9de68
...@@ -71,6 +71,7 @@ ERRMSG(BlitSameImageDepthOrStencil, ...@@ -71,6 +71,7 @@ ERRMSG(BlitSameImageDepthOrStencil,
"Read and write depth stencil attachments cannot be the same image."); "Read and write depth stencil attachments cannot be the same image.");
ERRMSG(BufferBoundForTransformFeedback, "Buffer is bound for transform feedback."); ERRMSG(BufferBoundForTransformFeedback, "Buffer is bound for transform feedback.");
ERRMSG(BufferNotBound, "A buffer must be bound."); ERRMSG(BufferNotBound, "A buffer must be bound.");
ERRMSG(BufferMapped, "An active buffer is mapped");
ERRMSG(CompressedTextureDimensionsMustMatchData, ERRMSG(CompressedTextureDimensionsMustMatchData,
"Compressed texture dimensions must exactly match the dimensions of the data passed in."); "Compressed texture dimensions must exactly match the dimensions of the data passed in.");
ERRMSG(CompressedTexturesNotAttachable, "Compressed textures cannot be attached to a framebuffer."); ERRMSG(CompressedTexturesNotAttachable, "Compressed textures cannot be attached to a framebuffer.");
...@@ -83,6 +84,7 @@ ERRMSG(DefaultFramebufferTarget, "It is invalid to change default FBO's attachme ...@@ -83,6 +84,7 @@ ERRMSG(DefaultFramebufferTarget, "It is invalid to change default FBO's attachme
ERRMSG(DispatchIndirectBufferNotBound, "Dispatch indirect buffer must be bound."); ERRMSG(DispatchIndirectBufferNotBound, "Dispatch indirect buffer must be bound.");
ERRMSG(DrawBufferTypeMismatch, ERRMSG(DrawBufferTypeMismatch,
"Fragment shader output type does not match the bound framebuffer attachment type."); "Fragment shader output type does not match the bound framebuffer attachment type.");
ERRMSG(DrawFramebufferIncomplete, "Draw framebuffer is incomplete");
ERRMSG(ElementArrayBufferBoundForTransformFeedback, ERRMSG(ElementArrayBufferBoundForTransformFeedback,
"It is undefined behavior to use an element array buffer that is bound for transform " "It is undefined behavior to use an element array buffer that is bound for transform "
"feedback."); "feedback.");
...@@ -293,6 +295,7 @@ ERRMSG(StencilReferenceMaskOrMismatch, ...@@ -293,6 +295,7 @@ ERRMSG(StencilReferenceMaskOrMismatch,
ERRMSG(StrideMustBeMultipleOfType, "Stride must be a multiple of the passed in datatype."); ERRMSG(StrideMustBeMultipleOfType, "Stride must be a multiple of the passed in datatype.");
ERRMSG(TextureNotBound, "A texture must be bound."); ERRMSG(TextureNotBound, "A texture must be bound.");
ERRMSG(TextureNotPow2, "The texture is a non-power-of-two texture."); ERRMSG(TextureNotPow2, "The texture is a non-power-of-two texture.");
ERRMSG(TextureTypeConflict, "Two textures of different types use the same sampler location.");
ERRMSG(TransformFeedbackBufferDoubleBound, ERRMSG(TransformFeedbackBufferDoubleBound,
"A transform feedback buffer that would be written to is also bound to a " "A transform feedback buffer that would be written to is also bound to a "
"non-transform-feedback target, which would cause undefined behavior."); "non-transform-feedback target, which would cause undefined behavior.");
......
...@@ -2542,7 +2542,9 @@ bool ValidateCopyTexImageParametersBase(Context *context, ...@@ -2542,7 +2542,9 @@ bool ValidateCopyTexImageParametersBase(Context *context,
return true; return true;
} }
ErrorAndMessage ValidateDrawStates(Context *context) // Note all errors returned from this function are INVALID_OPERATION except for the draw framebuffer
// completeness check.
const char *ValidateDrawStates(Context *context)
{ {
const Extensions &extensions = context->getExtensions(); const Extensions &extensions = context->getExtensions();
const State &state = context->getGLState(); const State &state = context->getGLState();
...@@ -2552,7 +2554,7 @@ ErrorAndMessage ValidateDrawStates(Context *context) ...@@ -2552,7 +2554,7 @@ ErrorAndMessage ValidateDrawStates(Context *context)
// https://www.khronos.org/registry/webgl/specs/latest/2.0/#5.14 // https://www.khronos.org/registry/webgl/specs/latest/2.0/#5.14
if (!extensions.webglCompatibility && state.getVertexArray()->hasMappedEnabledArrayBuffer()) if (!extensions.webglCompatibility && state.getVertexArray()->hasMappedEnabledArrayBuffer())
{ {
return {GL_INVALID_OPERATION, nullptr}; return kErrorBufferMapped;
} }
// Note: these separate values are not supported in WebGL, due to D3D's limitations. See // Note: these separate values are not supported in WebGL, due to D3D's limitations. See
...@@ -2586,14 +2588,15 @@ ErrorAndMessage ValidateDrawStates(Context *context) ...@@ -2586,14 +2588,15 @@ ErrorAndMessage ValidateDrawStates(Context *context)
WARN() << "This ANGLE implementation does not support separate front/back " WARN() << "This ANGLE implementation does not support separate front/back "
"stencil writemasks, reference values, or stencil mask values."; "stencil writemasks, reference values, or stencil mask values.";
} }
return {GL_INVALID_OPERATION, kErrorStencilReferenceMaskOrMismatch}; return kErrorStencilReferenceMaskOrMismatch;
} }
} }
} }
if (!framebuffer->isComplete(context)) if (!framebuffer->isComplete(context))
{ {
return {GL_INVALID_FRAMEBUFFER_OPERATION, nullptr}; // Note: this error should be generated as INVALID_FRAMEBUFFER_OPERATION.
return kErrorDrawFramebufferIncomplete;
} }
if (context->getStateCache().hasAnyEnabledClientAttrib()) if (context->getStateCache().hasAnyEnabledClientAttrib())
...@@ -2604,14 +2607,14 @@ ErrorAndMessage ValidateDrawStates(Context *context) ...@@ -2604,14 +2607,14 @@ ErrorAndMessage ValidateDrawStates(Context *context)
// If a vertex attribute is enabled as an array via enableVertexAttribArray but no // If a vertex attribute is enabled as an array via enableVertexAttribArray but no
// buffer is bound to that attribute via bindBuffer and vertexAttribPointer, then calls // buffer is bound to that attribute via bindBuffer and vertexAttribPointer, then calls
// to drawArrays or drawElements will generate an INVALID_OPERATION error. // to drawArrays or drawElements will generate an INVALID_OPERATION error.
return {GL_INVALID_OPERATION, kErrorVertexArrayNoBuffer}; return kErrorVertexArrayNoBuffer;
} }
if (state.getVertexArray()->hasEnabledNullPointerClientArray()) if (state.getVertexArray()->hasEnabledNullPointerClientArray())
{ {
// This is an application error that would normally result in a crash, but we catch it // This is an application error that would normally result in a crash, but we catch it
// and return an error // and return an error
return {GL_INVALID_OPERATION, kErrorVertexArrayNoBufferPointer}; return kErrorVertexArrayNoBufferPointer;
} }
} }
...@@ -2621,7 +2624,7 @@ ErrorAndMessage ValidateDrawStates(Context *context) ...@@ -2621,7 +2624,7 @@ ErrorAndMessage ValidateDrawStates(Context *context)
Program *program = state.getProgram(); Program *program = state.getProgram();
if (!program) if (!program)
{ {
return {GL_INVALID_OPERATION, kErrorProgramNotBound}; return kErrorProgramNotBound;
} }
// In OpenGL ES spec for UseProgram at section 7.3, trying to render without // In OpenGL ES spec for UseProgram at section 7.3, trying to render without
...@@ -2631,12 +2634,12 @@ ErrorAndMessage ValidateDrawStates(Context *context) ...@@ -2631,12 +2634,12 @@ ErrorAndMessage ValidateDrawStates(Context *context)
if (!program->hasLinkedShaderStage(ShaderType::Vertex) || if (!program->hasLinkedShaderStage(ShaderType::Vertex) ||
!program->hasLinkedShaderStage(ShaderType::Fragment)) !program->hasLinkedShaderStage(ShaderType::Fragment))
{ {
return {GL_INVALID_OPERATION, kErrorNoActiveGraphicsShaderStage}; return kErrorNoActiveGraphicsShaderStage;
} }
if (!program->validateSamplers(nullptr, context->getCaps())) if (!program->validateSamplers(nullptr, context->getCaps()))
{ {
return {GL_INVALID_OPERATION, nullptr}; return kErrorTextureTypeConflict;
} }
if (extensions.multiview) if (extensions.multiview)
...@@ -2645,20 +2648,20 @@ ErrorAndMessage ValidateDrawStates(Context *context) ...@@ -2645,20 +2648,20 @@ ErrorAndMessage ValidateDrawStates(Context *context)
const int framebufferNumViews = framebuffer->getNumViews(); const int framebufferNumViews = framebuffer->getNumViews();
if (framebufferNumViews != programNumViews) if (framebufferNumViews != programNumViews)
{ {
return {GL_INVALID_OPERATION, kErrorMultiviewMismatch}; return kErrorMultiviewMismatch;
} }
const TransformFeedback *transformFeedbackObject = state.getCurrentTransformFeedback(); const TransformFeedback *transformFeedbackObject = state.getCurrentTransformFeedback();
if (transformFeedbackObject != nullptr && transformFeedbackObject->isActive() && if (transformFeedbackObject != nullptr && transformFeedbackObject->isActive() &&
framebufferNumViews > 1) framebufferNumViews > 1)
{ {
return {GL_INVALID_OPERATION, kErrorMultiviewTransformFeedback}; return kErrorMultiviewTransformFeedback;
} }
if (extensions.disjointTimerQuery && framebufferNumViews > 1 && if (extensions.disjointTimerQuery && framebufferNumViews > 1 &&
state.isQueryActive(QueryType::TimeElapsed)) state.isQueryActive(QueryType::TimeElapsed))
{ {
return {GL_INVALID_OPERATION, kErrorMultiviewTimerQuery}; return kErrorMultiviewTimerQuery;
} }
} }
...@@ -2674,20 +2677,20 @@ ErrorAndMessage ValidateDrawStates(Context *context) ...@@ -2674,20 +2677,20 @@ ErrorAndMessage ValidateDrawStates(Context *context)
if (uniformBuffer.get() == nullptr) if (uniformBuffer.get() == nullptr)
{ {
// undefined behaviour // undefined behaviour
return {GL_INVALID_OPERATION, kErrorUniformBufferUnbound}; return kErrorUniformBufferUnbound;
} }
size_t uniformBufferSize = GetBoundBufferAvailableSize(uniformBuffer); size_t uniformBufferSize = GetBoundBufferAvailableSize(uniformBuffer);
if (uniformBufferSize < uniformBlock.dataSize) if (uniformBufferSize < uniformBlock.dataSize)
{ {
// undefined behaviour // undefined behaviour
return {GL_INVALID_OPERATION, kErrorUniformBufferTooSmall}; return kErrorUniformBufferTooSmall;
} }
if (extensions.webglCompatibility && if (extensions.webglCompatibility &&
uniformBuffer->isBoundForTransformFeedbackAndOtherUse()) uniformBuffer->isBoundForTransformFeedbackAndOtherUse())
{ {
return {GL_INVALID_OPERATION, kErrorUniformBufferBoundForTransformFeedback}; return kErrorUniformBufferBoundForTransformFeedback;
} }
} }
...@@ -2698,36 +2701,36 @@ ErrorAndMessage ValidateDrawStates(Context *context) ...@@ -2698,36 +2701,36 @@ ErrorAndMessage ValidateDrawStates(Context *context)
if (transformFeedbackObject != nullptr && transformFeedbackObject->isActive() && if (transformFeedbackObject != nullptr && transformFeedbackObject->isActive() &&
transformFeedbackObject->buffersBoundForOtherUse()) transformFeedbackObject->buffersBoundForOtherUse())
{ {
return {GL_INVALID_OPERATION, kErrorTransformFeedbackBufferDoubleBound}; return kErrorTransformFeedbackBufferDoubleBound;
} }
// Detect rendering feedback loops for WebGL. // Detect rendering feedback loops for WebGL.
if (framebuffer->formsRenderingFeedbackLoopWith(state)) if (framebuffer->formsRenderingFeedbackLoopWith(state))
{ {
return {GL_INVALID_OPERATION, kErrorFeedbackLoop}; return kErrorFeedbackLoop;
} }
// Detect that the vertex shader input types match the attribute types // Detect that the vertex shader input types match the attribute types
if (!ValidateVertexShaderAttributeTypeMatch(context)) if (!ValidateVertexShaderAttributeTypeMatch(context))
{ {
return {GL_INVALID_OPERATION, kErrorVertexShaderTypeMismatch}; return kErrorVertexShaderTypeMismatch;
} }
// Detect that the color buffer types match the fragment shader output types // Detect that the color buffer types match the fragment shader output types
if (!ValidateFragmentShaderColorBufferTypeMatch(context)) if (!ValidateFragmentShaderColorBufferTypeMatch(context))
{ {
return {GL_INVALID_OPERATION, kErrorDrawBufferTypeMismatch}; return kErrorDrawBufferTypeMismatch;
} }
const VertexArray *vao = context->getGLState().getVertexArray(); const VertexArray *vao = context->getGLState().getVertexArray();
if (vao->hasTransformFeedbackBindingConflict(context)) if (vao->hasTransformFeedbackBindingConflict(context))
{ {
return {GL_INVALID_OPERATION, kErrorVertexBufferBoundForTransformFeedback}; return kErrorVertexBufferBoundForTransformFeedback;
} }
} }
} }
return {GL_NO_ERROR, nullptr}; return nullptr;
} }
bool ValidateDrawBase(Context *context, PrimitiveMode mode, GLsizei count) bool ValidateDrawBase(Context *context, PrimitiveMode mode, GLsizei count)
...@@ -2768,17 +2771,15 @@ bool ValidateDrawBase(Context *context, PrimitiveMode mode, GLsizei count) ...@@ -2768,17 +2771,15 @@ bool ValidateDrawBase(Context *context, PrimitiveMode mode, GLsizei count)
const State &state = context->getGLState(); const State &state = context->getGLState();
const ErrorAndMessage &errorAndMessage = ValidateDrawStates(context); const char *errorMessage = ValidateDrawStates(context);
if (errorAndMessage.errorType != GL_NO_ERROR) if (errorMessage)
{ {
if (errorAndMessage.message) // All errors from ValidateDrawStates should return INVALID_OPERATION except Framebuffer
{ // Incomplete.
context->handleError(Error(errorAndMessage.errorType, errorAndMessage.message)); GLenum errorCode =
} (errorMessage == kErrorDrawFramebufferIncomplete ? GL_INVALID_FRAMEBUFFER_OPERATION
else : GL_INVALID_OPERATION);
{ context->handleError(Error(errorCode, errorMessage));
context->handleError(Error(errorAndMessage.errorType));
}
return false; return false;
} }
......
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