From 491c2a5496a263d51c6fdd731f06365addaf61ba Mon Sep 17 00:00:00 2001 From: Shahbaz Youssefi Date: Tue, 11 Jul 2023 15:20:09 -0400 Subject: [PATCH] Split the context-private part of the state cache In preparation for passing it directly to entry point implementations, ensuring no access to the share-group-accessible part. Bug: angleproject:8224 Change-Id: I705e6a8fb5204bab71caffff4dcb56d16c3d6e10 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4678782 Reviewed-by: Charlie Lao Reviewed-by: Geoff Lang Commit-Queue: Shahbaz Youssefi --- src/libANGLE/Context.cpp | 57 ++++------- src/libANGLE/Context.h | 106 +++++++++++---------- src/libANGLE/context_private_call_gles.cpp | 52 +++++----- src/libANGLE/validationES.h | 3 +- 4 files changed, 103 insertions(+), 115 deletions(-) diff --git a/src/libANGLE/Context.cpp b/src/libANGLE/Context.cpp index f5bf14eb3..1b167afd9 100644 --- a/src/libANGLE/Context.cpp +++ b/src/libANGLE/Context.cpp @@ -9717,7 +9717,7 @@ void Context::setLogicOpEnabledForGLES1(bool enabled) { // Same implementation as ContextPrivateEnable(GL_COLOR_LOGIC_OP), without the GLES1 forwarding. getMutablePrivateState()->setLogicOpEnabled(enabled); - onContextPrivateCapChange(); + getPrivateStateCache().onCapChange(); } egl::Error Context::releaseHighPowerGPU() @@ -10038,8 +10038,7 @@ StateCache::StateCache() mCachedBasicDrawElementsError(kInvalidPointer), mCachedProgramPipelineError(kInvalidPointer), mCachedTransformFeedbackActiveUnpaused(false), - mCachedCanDraw(false), - mIsCachedBasicDrawStatesErrorValid(true) + mCachedCanDraw(false) { mCachedValidDrawModes.fill(false); } @@ -10152,11 +10151,13 @@ void StateCache::updateBasicDrawElementsError() mCachedBasicDrawElementsError = kInvalidPointer; } -intptr_t StateCache::getBasicDrawStatesErrorImpl(const Context *context) const +intptr_t StateCache::getBasicDrawStatesErrorImpl(const Context *context, + const PrivateStateCache *privateStateCache) const { ASSERT(mCachedBasicDrawStatesErrorString == kInvalidPointer || - !mIsCachedBasicDrawStatesErrorValid); - ASSERT(mCachedBasicDrawStatesErrorCode == GL_NO_ERROR || !mIsCachedBasicDrawStatesErrorValid); + !privateStateCache->isCachedBasicDrawStatesErrorValid()); + ASSERT(mCachedBasicDrawStatesErrorCode == GL_NO_ERROR || + !privateStateCache->isCachedBasicDrawStatesErrorValid()); // Only assign the error code after ValidateDrawStates has completed. ValidateDrawStates calls // updateBasicDrawStatesError in some cases and resets the value mid-call. @@ -10170,7 +10171,7 @@ intptr_t StateCache::getBasicDrawStatesErrorImpl(const Context *context) const ASSERT((mCachedBasicDrawStatesErrorString == 0) == (mCachedBasicDrawStatesErrorCode == GL_NO_ERROR)); - mIsCachedBasicDrawStatesErrorValid = true; + privateStateCache->setCachedBasicDrawStatesErrorValid(); return mCachedBasicDrawStatesErrorString; } @@ -10243,21 +10244,6 @@ void StateCache::onDrawFramebufferChange(Context *context) updateBasicDrawStatesError(); } -void StateCache::onContextPrivateCapChange(Context *context) -{ - mIsCachedBasicDrawStatesErrorValid = false; -} - -void StateCache::onContextPrivateStencilStateChange(Context *context) -{ - mIsCachedBasicDrawStatesErrorValid = false; -} - -void StateCache::onContextPrivateDefaultVertexAttributeChange(Context *context) -{ - mIsCachedBasicDrawStatesErrorValid = false; -} - void StateCache::onActiveTextureChange(Context *context) { updateBasicDrawStatesError(); @@ -10291,21 +10277,6 @@ void StateCache::onShaderStorageBufferStateChange(Context *context) updateBasicDrawStatesError(); } -void StateCache::onContextPrivateColorMaskChange(Context *context) -{ - mIsCachedBasicDrawStatesErrorValid = false; -} - -void StateCache::onContextPrivateBlendFuncIndexedChange(Context *context) -{ - mIsCachedBasicDrawStatesErrorValid = false; -} - -void StateCache::onContextPrivateBlendEquationChange(Context *context) -{ - mIsCachedBasicDrawStatesErrorValid = false; -} - void StateCache::setValidDrawModes(bool pointsOK, bool linesOK, bool trisOK, @@ -10509,4 +10480,16 @@ void StateCache::updateCanDraw(Context *context) (context->isGLES1() || (context->getState().getProgramExecutable() && context->getState().getProgramExecutable()->hasVertexShader())); } + +bool StateCache::isCurrentContext(const Context *context, + const PrivateStateCache *privateStateCache) const +{ + // Ensure that the state cache is not queried by any context other than the one that owns it. + return &context->getStateCache() == this && + &context->getPrivateStateCache() == privateStateCache; +} + +PrivateStateCache::PrivateStateCache() : mIsCachedBasicDrawStatesErrorValid(true) {} + +PrivateStateCache::~PrivateStateCache() = default; } // namespace gl diff --git a/src/libANGLE/Context.h b/src/libANGLE/Context.h index 273192273..0f5e9d208 100644 --- a/src/libANGLE/Context.h +++ b/src/libANGLE/Context.h @@ -116,6 +116,34 @@ enum class VertexAttribTypeCase ValidSize3or4 = 3, }; +// Part of StateCache (see below) that is private to the context and is inaccessible to other +// contexts. +class PrivateStateCache final : angle::NonCopyable +{ + public: + PrivateStateCache(); + ~PrivateStateCache(); + + void onCapChange() { mIsCachedBasicDrawStatesErrorValid = false; } + void onColorMaskChange() { mIsCachedBasicDrawStatesErrorValid = false; } + void onDefaultVertexAttributeChange() { mIsCachedBasicDrawStatesErrorValid = false; } + void onBlendFuncIndexedChange() { mIsCachedBasicDrawStatesErrorValid = false; } + void onBlendEquationChange() { mIsCachedBasicDrawStatesErrorValid = false; } + void onStencilStateChange() { mIsCachedBasicDrawStatesErrorValid = false; } + + bool isCachedBasicDrawStatesErrorValid() const { return mIsCachedBasicDrawStatesErrorValid; } + void setCachedBasicDrawStatesErrorValid() const { mIsCachedBasicDrawStatesErrorValid = true; } + + private: + // StateCache::mCachedBasicDrawStatesError* may be invalidated through numerous calls (see the + // comment on getBasicDrawStatesErrorString), some of which may originate from other contexts + // (through the observer interface). However, ContextPrivate* helpers may also need to + // invalidate the draw states, but they are called without holding the share group lock. The + // following tracks whether StateCache::mCachedBasicDrawStatesError* values are valid and is + // accessed only by the context itself. + mutable bool mIsCachedBasicDrawStatesErrorValid; +}; + // Helper class for managing cache variables and state changes. class StateCache final : angle::NonCopyable { @@ -155,26 +183,31 @@ class StateCache final : angle::NonCopyable // 4. onVertexArrayStateChange. // 5. onVertexArrayBufferStateChange. // 6. onDrawFramebufferChange. - // 7. onContextPrivateCapChange. - // 8. onContextPrivateStencilStateChange. - // 9. onContextPrivateDefaultVertexAttributeChange. - // 10. onActiveTextureChange. - // 11. onQueryChange. - // 12. onActiveTransformFeedbackChange. - // 13. onUniformBufferStateChange. - // 14. onContextPrivateColorMaskChange. - // 15. onBufferBindingChange. - // 16. onContextPrivateBlendFuncIndexedChange. - // 17. onContextPrivateBlendEquationChange. - intptr_t getBasicDrawStatesErrorString(const Context *context) const + // 7. onActiveTextureChange. + // 8. onQueryChange. + // 9. onActiveTransformFeedbackChange. + // 10. onUniformBufferStateChange. + // 11. onBufferBindingChange. + // + // Additionally, the following in PrivateStateCache can lead to updateBasicDrawStatesError: + // 1. onCapChange. + // 2. onStencilStateChange. + // 3. onDefaultVertexAttributeChange. + // 4. onColorMaskChange. + // 5. onBlendFuncIndexedChange. + // 6. onBlendEquationChange. + intptr_t getBasicDrawStatesErrorString(const Context *context, + const PrivateStateCache *privateStateCache) const { - if (mIsCachedBasicDrawStatesErrorValid && + // This is only ever called with the context that owns this state cache + ASSERT(isCurrentContext(context, privateStateCache)); + if (privateStateCache->isCachedBasicDrawStatesErrorValid() && mCachedBasicDrawStatesErrorString != kInvalidPointer) { return mCachedBasicDrawStatesErrorString; } - return getBasicDrawStatesErrorImpl(context); + return getBasicDrawStatesErrorImpl(context, privateStateCache); } // The GL error enum to use when generating errors due to failed draw states. Only valid if @@ -283,18 +316,10 @@ class StateCache final : angle::NonCopyable void onAtomicCounterBufferStateChange(Context *context); void onShaderStorageBufferStateChange(Context *context); void onBufferBindingChange(Context *context); - // The following state change notifications are only called from context-private state change - // functions. They only affect the draw validation cache which is also context-private (i.e. - // not accessed by other contexts in the share group). Note that context-private state change - // functions are called without holding the share group lock. - void onContextPrivateCapChange(Context *context); - void onContextPrivateColorMaskChange(Context *context); - void onContextPrivateDefaultVertexAttributeChange(Context *context); - void onContextPrivateBlendFuncIndexedChange(Context *context); - void onContextPrivateBlendEquationChange(Context *context); - void onContextPrivateStencilStateChange(Context *context); private: + bool isCurrentContext(const Context *context, const PrivateStateCache *privateStateCache) const; + // Cache update functions. void updateActiveAttribsMask(Context *context); void updateVertexElementLimits(Context *context); @@ -318,7 +343,8 @@ class StateCache final : angle::NonCopyable bool triAdjOK, bool patchOK); - intptr_t getBasicDrawStatesErrorImpl(const Context *context) const; + intptr_t getBasicDrawStatesErrorImpl(const Context *context, + const PrivateStateCache *privateStateCache) const; intptr_t getProgramPipelineErrorImpl(const Context *context) const; intptr_t getBasicDrawElementsErrorImpl(const Context *context) const; @@ -363,14 +389,6 @@ class StateCache final : angle::NonCopyable mCachedIntegerVertexAttribTypesValidation; bool mCachedCanDraw; - - // mCachedBasicDrawStatesError* may be invalidated through numerous calls (see the comment on - // getBasicDrawStatesErrorString), some of which may originate from other contexts (through the - // observer interface). However, ContextPrivate* helpers may also need to invalidate the draw - // states, but they are called without holding the share group lock. The following tracks - // whether mCachedBasicDrawStatesError* values are valid and is accessed only by the context - // itself. - mutable bool mIsCachedBasicDrawStatesErrorValid; }; using VertexArrayMap = ResourceMap; @@ -575,24 +593,6 @@ class Context final : public egl::LabeledObject, angle::NonCopyable, public angl // To be used **only** directly by the entry points. PrivateState *getMutablePrivateState() { return mState.getMutablePrivateState(); } GLES1State *getMutableGLES1State() { return mState.getMutableGLES1State(); } - void onContextPrivateCapChange() { mStateCache.onContextPrivateCapChange(this); } - void onContextPrivateColorMaskChange() { mStateCache.onContextPrivateColorMaskChange(this); } - void onContextPrivateDefaultVertexAttributeChange() - { - mStateCache.onContextPrivateDefaultVertexAttributeChange(this); - } - void onContextPrivateBlendFuncIndexedChange() - { - mStateCache.onContextPrivateBlendFuncIndexedChange(this); - } - void onContextPrivateBlendEquationChange() - { - mStateCache.onContextPrivateBlendEquationChange(this); - } - void onContextPrivateStencilStateChange() - { - mStateCache.onContextPrivateStencilStateChange(this); - } bool skipValidation() const { @@ -658,6 +658,9 @@ class Context final : public egl::LabeledObject, angle::NonCopyable, public angl const StateCache &getStateCache() const { return mStateCache; } StateCache &getStateCache() { return mStateCache; } + const PrivateStateCache &getPrivateStateCache() const { return mPrivateStateCache; } + PrivateStateCache &getPrivateStateCache() { return mPrivateStateCache; } + void onSubjectStateChange(angle::SubjectIndex index, angle::SubjectMessage message) override; void onSamplerUniformChange(size_t textureUnitIndex); @@ -884,6 +887,7 @@ class Context final : public egl::LabeledObject, angle::NonCopyable, public angl state::DirtyObjects mDrawDirtyObjects; StateCache mStateCache; + PrivateStateCache mPrivateStateCache; state::DirtyObjects mTexImageDirtyObjects; state::DirtyObjects mReadPixelsDirtyObjects; diff --git a/src/libANGLE/context_private_call_gles.cpp b/src/libANGLE/context_private_call_gles.cpp index e52b3c95c..5de8e2b79 100644 --- a/src/libANGLE/context_private_call_gles.cpp +++ b/src/libANGLE/context_private_call_gles.cpp @@ -73,7 +73,7 @@ void ContextPrivateColorMask(Context *context, { context->getMutablePrivateState()->setColorMask(ConvertToBool(red), ConvertToBool(green), ConvertToBool(blue), ConvertToBool(alpha)); - context->onContextPrivateColorMaskChange(); + context->getPrivateStateCache().onColorMaskChange(); } void ContextPrivateColorMaski(Context *context, @@ -85,7 +85,7 @@ void ContextPrivateColorMaski(Context *context, { context->getMutablePrivateState()->setColorMaskIndexed( ConvertToBool(r), ConvertToBool(g), ConvertToBool(b), ConvertToBool(a), index); - context->onContextPrivateColorMaskChange(); + context->getPrivateStateCache().onColorMaskChange(); } void ContextPrivateDepthMask(Context *context, GLboolean flag) @@ -96,25 +96,25 @@ void ContextPrivateDepthMask(Context *context, GLboolean flag) void ContextPrivateDisable(Context *context, GLenum cap) { context->getMutablePrivateState()->setEnableFeature(cap, false); - context->onContextPrivateCapChange(); + context->getPrivateStateCache().onCapChange(); } void ContextPrivateDisablei(Context *context, GLenum target, GLuint index) { context->getMutablePrivateState()->setEnableFeatureIndexed(target, false, index); - context->onContextPrivateCapChange(); + context->getPrivateStateCache().onCapChange(); } void ContextPrivateEnable(Context *context, GLenum cap) { context->getMutablePrivateState()->setEnableFeature(cap, true); - context->onContextPrivateCapChange(); + context->getPrivateStateCache().onCapChange(); } void ContextPrivateEnablei(Context *context, GLenum target, GLuint index) { context->getMutablePrivateState()->setEnableFeatureIndexed(target, true, index); - context->onContextPrivateCapChange(); + context->getPrivateStateCache().onCapChange(); } void ContextPrivateActiveTexture(Context *context, GLenum texture) @@ -196,42 +196,42 @@ void ContextPrivateVertexAttrib1f(Context *context, GLuint index, GLfloat x) { GLfloat vals[4] = {x, 0, 0, 1}; context->getMutablePrivateState()->setVertexAttribf(index, vals); - context->onContextPrivateDefaultVertexAttributeChange(); + context->getPrivateStateCache().onDefaultVertexAttributeChange(); } void ContextPrivateVertexAttrib1fv(Context *context, GLuint index, const GLfloat *values) { GLfloat vals[4] = {values[0], 0, 0, 1}; context->getMutablePrivateState()->setVertexAttribf(index, vals); - context->onContextPrivateDefaultVertexAttributeChange(); + context->getPrivateStateCache().onDefaultVertexAttributeChange(); } void ContextPrivateVertexAttrib2f(Context *context, GLuint index, GLfloat x, GLfloat y) { GLfloat vals[4] = {x, y, 0, 1}; context->getMutablePrivateState()->setVertexAttribf(index, vals); - context->onContextPrivateDefaultVertexAttributeChange(); + context->getPrivateStateCache().onDefaultVertexAttributeChange(); } void ContextPrivateVertexAttrib2fv(Context *context, GLuint index, const GLfloat *values) { GLfloat vals[4] = {values[0], values[1], 0, 1}; context->getMutablePrivateState()->setVertexAttribf(index, vals); - context->onContextPrivateDefaultVertexAttributeChange(); + context->getPrivateStateCache().onDefaultVertexAttributeChange(); } void ContextPrivateVertexAttrib3f(Context *context, GLuint index, GLfloat x, GLfloat y, GLfloat z) { GLfloat vals[4] = {x, y, z, 1}; context->getMutablePrivateState()->setVertexAttribf(index, vals); - context->onContextPrivateDefaultVertexAttributeChange(); + context->getPrivateStateCache().onDefaultVertexAttributeChange(); } void ContextPrivateVertexAttrib3fv(Context *context, GLuint index, const GLfloat *values) { GLfloat vals[4] = {values[0], values[1], values[2], 1}; context->getMutablePrivateState()->setVertexAttribf(index, vals); - context->onContextPrivateDefaultVertexAttributeChange(); + context->getPrivateStateCache().onDefaultVertexAttributeChange(); } void ContextPrivateVertexAttrib4f(Context *context, @@ -243,13 +243,13 @@ void ContextPrivateVertexAttrib4f(Context *context, { GLfloat vals[4] = {x, y, z, w}; context->getMutablePrivateState()->setVertexAttribf(index, vals); - context->onContextPrivateDefaultVertexAttributeChange(); + context->getPrivateStateCache().onDefaultVertexAttributeChange(); } void ContextPrivateVertexAttrib4fv(Context *context, GLuint index, const GLfloat *values) { context->getMutablePrivateState()->setVertexAttribf(index, values); - context->onContextPrivateDefaultVertexAttributeChange(); + context->getPrivateStateCache().onDefaultVertexAttributeChange(); } void ContextPrivateVertexAttribI4i(Context *context, @@ -261,13 +261,13 @@ void ContextPrivateVertexAttribI4i(Context *context, { GLint vals[4] = {x, y, z, w}; context->getMutablePrivateState()->setVertexAttribi(index, vals); - context->onContextPrivateDefaultVertexAttributeChange(); + context->getPrivateStateCache().onDefaultVertexAttributeChange(); } void ContextPrivateVertexAttribI4iv(Context *context, GLuint index, const GLint *values) { context->getMutablePrivateState()->setVertexAttribi(index, values); - context->onContextPrivateDefaultVertexAttributeChange(); + context->getPrivateStateCache().onDefaultVertexAttributeChange(); } void ContextPrivateVertexAttribI4ui(Context *context, @@ -279,13 +279,13 @@ void ContextPrivateVertexAttribI4ui(Context *context, { GLuint vals[4] = {x, y, z, w}; context->getMutablePrivateState()->setVertexAttribu(index, vals); - context->onContextPrivateDefaultVertexAttributeChange(); + context->getPrivateStateCache().onDefaultVertexAttributeChange(); } void ContextPrivateVertexAttribI4uiv(Context *context, GLuint index, const GLuint *values) { context->getMutablePrivateState()->setVertexAttribu(index, values); - context->onContextPrivateDefaultVertexAttributeChange(); + context->getPrivateStateCache().onDefaultVertexAttributeChange(); } void ContextPrivateViewport(Context *context, GLint x, GLint y, GLsizei width, GLsizei height) @@ -370,19 +370,19 @@ void ContextPrivateBlendColor(Context *context, void ContextPrivateBlendEquation(Context *context, GLenum mode) { context->getMutablePrivateState()->setBlendEquation(mode, mode); - context->onContextPrivateBlendEquationChange(); + context->getPrivateStateCache().onBlendEquationChange(); } void ContextPrivateBlendEquationi(Context *context, GLuint buf, GLenum mode) { context->getMutablePrivateState()->setBlendEquationIndexed(mode, mode, buf); - context->onContextPrivateBlendEquationChange(); + context->getPrivateStateCache().onBlendEquationChange(); } void ContextPrivateBlendEquationSeparate(Context *context, GLenum modeRGB, GLenum modeAlpha) { context->getMutablePrivateState()->setBlendEquation(modeRGB, modeAlpha); - context->onContextPrivateBlendEquationChange(); + context->getPrivateStateCache().onBlendEquationChange(); } void ContextPrivateBlendEquationSeparatei(Context *context, @@ -391,7 +391,7 @@ void ContextPrivateBlendEquationSeparatei(Context *context, GLenum modeAlpha) { context->getMutablePrivateState()->setBlendEquationIndexed(modeRGB, modeAlpha, buf); - context->onContextPrivateBlendEquationChange(); + context->getPrivateStateCache().onBlendEquationChange(); } void ContextPrivateBlendFunc(Context *context, GLenum sfactor, GLenum dfactor) @@ -404,7 +404,7 @@ void ContextPrivateBlendFunci(Context *context, GLuint buf, GLenum src, GLenum d context->getMutablePrivateState()->setBlendFactorsIndexed(src, dst, src, dst, buf); if (context->getState().noSimultaneousConstantColorAndAlphaBlendFunc()) { - context->onContextPrivateBlendFuncIndexedChange(); + context->getPrivateStateCache().onBlendFuncIndexedChange(); } } @@ -428,7 +428,7 @@ void ContextPrivateBlendFuncSeparatei(Context *context, buf); if (context->getState().noSimultaneousConstantColorAndAlphaBlendFunc()) { - context->onContextPrivateBlendFuncIndexedChange(); + context->getPrivateStateCache().onBlendFuncIndexedChange(); } } @@ -454,7 +454,7 @@ void ContextPrivateStencilFuncSeparate(Context *context, context->getMutablePrivateState()->setStencilBackParams(func, clampedRef, mask); } - context->onContextPrivateStencilStateChange(); + context->getPrivateStateCache().onStencilStateChange(); } void ContextPrivateStencilMask(Context *context, GLuint mask) @@ -474,7 +474,7 @@ void ContextPrivateStencilMaskSeparate(Context *context, GLenum face, GLuint mas context->getMutablePrivateState()->setStencilBackWritemask(mask); } - context->onContextPrivateStencilStateChange(); + context->getPrivateStateCache().onStencilStateChange(); } void ContextPrivateStencilOp(Context *context, GLenum fail, GLenum zfail, GLenum zpass) diff --git a/src/libANGLE/validationES.h b/src/libANGLE/validationES.h index 3e7eff54c..dbed06356 100644 --- a/src/libANGLE/validationES.h +++ b/src/libANGLE/validationES.h @@ -432,7 +432,8 @@ ANGLE_INLINE bool ValidateDrawBase(const Context *context, angle::EntryPoint entryPoint, PrimitiveMode mode) { - intptr_t drawStatesError = context->getStateCache().getBasicDrawStatesErrorString(context); + intptr_t drawStatesError = context->getStateCache().getBasicDrawStatesErrorString( + context, &context->getPrivateStateCache()); if (drawStatesError) { const char *errorMessage = reinterpret_cast(drawStatesError);