Commit 427064d2 by Jamie Madill Committed by Commit Bot

Fix perf regression with checkStatus.

Returning an Error was costing us performance in extra error checks. This intead uses the Context to process the Error immediately, and returns a value from isComplete and checkStatus. Improves performance in draw call validation. Bug: chromium:822235 Change-Id: I0793fc690e86137425fed593d45083e40aee8db9 Reviewed-on: https://chromium-review.googlesource.com/1011370Reviewed-by: 's avatarLuc Ferron <lucferron@chromium.org> Reviewed-by: 's avatarYuly Novikov <ynovikov@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org>
parent ec10096d
...@@ -2420,7 +2420,7 @@ void Context::getProgramInterfaceivRobust(GLuint program, ...@@ -2420,7 +2420,7 @@ void Context::getProgramInterfaceivRobust(GLuint program,
UNIMPLEMENTED(); UNIMPLEMENTED();
} }
void Context::handleError(const Error &error) void Context::handleError(const Error &error) const
{ {
if (error.isError()) if (error.isError())
{ {
...@@ -2454,7 +2454,7 @@ GLenum Context::getError() ...@@ -2454,7 +2454,7 @@ GLenum Context::getError()
} }
// NOTE: this function should not assume that this context is current! // NOTE: this function should not assume that this context is current!
void Context::markContextLost() void Context::markContextLost() const
{ {
if (mResetStrategy == GL_LOSE_CONTEXT_ON_RESET_EXT) if (mResetStrategy == GL_LOSE_CONTEXT_ON_RESET_EXT)
{ {
...@@ -2464,7 +2464,7 @@ void Context::markContextLost() ...@@ -2464,7 +2464,7 @@ void Context::markContextLost()
mContextLost = true; mContextLost = true;
} }
bool Context::isContextLost() bool Context::isContextLost() const
{ {
return mContextLost; return mContextLost;
} }
...@@ -3643,9 +3643,7 @@ void Context::invalidateFramebuffer(GLenum target, ...@@ -3643,9 +3643,7 @@ void Context::invalidateFramebuffer(GLenum target,
Framebuffer *framebuffer = mGLState.getTargetFramebuffer(target); Framebuffer *framebuffer = mGLState.getTargetFramebuffer(target);
ASSERT(framebuffer); ASSERT(framebuffer);
bool complete = false; if (!framebuffer->isComplete(this))
ANGLE_CONTEXT_TRY(framebuffer->isComplete(this, &complete));
if (!complete)
{ {
return; return;
} }
...@@ -3667,9 +3665,7 @@ void Context::invalidateSubFramebuffer(GLenum target, ...@@ -3667,9 +3665,7 @@ void Context::invalidateSubFramebuffer(GLenum target,
Framebuffer *framebuffer = mGLState.getTargetFramebuffer(target); Framebuffer *framebuffer = mGLState.getTargetFramebuffer(target);
ASSERT(framebuffer); ASSERT(framebuffer);
bool complete = false; if (!framebuffer->isComplete(this))
ANGLE_CONTEXT_TRY(framebuffer->isComplete(this, &complete));
if (!complete)
{ {
return; return;
} }
...@@ -4918,15 +4914,7 @@ GLenum Context::checkFramebufferStatus(GLenum target) ...@@ -4918,15 +4914,7 @@ GLenum Context::checkFramebufferStatus(GLenum target)
{ {
Framebuffer *framebuffer = mGLState.getTargetFramebuffer(target); Framebuffer *framebuffer = mGLState.getTargetFramebuffer(target);
ASSERT(framebuffer); ASSERT(framebuffer);
return framebuffer->checkStatus(this);
GLenum status = GL_NONE;
Error err = framebuffer->checkStatus(this, &status);
if (err.isError())
{
handleError(err);
return 0;
}
return status;
} }
void Context::compileShader(GLuint shader) void Context::compileShader(GLuint shader)
......
...@@ -1374,11 +1374,11 @@ class Context final : angle::NonCopyable ...@@ -1374,11 +1374,11 @@ class Context final : angle::NonCopyable
void memoryBarrierByRegion(GLbitfield barriers); void memoryBarrierByRegion(GLbitfield barriers);
// Consumes the error. // Consumes the error.
void handleError(const Error &error); void handleError(const Error &error) const;
GLenum getError(); GLenum getError();
void markContextLost(); void markContextLost() const;
bool isContextLost(); bool isContextLost() const;
GLenum getGraphicsResetStatus(); GLenum getGraphicsResetStatus();
bool isResetNotificationEnabled(); bool isResetNotificationEnabled();
...@@ -1546,13 +1546,13 @@ class Context final : angle::NonCopyable ...@@ -1546,13 +1546,13 @@ class Context final : angle::NonCopyable
// Recorded errors // Recorded errors
typedef std::set<GLenum> ErrorSet; typedef std::set<GLenum> ErrorSet;
ErrorSet mErrors; mutable ErrorSet mErrors;
// Current/lost context flags // Current/lost context flags
bool mHasBeenCurrent; bool mHasBeenCurrent;
bool mContextLost; mutable bool mContextLost;
GLenum mResetStatus; mutable GLenum mResetStatus;
bool mContextLostForced; mutable bool mContextLostForced;
GLenum mResetStrategy; GLenum mResetStrategy;
bool mRobustAccess; bool mRobustAccess;
egl::Surface *mCurrentSurface; egl::Surface *mCurrentSurface;
......
...@@ -97,7 +97,7 @@ void Debug::insertMessage(GLenum source, ...@@ -97,7 +97,7 @@ void Debug::insertMessage(GLenum source,
GLenum type, GLenum type,
GLuint id, GLuint id,
GLenum severity, GLenum severity,
const std::string &message) const std::string &message) const
{ {
std::string messageCopy(message); std::string messageCopy(message);
insertMessage(source, type, id, severity, std::move(messageCopy)); insertMessage(source, type, id, severity, std::move(messageCopy));
...@@ -107,7 +107,7 @@ void Debug::insertMessage(GLenum source, ...@@ -107,7 +107,7 @@ void Debug::insertMessage(GLenum source,
GLenum type, GLenum type,
GLuint id, GLuint id,
GLenum severity, GLenum severity,
std::string &&message) std::string &&message) const
{ {
if (!isMessageEnabled(source, type, id, severity)) if (!isMessageEnabled(source, type, id, severity))
{ {
......
...@@ -49,12 +49,12 @@ class Debug : angle::NonCopyable ...@@ -49,12 +49,12 @@ class Debug : angle::NonCopyable
GLenum type, GLenum type,
GLuint id, GLuint id,
GLenum severity, GLenum severity,
const std::string &message); const std::string &message) const;
void insertMessage(GLenum source, void insertMessage(GLenum source,
GLenum type, GLenum type,
GLuint id, GLuint id,
GLenum severity, GLenum severity,
std::string &&message); std::string &&message) const;
void setMessageControl(GLenum source, void setMessageControl(GLenum source,
GLenum type, GLenum type,
...@@ -119,7 +119,7 @@ class Debug : angle::NonCopyable ...@@ -119,7 +119,7 @@ class Debug : angle::NonCopyable
bool mOutputEnabled; bool mOutputEnabled;
GLDEBUGPROCKHR mCallbackFunction; GLDEBUGPROCKHR mCallbackFunction;
const void *mCallbackUserParam; const void *mCallbackUserParam;
std::deque<Message> mMessages; mutable std::deque<Message> mMessages;
GLuint mMaxLoggedMessages; GLuint mMaxLoggedMessages;
bool mOutputSynchronous; bool mOutputSynchronous;
std::vector<Group> mGroups; std::vector<Group> mGroups;
......
...@@ -981,7 +981,7 @@ void Framebuffer::invalidateCompletenessCache() ...@@ -981,7 +981,7 @@ void Framebuffer::invalidateCompletenessCache()
} }
} }
Error Framebuffer::checkStatus(const Context *context, GLenum *statusOut) GLenum Framebuffer::checkStatus(const Context *context)
{ {
// The default framebuffer is always complete except when it is surfaceless in which // The default framebuffer is always complete except when it is surfaceless in which
// case it is always unsupported. We return early because the default framebuffer may // case it is always unsupported. We return early because the default framebuffer may
...@@ -991,8 +991,7 @@ Error Framebuffer::checkStatus(const Context *context, GLenum *statusOut) ...@@ -991,8 +991,7 @@ Error Framebuffer::checkStatus(const Context *context, GLenum *statusOut)
ASSERT(mCachedStatus.valid()); ASSERT(mCachedStatus.valid());
ASSERT(mCachedStatus.value() == GL_FRAMEBUFFER_COMPLETE || ASSERT(mCachedStatus.value() == GL_FRAMEBUFFER_COMPLETE ||
mCachedStatus.value() == GL_FRAMEBUFFER_UNDEFINED_OES); mCachedStatus.value() == GL_FRAMEBUFFER_UNDEFINED_OES);
*statusOut = mCachedStatus.value(); return mCachedStatus.value();
return NoError();
} }
if (hasAnyDirtyBit() || !mCachedStatus.valid()) if (hasAnyDirtyBit() || !mCachedStatus.valid())
...@@ -1001,7 +1000,12 @@ Error Framebuffer::checkStatus(const Context *context, GLenum *statusOut) ...@@ -1001,7 +1000,12 @@ Error Framebuffer::checkStatus(const Context *context, GLenum *statusOut)
if (mCachedStatus.value() == GL_FRAMEBUFFER_COMPLETE) if (mCachedStatus.value() == GL_FRAMEBUFFER_COMPLETE)
{ {
ANGLE_TRY(syncState(context)); Error err = syncState(context);
if (err.isError())
{
context->handleError(err);
return GetDefaultReturnValue<EntryPoint::CheckFramebufferStatus, GLenum>();
}
if (!mImpl->checkStatus(context)) if (!mImpl->checkStatus(context))
{ {
mCachedStatus = GL_FRAMEBUFFER_UNSUPPORTED; mCachedStatus = GL_FRAMEBUFFER_UNSUPPORTED;
...@@ -1009,8 +1013,7 @@ Error Framebuffer::checkStatus(const Context *context, GLenum *statusOut) ...@@ -1009,8 +1013,7 @@ Error Framebuffer::checkStatus(const Context *context, GLenum *statusOut)
} }
} }
*statusOut = mCachedStatus.value(); return mCachedStatus.value();
return NoError();
} }
GLenum Framebuffer::checkStatusWithGLFrontEnd(const Context *context) GLenum Framebuffer::checkStatusWithGLFrontEnd(const Context *context)
...@@ -1433,12 +1436,9 @@ Error Framebuffer::blit(const Context *context, ...@@ -1433,12 +1436,9 @@ Error Framebuffer::blit(const Context *context,
return mImpl->blit(context, sourceArea, destArea, blitMask, filter); return mImpl->blit(context, sourceArea, destArea, blitMask, filter);
} }
Error Framebuffer::getSamples(const Context *context, int *samplesOut) int Framebuffer::getSamples(const Context *context)
{ {
bool completeness = false; return (isComplete(context) ? getCachedSamples(context) : 0);
ANGLE_TRY(isComplete(context, &completeness));
*samplesOut = completeness ? getCachedSamples(context) : 0;
return NoError();
} }
int Framebuffer::getCachedSamples(const Context *context) int Framebuffer::getCachedSamples(const Context *context)
...@@ -1792,12 +1792,9 @@ FramebufferAttachment *Framebuffer::getAttachmentFromSubjectIndex(angle::Subject ...@@ -1792,12 +1792,9 @@ FramebufferAttachment *Framebuffer::getAttachmentFromSubjectIndex(angle::Subject
} }
} }
Error Framebuffer::isComplete(const Context *context, bool *completeOut) bool Framebuffer::isComplete(const Context *context)
{ {
GLenum status = GL_NONE; return (checkStatus(context) == GL_FRAMEBUFFER_COMPLETE);
ANGLE_TRY(checkStatus(context, &status));
*completeOut = (status == GL_FRAMEBUFFER_COMPLETE);
return NoError();
} }
bool Framebuffer::formsRenderingFeedbackLoopWith(const State &state) const bool Framebuffer::formsRenderingFeedbackLoopWith(const State &state) const
......
...@@ -214,7 +214,7 @@ class Framebuffer final : public angle::ObserverInterface, public LabeledObject ...@@ -214,7 +214,7 @@ class Framebuffer final : public angle::ObserverInterface, public LabeledObject
bool usingExtendedDrawBuffers() const; bool usingExtendedDrawBuffers() const;
// This method calls checkStatus. // This method calls checkStatus.
Error getSamples(const Context *context, int *samplesOut); int getSamples(const Context *context);
Error getSamplePosition(size_t index, GLfloat *xy) const; Error getSamplePosition(size_t index, GLfloat *xy) const;
...@@ -229,13 +229,13 @@ class Framebuffer final : public angle::ObserverInterface, public LabeledObject ...@@ -229,13 +229,13 @@ class Framebuffer final : public angle::ObserverInterface, public LabeledObject
void invalidateCompletenessCache(); void invalidateCompletenessCache();
Error checkStatus(const Context *context, GLenum *statusOut); GLenum checkStatus(const Context *context);
// For when we don't want to check completeness in getSamples(). // For when we don't want to check completeness in getSamples().
int getCachedSamples(const Context *context); int getCachedSamples(const Context *context);
// Helper for checkStatus == GL_FRAMEBUFFER_COMPLETE. // Helper for checkStatus == GL_FRAMEBUFFER_COMPLETE.
Error isComplete(const Context *context, bool *completeOut); bool isComplete(const Context *context);
bool hasValidDepthStencil() const; bool hasValidDepthStencil() const;
......
...@@ -2148,12 +2148,9 @@ Error State::getIntegerv(const Context *context, GLenum pname, GLint *params) ...@@ -2148,12 +2148,9 @@ Error State::getIntegerv(const Context *context, GLenum pname, GLint *params)
case GL_SAMPLES: case GL_SAMPLES:
{ {
Framebuffer *framebuffer = mDrawFramebuffer; Framebuffer *framebuffer = mDrawFramebuffer;
bool complete = false; if (framebuffer->isComplete(context))
ANGLE_TRY(framebuffer->isComplete(context, &complete));
if (complete)
{ {
GLint samples = 0; GLint samples = framebuffer->getSamples(context);
ANGLE_TRY(framebuffer->getSamples(context, &samples));
switch (pname) switch (pname)
{ {
case GL_SAMPLE_BUFFERS: case GL_SAMPLE_BUFFERS:
......
...@@ -1277,12 +1277,12 @@ bool ValidateBlitFramebufferParameters(Context *context, ...@@ -1277,12 +1277,12 @@ bool ValidateBlitFramebufferParameters(Context *context,
return false; return false;
} }
if (!ValidateFramebufferComplete(context, readFramebuffer, true)) if (!ValidateFramebufferComplete(context, readFramebuffer))
{ {
return false; return false;
} }
if (!ValidateFramebufferComplete(context, drawFramebuffer, true)) if (!ValidateFramebufferComplete(context, drawFramebuffer))
{ {
return false; return false;
} }
...@@ -2251,7 +2251,7 @@ bool ValidateStateQuery(Context *context, GLenum pname, GLenum *nativeType, unsi ...@@ -2251,7 +2251,7 @@ bool ValidateStateQuery(Context *context, GLenum pname, GLenum *nativeType, unsi
Framebuffer *readFramebuffer = context->getGLState().getReadFramebuffer(); Framebuffer *readFramebuffer = context->getGLState().getReadFramebuffer();
ASSERT(readFramebuffer); ASSERT(readFramebuffer);
if (!ValidateFramebufferComplete(context, readFramebuffer, false)) if (!ValidateFramebufferComplete<InvalidOperation>(context, readFramebuffer))
{ {
return false; return false;
} }
...@@ -2439,7 +2439,7 @@ bool ValidateCopyTexImageParametersBase(Context *context, ...@@ -2439,7 +2439,7 @@ bool ValidateCopyTexImageParametersBase(Context *context,
const gl::State &state = context->getGLState(); const gl::State &state = context->getGLState();
Framebuffer *readFramebuffer = state.getReadFramebuffer(); Framebuffer *readFramebuffer = state.getReadFramebuffer();
if (!ValidateFramebufferComplete(context, readFramebuffer, true)) if (!ValidateFramebufferComplete(context, readFramebuffer))
{ {
return false; return false;
} }
...@@ -2670,7 +2670,7 @@ bool ValidateDrawBase(Context *context, GLenum mode, GLsizei count) ...@@ -2670,7 +2670,7 @@ bool ValidateDrawBase(Context *context, GLenum mode, GLsizei count)
} }
} }
if (!ValidateFramebufferComplete(context, framebuffer, true)) if (!ValidateFramebufferComplete(context, framebuffer))
{ {
return false; return false;
} }
...@@ -5576,7 +5576,7 @@ bool ValidateReadPixelsBase(Context *context, ...@@ -5576,7 +5576,7 @@ bool ValidateReadPixelsBase(Context *context,
Framebuffer *readFramebuffer = context->getGLState().getReadFramebuffer(); Framebuffer *readFramebuffer = context->getGLState().getReadFramebuffer();
if (!ValidateFramebufferComplete(context, readFramebuffer, true)) if (!ValidateFramebufferComplete(context, readFramebuffer))
{ {
return false; return false;
} }
...@@ -6333,33 +6333,9 @@ bool ValidateGetInternalFormativBase(Context *context, ...@@ -6333,33 +6333,9 @@ bool ValidateGetInternalFormativBase(Context *context,
return true; return true;
} }
// We should check with Khronos if returning INVALID_FRAMEBUFFER_OPERATION is OK when querying
// implementation format info for incomplete framebuffers. It seems like these queries are
// incongruent with the other errors.
bool ValidateFramebufferComplete(Context *context, Framebuffer *framebuffer, bool isFramebufferOp)
{
bool complete = false;
ANGLE_VALIDATION_TRY(framebuffer->isComplete(context, &complete));
if (!complete)
{
if (isFramebufferOp)
{
context->handleError(InvalidFramebufferOperation());
}
else
{
context->handleError(InvalidOperation());
}
return false;
}
return true;
}
bool ValidateFramebufferNotMultisampled(Context *context, Framebuffer *framebuffer) bool ValidateFramebufferNotMultisampled(Context *context, Framebuffer *framebuffer)
{ {
GLint samples = 0; if (framebuffer->getSamples(context) != 0)
ANGLE_VALIDATION_TRY(framebuffer->getSamples(context, &samples));
if (samples != 0)
{ {
context->handleError(InvalidOperation()); context->handleError(InvalidOperation());
return false; return false;
......
...@@ -10,6 +10,8 @@ ...@@ -10,6 +10,8 @@
#define LIBANGLE_VALIDATION_ES_H_ #define LIBANGLE_VALIDATION_ES_H_
#include "common/mathutil.h" #include "common/mathutil.h"
#include "libANGLE/Context.h"
#include "libANGLE/Framebuffer.h"
#include "libANGLE/PackedGLEnums.h" #include "libANGLE/PackedGLEnums.h"
#include <GLES2/gl2.h> #include <GLES2/gl2.h>
...@@ -676,7 +678,6 @@ bool ValidateGetInternalFormativBase(Context *context, ...@@ -676,7 +678,6 @@ bool ValidateGetInternalFormativBase(Context *context,
GLsizei bufSize, GLsizei bufSize,
GLsizei *numParams); GLsizei *numParams);
bool ValidateFramebufferComplete(Context *context, Framebuffer *framebuffer, bool isFramebufferOp);
bool ValidateFramebufferNotMultisampled(Context *context, Framebuffer *framebuffer); bool ValidateFramebufferNotMultisampled(Context *context, Framebuffer *framebuffer);
bool ValidateMultitextureUnit(Context *context, GLenum texture); bool ValidateMultitextureUnit(Context *context, GLenum texture);
...@@ -687,6 +688,21 @@ bool ValidateMultitextureUnit(Context *context, GLenum texture); ...@@ -687,6 +688,21 @@ bool ValidateMultitextureUnit(Context *context, GLenum texture);
return false; return false;
#define ANGLE_VALIDATION_TRY(EXPR) ANGLE_TRY_TEMPLATE(EXPR, ANGLE_HANDLE_VALIDATION_ERR); #define ANGLE_VALIDATION_TRY(EXPR) ANGLE_TRY_TEMPLATE(EXPR, ANGLE_HANDLE_VALIDATION_ERR);
// We should check with Khronos if returning INVALID_FRAMEBUFFER_OPERATION is OK when querying
// implementation format info for incomplete framebuffers. It seems like these queries are
// incongruent with the other errors.
// Inlined for speed.
template <typename ErrorStream = InvalidFramebufferOperation>
ANGLE_INLINE bool ValidateFramebufferComplete(Context *context, Framebuffer *framebuffer)
{
if (!framebuffer->isComplete(context))
{
context->handleError(ErrorStream());
return false;
}
return true;
}
} // namespace gl } // namespace gl
#endif // LIBANGLE_VALIDATION_ES_H_ #endif // LIBANGLE_VALIDATION_ES_H_
...@@ -2514,8 +2514,7 @@ bool ValidateBlitFramebufferANGLE(Context *context, ...@@ -2514,8 +2514,7 @@ bool ValidateBlitFramebufferANGLE(Context *context,
} }
} }
GLint samples = 0; GLint samples = readFramebuffer->getSamples(context);
ANGLE_VALIDATION_TRY(readFramebuffer->getSamples(context, &samples));
if (samples != 0 && if (samples != 0 &&
IsPartialBlit(context, readColorAttachment, drawColorAttachment, srcX0, srcY0, IsPartialBlit(context, readColorAttachment, drawColorAttachment, srcX0, srcY0,
srcX1, srcY1, dstX0, dstY0, dstX1, dstY1)) srcX1, srcY1, dstX0, dstY0, dstX1, dstY1))
...@@ -2566,7 +2565,7 @@ bool ValidateClear(Context *context, GLbitfield mask) ...@@ -2566,7 +2565,7 @@ bool ValidateClear(Context *context, GLbitfield mask)
{ {
Framebuffer *fbo = context->getGLState().getDrawFramebuffer(); Framebuffer *fbo = context->getGLState().getDrawFramebuffer();
if (!ValidateFramebufferComplete(context, fbo, true)) if (!ValidateFramebufferComplete(context, fbo))
{ {
return false; return false;
} }
......
...@@ -808,7 +808,7 @@ bool ValidateES3CopyTexImageParametersBase(Context *context, ...@@ -808,7 +808,7 @@ bool ValidateES3CopyTexImageParametersBase(Context *context,
gl::Framebuffer *framebuffer = state.getReadFramebuffer(); gl::Framebuffer *framebuffer = state.getReadFramebuffer();
GLuint readFramebufferID = framebuffer->id(); GLuint readFramebufferID = framebuffer->id();
if (!ValidateFramebufferComplete(context, framebuffer, true)) if (!ValidateFramebufferComplete(context, framebuffer))
{ {
return false; return false;
} }
...@@ -1244,7 +1244,7 @@ bool ValidateClearBuffer(Context *context) ...@@ -1244,7 +1244,7 @@ bool ValidateClearBuffer(Context *context)
return false; return false;
} }
if (!ValidateFramebufferComplete(context, context->getGLState().getDrawFramebuffer(), true)) if (!ValidateFramebufferComplete(context, context->getGLState().getDrawFramebuffer()))
{ {
return false; return false;
} }
......
...@@ -1075,8 +1075,7 @@ bool ValidateGetMultisamplefv(Context *context, GLenum pname, GLuint index, GLfl ...@@ -1075,8 +1075,7 @@ bool ValidateGetMultisamplefv(Context *context, GLenum pname, GLuint index, GLfl
} }
Framebuffer *framebuffer = context->getGLState().getDrawFramebuffer(); Framebuffer *framebuffer = context->getGLState().getDrawFramebuffer();
GLint samples = 0; GLint samples = framebuffer->getSamples(context);
ANGLE_VALIDATION_TRY(framebuffer->getSamples(context, &samples));
if (index >= static_cast<GLuint>(samples)) if (index >= static_cast<GLuint>(samples))
{ {
......
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