From 7e0fb7e402d9f8dacdb18f467d3ca65a15cffa2f Mon Sep 17 00:00:00 2001 From: Shahbaz Youssefi Date: Wed, 5 Jul 2023 17:20:23 -0400 Subject: [PATCH] Make glIsEnabled* entry points lockless These entry points only set context-local state and thus don't require locking. Bug: angleproject:8224 Change-Id: I6fe40bf4381e1d42248358f773ec9d5675883ada Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4666356 Reviewed-by: Charlie Lao Reviewed-by: Igor Nazarov Commit-Queue: Shahbaz Youssefi --- .../GL_EGL_entry_points.json | 14 +++--- scripts/generate_entry_points.py | 46 +++++++++++++++++-- src/libANGLE/Context.cpp | 10 ---- src/libANGLE/Context_gles_2_0_autogen.h | 1 - src/libANGLE/Context_gles_3_2_autogen.h | 1 - src/libANGLE/State.h | 1 + src/libANGLE/context_local_call_gles.cpp | 9 ++++ .../context_local_call_gles_autogen.h | 2 + .../entry_points_gles_2_0_autogen.cpp | 3 +- .../entry_points_gles_3_2_autogen.cpp | 3 +- .../entry_points_gles_ext_autogen.cpp | 6 +-- 11 files changed, 64 insertions(+), 32 deletions(-) diff --git a/scripts/code_generation_hashes/GL_EGL_entry_points.json b/scripts/code_generation_hashes/GL_EGL_entry_points.json index d3c830049..b5a245c2f 100644 --- a/scripts/code_generation_hashes/GL_EGL_entry_points.json +++ b/scripts/code_generation_hashes/GL_EGL_entry_points.json @@ -6,7 +6,7 @@ "scripts/entry_point_packed_gl_enums.json": "1c6b036918aabb9822a638fbf33f87f4", "scripts/generate_entry_points.py": - "71a200020b2e7d42d6a7745244b55935", + "01e764c05b0017ccc7bf5b3ff743f9ec", "scripts/gl_angle_ext.xml": "49a0bf469d6f44c532098ef3a9fd087f", "scripts/registry_xml.py": @@ -30,13 +30,13 @@ "src/libANGLE/Context_gles_1_0_autogen.h": "b0093ce99cb355161c6f1ac8b4ded94f", "src/libANGLE/Context_gles_2_0_autogen.h": - "e0f142a484e9e4b856e53ed043eb1d33", + "bf03a51e6a27817e503bb822868d290e", "src/libANGLE/Context_gles_3_0_autogen.h": "d1697421290173be5ca0bc236fd479b4", "src/libANGLE/Context_gles_3_1_autogen.h": "17b51301bb3edd475460e8e77ff21a1c", "src/libANGLE/Context_gles_3_2_autogen.h": - "1da3565a1424a371b33dce5cae018cb5", + "84d3d40446ba74d0fbf84dc523cf1073", "src/libANGLE/Context_gles_ext_autogen.h": "289bd74bd8002bcc2a07d54de9672a3c", "src/libANGLE/capture/capture_egl_autogen.cpp": @@ -86,7 +86,7 @@ "src/libANGLE/context_local_call_gl_autogen.h": "f4171b14eac111a51ec641e24e13af42", "src/libANGLE/context_local_call_gles_autogen.h": - "e1b54321a764a59e33a5d47e0c3e68dd", + "ba14981bf40a7f4a4f989b1356b035ad", "src/libANGLE/validationCL_autogen.h": "0022d0cdb6a9e2ef4a59b71164f62333", "src/libANGLE/validationEGL_autogen.h": @@ -158,7 +158,7 @@ "src/libGLESv2/entry_points_gles_1_0_autogen.h": "1d3aef77845a416497070985a8e9cb31", "src/libGLESv2/entry_points_gles_2_0_autogen.cpp": - "d7c98d1846a9df56eb74f9f962881dd7", + "9ed76031c0e2294c2ade46ec7d041e99", "src/libGLESv2/entry_points_gles_2_0_autogen.h": "691c60c2dfed9beca68aa1f32aa2c71b", "src/libGLESv2/entry_points_gles_3_0_autogen.cpp": @@ -170,11 +170,11 @@ "src/libGLESv2/entry_points_gles_3_1_autogen.h": "a7327c330a91665fc31accbb78793b42", "src/libGLESv2/entry_points_gles_3_2_autogen.cpp": - "44afb72ba15730097e68862d43868f3d", + "386af5a47a38b4e63a9967ed644d62de", "src/libGLESv2/entry_points_gles_3_2_autogen.h": "647f932a299cdb4726b60bbba059f0d2", "src/libGLESv2/entry_points_gles_ext_autogen.cpp": - "9d95637820bedc4205ecb493ceaeaf3c", + "7d00b1545f3d0d5621e54103675b02fe", "src/libGLESv2/entry_points_gles_ext_autogen.h": "7bb44566362d1de21552faf427517085", "src/libGLESv2/libGLESv2_autogen.cpp": diff --git a/scripts/generate_entry_points.py b/scripts/generate_entry_points.py index d1a74a717..3957fa0bc 100755 --- a/scripts/generate_entry_points.py +++ b/scripts/generate_entry_points.py @@ -155,6 +155,8 @@ CONTEXT_LOCAL_LIST = [ 'glEnablei', 'glFrontFace', 'glHint', + 'glIsEnabled', + 'glIsEnabledi', 'glLineWidth', 'glLogicOpANGLE', 'glMinSampleShading', @@ -387,6 +389,36 @@ TEMPLATE_GLES_ENTRY_POINT_WITH_RETURN = """\ }} """ +TEMPLATE_GLES_LOCAL_STATE_ENTRY_POINT_WITH_RETURN = """\ +{return_type} GL_APIENTRY GL_{name}({params}) +{{ + Context *context = {context_getter}; + {event_comment}EVENT(context, GL{name}, "context = %d{comma_if_needed}{format_params}", CID(context){comma_if_needed}{pass_params}); + + {return_type} returnValue; + if ({valid_context_check}) + {{{packed_gl_enum_conversions} + bool isCallValid = (context->skipValidation() || {validation_expression}); + if (isCallValid) + {{ + returnValue = ContextLocal{name_no_suffix}(context, {internal_params}); + }} + else + {{ + returnValue = GetDefaultReturnValue(); + }} + ANGLE_CAPTURE_GL({name}, isCallValid, {gl_capture_params}, returnValue); + }} + else + {{ + {constext_lost_error_generator} + returnValue = GetDefaultReturnValue(); + }} + ASSERT(!egl::Display::GetCurrentThreadUnlockedTailCall()->any()); + return returnValue; +}} +""" + TEMPLATE_EGL_ENTRY_POINT_NO_RETURN = """\ void EGLAPIENTRY EGL_{name}({params}) {{ @@ -1227,7 +1259,7 @@ TEMPLATE_CAPTURE_PROTO = "angle::CallCapture Capture%s(%s);" TEMPLATE_VALIDATION_PROTO = "%s Validate%s(%s);" -TEMPLATE_CONTEXT_LOCAL_CALL_PROTO = "void ContextLocal%s(%s);" +TEMPLATE_CONTEXT_LOCAL_CALL_PROTO = "%s ContextLocal%s(%s);" TEMPLATE_CONTEXT_LOCK_PROTO = "ScopedContextMutexLock GetContextLock_%s(%s);" @@ -1769,6 +1801,8 @@ def get_def_template(api, cmd_name, return_type, has_errcode_ret): return TEMPLATE_CL_ENTRY_POINT_WITH_ERRCODE_RET else: return TEMPLATE_CL_ENTRY_POINT_WITH_RETURN_POINTER + elif is_context_local_state_command(api, cmd_name): + return TEMPLATE_GLES_LOCAL_STATE_ENTRY_POINT_WITH_RETURN else: return TEMPLATE_GLES_ENTRY_POINT_WITH_RETURN @@ -2075,7 +2109,7 @@ def format_validation_proto(api, cmd_name, params, cmd_packed_gl_enums, packed_p return TEMPLATE_VALIDATION_PROTO % (return_type, strip_api_prefix(cmd_name), internal_params) -def format_context_local_call_proto(api, cmd_name, params, cmd_packed_gl_enums, +def format_context_local_call_proto(api, cmd_name, proto, params, cmd_packed_gl_enums, packed_param_types): with_extra_params = ["Context *context"] + params packed_enums = get_packed_enums(api, cmd_packed_gl_enums, cmd_name, packed_param_types, @@ -2083,7 +2117,9 @@ def format_context_local_call_proto(api, cmd_name, params, cmd_packed_gl_enums, internal_params = get_context_local_call_params(api, cmd_name, with_extra_params, cmd_packed_gl_enums, packed_param_types) stripped_name = strip_suffix(api, strip_api_prefix(cmd_name)) - return TEMPLATE_CONTEXT_LOCAL_CALL_PROTO % (stripped_name, internal_params), stripped_name + return_type = proto[:-len(cmd_name)].strip() + return TEMPLATE_CONTEXT_LOCAL_CALL_PROTO % (return_type, stripped_name, + internal_params), stripped_name def format_context_lock_proto(api, cmd_name, params, cmd_packed_gl_enums, packed_param_types): @@ -2146,8 +2182,8 @@ class ANGLEEntryPoints(registry_xml.EntryPoints): packed_param_types)) if is_context_local_state_command(self.api, cmd_name): - proto, function = format_context_local_call_proto(self.api, cmd_name, param_text, - cmd_packed_enums, + proto, function = format_context_local_call_proto(self.api, cmd_name, proto_text, + param_text, cmd_packed_enums, packed_param_types) self.context_local_call_protos.append(proto) self.context_local_call_functions.append(function) diff --git a/src/libANGLE/Context.cpp b/src/libANGLE/Context.cpp index 39b79a23b..5c901c30c 100644 --- a/src/libANGLE/Context.cpp +++ b/src/libANGLE/Context.cpp @@ -7268,16 +7268,6 @@ GLboolean Context::isBuffer(BufferID buffer) const return ConvertToGLBoolean(getBuffer(buffer)); } -GLboolean Context::isEnabled(GLenum cap) const -{ - return mState.getEnableFeature(cap); -} - -GLboolean Context::isEnabledi(GLenum target, GLuint index) const -{ - return mState.getEnableFeatureIndexed(target, index); -} - GLboolean Context::isFramebuffer(FramebufferID framebuffer) const { if (framebuffer.value == 0) diff --git a/src/libANGLE/Context_gles_2_0_autogen.h b/src/libANGLE/Context_gles_2_0_autogen.h index 7317d9939..e9372b9dd 100644 --- a/src/libANGLE/Context_gles_2_0_autogen.h +++ b/src/libANGLE/Context_gles_2_0_autogen.h @@ -96,7 +96,6 @@ void getVertexAttribfv(GLuint index, GLenum pname, GLfloat *params); \ void getVertexAttribiv(GLuint index, GLenum pname, GLint *params); \ GLboolean isBuffer(BufferID bufferPacked) const; \ - GLboolean isEnabled(GLenum cap) const; \ GLboolean isFramebuffer(FramebufferID framebufferPacked) const; \ GLboolean isProgram(ShaderProgramID programPacked) const; \ GLboolean isRenderbuffer(RenderbufferID renderbufferPacked) const; \ diff --git a/src/libANGLE/Context_gles_3_2_autogen.h b/src/libANGLE/Context_gles_3_2_autogen.h index 26afdfdbe..46645d804 100644 --- a/src/libANGLE/Context_gles_3_2_autogen.h +++ b/src/libANGLE/Context_gles_3_2_autogen.h @@ -50,7 +50,6 @@ GLsizei bufSize, GLint *params); \ void getnUniformuiv(ShaderProgramID programPacked, UniformLocation locationPacked, \ GLsizei bufSize, GLuint *params); \ - GLboolean isEnabledi(GLenum target, GLuint index) const; \ void objectLabel(GLenum identifier, GLuint name, GLsizei length, const GLchar *label); \ void objectPtrLabel(const void *ptr, GLsizei length, const GLchar *label); \ void patchParameteri(GLenum pname, GLint value); \ diff --git a/src/libANGLE/State.h b/src/libANGLE/State.h index 6f082c2a9..f835587c1 100644 --- a/src/libANGLE/State.h +++ b/src/libANGLE/State.h @@ -1403,6 +1403,7 @@ class State : angle::NonCopyable { return mLocalState.getEnableFeatureIndexed(feature, index); } + const LocalState &localState() const { return mLocalState; } const GLES1State &gles1() const { return mLocalState.gles1(); } // Used by the capture/replay tool to create state. diff --git a/src/libANGLE/context_local_call_gles.cpp b/src/libANGLE/context_local_call_gles.cpp index 789c6ef7e..a94ffb75f 100644 --- a/src/libANGLE/context_local_call_gles.cpp +++ b/src/libANGLE/context_local_call_gles.cpp @@ -571,4 +571,13 @@ void ContextLocalHint(Context *context, GLenum target, GLenum mode) } } +GLboolean ContextLocalIsEnabled(Context *context, GLenum cap) +{ + return context->getState().localState().getEnableFeature(cap); +} + +GLboolean ContextLocalIsEnabledi(Context *context, GLenum target, GLuint index) +{ + return context->getState().localState().getEnableFeatureIndexed(target, index); +} } // namespace gl diff --git a/src/libANGLE/context_local_call_gles_autogen.h b/src/libANGLE/context_local_call_gles_autogen.h index 792509739..efc418b3c 100644 --- a/src/libANGLE/context_local_call_gles_autogen.h +++ b/src/libANGLE/context_local_call_gles_autogen.h @@ -49,6 +49,7 @@ void ContextLocalDisable(Context *context, GLenum cap); void ContextLocalEnable(Context *context, GLenum cap); void ContextLocalFrontFace(Context *context, GLenum mode); void ContextLocalHint(Context *context, GLenum target, GLenum mode); +GLboolean ContextLocalIsEnabled(Context *context, GLenum cap); void ContextLocalLineWidth(Context *context, GLfloat width); void ContextLocalPixelStorei(Context *context, GLenum pname, GLint param); void ContextLocalPolygonOffset(Context *context, GLfloat factor, GLfloat units); @@ -117,6 +118,7 @@ void ContextLocalColorMaski(Context *context, GLboolean a); void ContextLocalDisablei(Context *context, GLenum target, GLuint index); void ContextLocalEnablei(Context *context, GLenum target, GLuint index); +GLboolean ContextLocalIsEnabledi(Context *context, GLenum target, GLuint index); void ContextLocalMinSampleShading(Context *context, GLfloat value); void ContextLocalPrimitiveBoundingBox(Context *context, GLfloat minX, diff --git a/src/libGLESv2/entry_points_gles_2_0_autogen.cpp b/src/libGLESv2/entry_points_gles_2_0_autogen.cpp index a7ac7c17d..9a1040b7b 100644 --- a/src/libGLESv2/entry_points_gles_2_0_autogen.cpp +++ b/src/libGLESv2/entry_points_gles_2_0_autogen.cpp @@ -2380,12 +2380,11 @@ GLboolean GL_APIENTRY GL_IsEnabled(GLenum cap) GLboolean returnValue; if (context) { - SCOPED_SHARE_CONTEXT_LOCK(context); bool isCallValid = (context->skipValidation() || ValidateIsEnabled(context, angle::EntryPoint::GLIsEnabled, cap)); if (isCallValid) { - returnValue = context->isEnabled(cap); + returnValue = ContextLocalIsEnabled(context, cap); } else { diff --git a/src/libGLESv2/entry_points_gles_3_2_autogen.cpp b/src/libGLESv2/entry_points_gles_3_2_autogen.cpp index d9159f9e8..39c9b9604 100644 --- a/src/libGLESv2/entry_points_gles_3_2_autogen.cpp +++ b/src/libGLESv2/entry_points_gles_3_2_autogen.cpp @@ -902,13 +902,12 @@ GLboolean GL_APIENTRY GL_IsEnabledi(GLenum target, GLuint index) GLboolean returnValue; if (context) { - SCOPED_SHARE_CONTEXT_LOCK(context); bool isCallValid = (context->skipValidation() || ValidateIsEnabledi(context, angle::EntryPoint::GLIsEnabledi, target, index)); if (isCallValid) { - returnValue = context->isEnabledi(target, index); + returnValue = ContextLocalIsEnabledi(context, target, index); } else { diff --git a/src/libGLESv2/entry_points_gles_ext_autogen.cpp b/src/libGLESv2/entry_points_gles_ext_autogen.cpp index 368b0f803..02cbdb5b0 100644 --- a/src/libGLESv2/entry_points_gles_ext_autogen.cpp +++ b/src/libGLESv2/entry_points_gles_ext_autogen.cpp @@ -6066,13 +6066,12 @@ GLboolean GL_APIENTRY GL_IsEnablediEXT(GLenum target, GLuint index) GLboolean returnValue; if (context) { - SCOPED_SHARE_CONTEXT_LOCK(context); bool isCallValid = (context->skipValidation() || ValidateIsEnablediEXT(context, angle::EntryPoint::GLIsEnablediEXT, target, index)); if (isCallValid) { - returnValue = context->isEnabledi(target, index); + returnValue = ContextLocalIsEnabledi(context, target, index); } else { @@ -10721,13 +10720,12 @@ GLboolean GL_APIENTRY GL_IsEnablediOES(GLenum target, GLuint index) GLboolean returnValue; if (context) { - SCOPED_SHARE_CONTEXT_LOCK(context); bool isCallValid = (context->skipValidation() || ValidateIsEnablediOES(context, angle::EntryPoint::GLIsEnablediOES, target, index)); if (isCallValid) { - returnValue = context->isEnabledi(target, index); + returnValue = ContextLocalIsEnabledi(context, target, index); } else {