Framebuffer: Handle errors in checkStatus.
This pipes a lot more errors around in the Validation, where
they now will be caught.
Bug: angleproject:2372
Change-Id: Ibb4e47ddc932995a02dd92e10578b7a4097182a9
Reviewed-on: https://chromium-review.googlesource.com/954406
Reviewed-by: Luc Ferron <lucferron@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
diff --git a/src/libANGLE/Context.cpp b/src/libANGLE/Context.cpp
index dd554a3..ad3dad0 100644
--- a/src/libANGLE/Context.cpp
+++ b/src/libANGLE/Context.cpp
@@ -1593,7 +1593,7 @@
*params = mCaps.maxGeometryShaderStorageBlocks;
break;
default:
- mGLState.getIntegerv(this, pname, params);
+ handleError(mGLState.getIntegerv(this, pname, params));
break;
}
}
@@ -3339,7 +3339,9 @@
Framebuffer *framebuffer = mGLState.getTargetFramebuffer(target);
ASSERT(framebuffer);
- if (framebuffer->checkStatus(this) != GL_FRAMEBUFFER_COMPLETE)
+ bool complete = false;
+ ANGLE_CONTEXT_TRY(framebuffer->isComplete(this, &complete));
+ if (!complete)
{
return;
}
@@ -3361,7 +3363,9 @@
Framebuffer *framebuffer = mGLState.getTargetFramebuffer(target);
ASSERT(framebuffer);
- if (framebuffer->checkStatus(this) != GL_FRAMEBUFFER_COMPLETE)
+ bool complete = false;
+ ANGLE_CONTEXT_TRY(framebuffer->isComplete(this, &complete));
+ if (!complete)
{
return;
}
@@ -4419,7 +4423,14 @@
Framebuffer *framebuffer = mGLState.getTargetFramebuffer(target);
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)
diff --git a/src/libANGLE/Framebuffer.cpp b/src/libANGLE/Framebuffer.cpp
index 8d13266..1087c17 100644
--- a/src/libANGLE/Framebuffer.cpp
+++ b/src/libANGLE/Framebuffer.cpp
@@ -981,7 +981,7 @@
}
}
-GLenum Framebuffer::checkStatus(const Context *context)
+Error Framebuffer::checkStatus(const Context *context, GLenum *statusOut)
{
// 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
@@ -991,18 +991,29 @@
ASSERT(mCachedStatus.valid());
ASSERT(mCachedStatus.value() == GL_FRAMEBUFFER_COMPLETE ||
mCachedStatus.value() == GL_FRAMEBUFFER_UNDEFINED_OES);
- return mCachedStatus.value();
+ *statusOut = mCachedStatus.value();
+ return NoError();
}
if (hasAnyDirtyBit() || !mCachedStatus.valid())
{
- mCachedStatus = checkStatusImpl(context);
+ mCachedStatus = checkStatusWithGLFrontEnd(context);
+
+ if (mCachedStatus.value() == GL_FRAMEBUFFER_COMPLETE)
+ {
+ ANGLE_TRY(syncState(context));
+ if (!mImpl->checkStatus(context))
+ {
+ mCachedStatus = GL_FRAMEBUFFER_UNSUPPORTED;
+ }
+ }
}
- return mCachedStatus.value();
+ *statusOut = mCachedStatus.value();
+ return NoError();
}
-GLenum Framebuffer::checkStatusImpl(const Context *context)
+GLenum Framebuffer::checkStatusWithGLFrontEnd(const Context *context)
{
const ContextState &state = context->getContextState();
@@ -1199,13 +1210,6 @@
}
}
- // TODO(jmadill): Don't swallow an error here. http://anglebug.com/2372
- ANGLE_SWALLOW_ERR(syncState(context));
- if (!mImpl->checkStatus(context))
- {
- return GL_FRAMEBUFFER_UNSUPPORTED;
- }
-
return GL_FRAMEBUFFER_COMPLETE;
}
@@ -1425,14 +1429,12 @@
return mImpl->blit(context, sourceArea, destArea, blitMask, filter);
}
-int Framebuffer::getSamples(const Context *context)
+Error Framebuffer::getSamples(const Context *context, int *samplesOut)
{
- if (complete(context))
- {
- return getCachedSamples(context);
- }
-
- return 0;
+ bool completeness = false;
+ ANGLE_TRY(isComplete(context, &completeness));
+ *samplesOut = completeness ? getCachedSamples(context) : 0;
+ return NoError();
}
int Framebuffer::getCachedSamples(const Context *context)
@@ -1724,6 +1726,8 @@
mDirtyBits.set(dirtyBit);
mState.mResourceNeedsInit.set(dirtyBit, attachment->initState() == InitState::MayNeedInit);
onDirtyBinding->bind(resource ? resource->getSubject() : nullptr);
+
+ invalidateCompletenessCache();
}
void Framebuffer::resetAttachment(const Context *context, GLenum binding)
@@ -1738,10 +1742,6 @@
mDirtyBitsGuard = mDirtyBits;
ANGLE_TRY(mImpl->syncState(context, mDirtyBits));
mDirtyBits.reset();
- if (mId != 0)
- {
- mCachedStatus.reset();
- }
mDirtyBitsGuard.reset();
}
return NoError();
@@ -1788,9 +1788,12 @@
}
}
-bool Framebuffer::complete(const Context *context)
+Error Framebuffer::isComplete(const Context *context, bool *completeOut)
{
- return (checkStatus(context) == GL_FRAMEBUFFER_COMPLETE);
+ GLenum status = GL_NONE;
+ ANGLE_TRY(checkStatus(context, &status));
+ *completeOut = (status == GL_FRAMEBUFFER_COMPLETE);
+ return NoError();
}
bool Framebuffer::formsRenderingFeedbackLoopWith(const State &state) const
@@ -1901,24 +1904,28 @@
{
mState.mDefaultWidth = defaultWidth;
mDirtyBits.set(DIRTY_BIT_DEFAULT_WIDTH);
+ invalidateCompletenessCache();
}
void Framebuffer::setDefaultHeight(GLint defaultHeight)
{
mState.mDefaultHeight = defaultHeight;
mDirtyBits.set(DIRTY_BIT_DEFAULT_HEIGHT);
+ invalidateCompletenessCache();
}
void Framebuffer::setDefaultSamples(GLint defaultSamples)
{
mState.mDefaultSamples = defaultSamples;
mDirtyBits.set(DIRTY_BIT_DEFAULT_SAMPLES);
+ invalidateCompletenessCache();
}
void Framebuffer::setDefaultFixedSampleLocations(bool defaultFixedSampleLocations)
{
mState.mDefaultFixedSampleLocations = defaultFixedSampleLocations;
mDirtyBits.set(DIRTY_BIT_DEFAULT_FIXED_SAMPLE_LOCATIONS);
+ invalidateCompletenessCache();
}
GLsizei Framebuffer::getNumViews() const
diff --git a/src/libANGLE/Framebuffer.h b/src/libANGLE/Framebuffer.h
index 9d6b3fd..32a8938 100644
--- a/src/libANGLE/Framebuffer.h
+++ b/src/libANGLE/Framebuffer.h
@@ -214,7 +214,7 @@
bool usingExtendedDrawBuffers() const;
// This method calls checkStatus.
- int getSamples(const Context *context);
+ Error getSamples(const Context *context, int *samplesOut);
Error getSamplePosition(size_t index, GLfloat *xy) const;
@@ -229,13 +229,13 @@
void invalidateCompletenessCache();
- GLenum checkStatus(const Context *context);
+ Error checkStatus(const Context *context, GLenum *statusOut);
// For when we don't want to check completeness in getSamples().
int getCachedSamples(const Context *context);
// Helper for checkStatus == GL_FRAMEBUFFER_COMPLETE.
- bool complete(const Context *context);
+ Error isComplete(const Context *context, bool *completeOut);
bool hasValidDepthStencil() const;
@@ -328,7 +328,7 @@
GLenum matchType,
GLuint matchId,
size_t dirtyBit);
- GLenum checkStatusImpl(const Context *context);
+ GLenum checkStatusWithGLFrontEnd(const Context *context);
void setAttachment(const Context *context,
GLenum type,
GLenum binding,
diff --git a/src/libANGLE/State.cpp b/src/libANGLE/State.cpp
index 8490a5a..3d65e42 100644
--- a/src/libANGLE/State.cpp
+++ b/src/libANGLE/State.cpp
@@ -1831,7 +1831,7 @@
}
}
-void State::getIntegerv(const Context *context, GLenum pname, GLint *params)
+Error State::getIntegerv(const Context *context, GLenum pname, GLint *params)
{
if (pname >= GL_DRAW_BUFFER0_EXT && pname <= GL_DRAW_BUFFER15_EXT)
{
@@ -1839,7 +1839,7 @@
ASSERT(colorAttachment < mMaxDrawBuffers);
Framebuffer *framebuffer = mDrawFramebuffer;
*params = framebuffer->getDrawBufferState(colorAttachment);
- return;
+ return NoError();
}
// Please note: DEPTH_CLEAR_VALUE is not included in our internal getIntegerv implementation
@@ -1934,12 +1934,16 @@
case GL_SAMPLES:
{
Framebuffer *framebuffer = mDrawFramebuffer;
- if (framebuffer->checkStatus(context) == GL_FRAMEBUFFER_COMPLETE)
+ bool complete = false;
+ ANGLE_TRY(framebuffer->isComplete(context, &complete));
+ if (complete)
{
+ GLint samples = 0;
+ ANGLE_TRY(framebuffer->getSamples(context, &samples));
switch (pname)
{
case GL_SAMPLE_BUFFERS:
- if (framebuffer->getSamples(context) != 0)
+ if (samples != 0)
{
*params = 1;
}
@@ -1949,7 +1953,7 @@
}
break;
case GL_SAMPLES:
- *params = framebuffer->getSamples(context);
+ *params = samples;
break;
}
}
@@ -2123,6 +2127,8 @@
UNREACHABLE();
break;
}
+
+ return NoError();
}
void State::getPointerv(GLenum pname, void **params) const
diff --git a/src/libANGLE/State.h b/src/libANGLE/State.h
index 9a93a0d..3d823a3 100644
--- a/src/libANGLE/State.h
+++ b/src/libANGLE/State.h
@@ -333,7 +333,7 @@
// State query functions
void getBooleanv(GLenum pname, GLboolean *params);
void getFloatv(GLenum pname, GLfloat *params);
- void getIntegerv(const Context *context, GLenum pname, GLint *params);
+ Error getIntegerv(const Context *context, GLenum pname, GLint *params);
void getPointerv(GLenum pname, void **params) const;
void getIntegeri_v(GLenum target, GLuint index, GLint *data);
void getInteger64i_v(GLenum target, GLuint index, GLint64 *data);
diff --git a/src/libANGLE/validationES.cpp b/src/libANGLE/validationES.cpp
index 47cc3c7..86c898f 100644
--- a/src/libANGLE/validationES.cpp
+++ b/src/libANGLE/validationES.cpp
@@ -1245,15 +1245,13 @@
return false;
}
- if (readFramebuffer->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+ if (!ValidateFramebufferComplete(context, readFramebuffer, true))
{
- context->handleError(InvalidFramebufferOperation());
return false;
}
- if (drawFramebuffer->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+ if (!ValidateFramebufferComplete(context, drawFramebuffer, true))
{
- context->handleError(InvalidFramebufferOperation());
return false;
}
@@ -1263,9 +1261,8 @@
return false;
}
- if (drawFramebuffer->getSamples(context) != 0)
+ if (!ValidateFramebufferNotMultisampled(context, drawFramebuffer))
{
- context->handleError(InvalidOperation());
return false;
}
@@ -2183,23 +2180,21 @@
case GL_IMPLEMENTATION_COLOR_READ_TYPE:
case GL_IMPLEMENTATION_COLOR_READ_FORMAT:
{
- if (context->getGLState().getReadFramebuffer()->checkStatus(context) !=
- GL_FRAMEBUFFER_COMPLETE)
+ Framebuffer *readFramebuffer = context->getGLState().getReadFramebuffer();
+ ASSERT(readFramebuffer);
+
+ if (!ValidateFramebufferComplete(context, readFramebuffer, false))
{
- context->handleError(InvalidOperation());
return false;
}
- const Framebuffer *framebuffer = context->getGLState().getReadFramebuffer();
- ASSERT(framebuffer);
-
- if (framebuffer->getReadBufferState() == GL_NONE)
+ if (readFramebuffer->getReadBufferState() == GL_NONE)
{
ANGLE_VALIDATION_ERR(context, InvalidOperation(), ReadBufferNone);
return false;
}
- const FramebufferAttachment *attachment = framebuffer->getReadColorbuffer();
+ const FramebufferAttachment *attachment = readFramebuffer->getReadColorbuffer();
if (!attachment)
{
context->handleError(InvalidOperation());
@@ -2293,17 +2288,15 @@
return false;
}
- const auto &state = context->getGLState();
+ const gl::State &state = context->getGLState();
Framebuffer *readFramebuffer = state.getReadFramebuffer();
- if (readFramebuffer->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+ if (!ValidateFramebufferComplete(context, readFramebuffer, true))
{
- context->handleError(InvalidFramebufferOperation());
return false;
}
- if (readFramebuffer->id() != 0 && readFramebuffer->getSamples(context) != 0)
+ if (readFramebuffer->id() != 0 && !ValidateFramebufferNotMultisampled(context, readFramebuffer))
{
- context->handleError(InvalidOperation());
return false;
}
@@ -2510,9 +2503,8 @@
}
}
- if (framebuffer->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+ if (!ValidateFramebufferComplete(context, framebuffer, true))
{
- context->handleError(InvalidFramebufferOperation());
return false;
}
@@ -5169,15 +5161,13 @@
Framebuffer *readFramebuffer = context->getGLState().getReadFramebuffer();
- if (readFramebuffer->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+ if (!ValidateFramebufferComplete(context, readFramebuffer, true))
{
- context->handleError(InvalidFramebufferOperation());
return false;
}
- if (readFramebuffer->id() != 0 && readFramebuffer->getSamples(context) != 0)
+ if (readFramebuffer->id() != 0 && !ValidateFramebufferNotMultisampled(context, readFramebuffer))
{
- context->handleError(InvalidOperation());
return false;
}
@@ -5924,4 +5914,38 @@
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)
+{
+ GLint samples = 0;
+ ANGLE_VALIDATION_TRY(framebuffer->getSamples(context, &samples));
+ if (samples != 0)
+ {
+ context->handleError(InvalidOperation());
+ return false;
+ }
+ return true;
+}
+
} // namespace gl
diff --git a/src/libANGLE/validationES.h b/src/libANGLE/validationES.h
index c9aa435..59954a0 100644
--- a/src/libANGLE/validationES.h
+++ b/src/libANGLE/validationES.h
@@ -26,6 +26,7 @@
{
class Context;
struct Format;
+class Framebuffer;
struct LinkedUniform;
class Program;
class Shader;
@@ -592,6 +593,15 @@
GLsizei bufSize,
GLsizei *numParams);
+bool ValidateFramebufferComplete(Context *context, Framebuffer *framebuffer, bool isFramebufferOp);
+bool ValidateFramebufferNotMultisampled(Context *context, Framebuffer *framebuffer);
+
+// Utility macro for handling implementation methods inside Validation.
+#define ANGLE_HANDLE_VALIDATION_ERR(X) \
+ context->handleError(X); \
+ return false;
+#define ANGLE_VALIDATION_TRY(EXPR) ANGLE_TRY_TEMPLATE(EXPR, ANGLE_HANDLE_VALIDATION_ERR);
+
} // namespace gl
#endif // LIBANGLE_VALIDATION_ES_H_
diff --git a/src/libANGLE/validationES2.cpp b/src/libANGLE/validationES2.cpp
index d9261c5..10c6e5c 100644
--- a/src/libANGLE/validationES2.cpp
+++ b/src/libANGLE/validationES2.cpp
@@ -2493,7 +2493,9 @@
}
}
- if (readFramebuffer->getSamples(context) != 0 &&
+ GLint samples = 0;
+ ANGLE_VALIDATION_TRY(readFramebuffer->getSamples(context, &samples));
+ if (samples != 0 &&
IsPartialBlit(context, readColorAttachment, drawColorAttachment, srcX0, srcY0,
srcX1, srcY1, dstX0, dstY0, dstX1, dstY1))
{
@@ -2542,9 +2544,9 @@
bool ValidateClear(Context *context, GLbitfield mask)
{
Framebuffer *fbo = context->getGLState().getDrawFramebuffer();
- if (fbo->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+
+ if (!ValidateFramebufferComplete(context, fbo, true))
{
- context->handleError(InvalidFramebufferOperation());
return false;
}
diff --git a/src/libANGLE/validationES3.cpp b/src/libANGLE/validationES3.cpp
index 2840482..07f8058 100644
--- a/src/libANGLE/validationES3.cpp
+++ b/src/libANGLE/validationES3.cpp
@@ -808,15 +808,13 @@
gl::Framebuffer *framebuffer = state.getReadFramebuffer();
GLuint readFramebufferID = framebuffer->id();
- if (framebuffer->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+ if (!ValidateFramebufferComplete(context, framebuffer, true))
{
- context->handleError(InvalidFramebufferOperation());
return false;
}
- if (readFramebufferID != 0 && framebuffer->getSamples(context) != 0)
+ if (readFramebufferID != 0 && !ValidateFramebufferNotMultisampled(context, framebuffer))
{
- context->handleError(InvalidOperation());
return false;
}
@@ -1246,9 +1244,8 @@
return false;
}
- if (context->getGLState().getDrawFramebuffer()->checkStatus(context) != GL_FRAMEBUFFER_COMPLETE)
+ if (!ValidateFramebufferComplete(context, context->getGLState().getDrawFramebuffer(), true))
{
- context->handleError(InvalidFramebufferOperation());
return false;
}
diff --git a/src/libANGLE/validationES31.cpp b/src/libANGLE/validationES31.cpp
index 2ceabf0..f9cf8d5 100644
--- a/src/libANGLE/validationES31.cpp
+++ b/src/libANGLE/validationES31.cpp
@@ -1048,8 +1048,10 @@
}
Framebuffer *framebuffer = context->getGLState().getDrawFramebuffer();
+ GLint samples = 0;
+ ANGLE_VALIDATION_TRY(framebuffer->getSamples(context, &samples));
- if (index >= static_cast<GLuint>(framebuffer->getSamples(context)))
+ if (index >= static_cast<GLuint>(samples))
{
context->handleError(InvalidValue() << "Index must be less than the value of SAMPLES.");
return false;