Block framebuffer queries on overridden PLS state

Generate an error from glGetFramebufferAttachmentParameteriv() when
querying a color attachment that has been overridden by PLS.

Bug: angleproject:40096838
Change-Id: I83e68c3527f034f9a24822cf4f57789f81b9b6af
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6338390
Reviewed-by: Kenneth Russell <[email protected]>
Reviewed-by: Alexey Knyazev <[email protected]>
Commit-Queue: Chris Dalton <[email protected]>
Reviewed-by: Shahbaz Youssefi <[email protected]>

Author: csmartdalton

extensions/ANGLE_shader_pixel_local_storage.txt

@@ -558,6 +558,18 @@ Additions to the OpenGL ES Specification, Version 3.0.6
             (MAX_COMBINED_DRAW_BUFFERS_AND_PIXEL_LOCAL_STORAGE_PLANES_ANGLE -
              ACTIVE_PIXEL_LOCAL_STORAGE_PLANES_ANGLE)
 
+      * INVALID_OPERATION is generated by calls to
+        GetFramebufferAttachmentParameteriv() if <attachment> references a COLOR
+        attachment and any of the following are true:
+
+        <attachment> >=
+            COLOR_ATTACHMENT0 +
+            MAX_COLOR_ATTACHMENTS_WITH_ACTIVE_PIXEL_LOCAL_STORAGE_ANGLE
+        <attachment> >=
+            COLOR_ATTACHMENT0 +
+            (MAX_COMBINED_DRAW_BUFFERS_AND_PIXEL_LOCAL_STORAGE_PLANES_ANGLE -
+             ACTIVE_PIXEL_LOCAL_STORAGE_PLANES_ANGLE)
+
       * Calls to EnableiOES(BLEND, <i>) and ColorMaskiOES(<i>, ...) are ignored
         if any of the following are true:
 

src/libANGLE/ErrorStrings.h

@@ -507,6 +507,7 @@ inline constexpr const char *kPixelDataNull = "Pixel data cannot be null.";
 inline constexpr const char *kPixelPackBufferBoundForTransformFeedback = "It is undefined behavior to use a pixel pack buffer that is bound for transform feedback.";
 inline constexpr const char *kPixelUnpackBufferBoundForTransformFeedback = "It is undefined behavior to use a pixel unpack buffer that is bound for transform feedback.";
 inline constexpr const char *kPLSActive = "Operation not permitted while pixel local storage is active.";
+inline constexpr const char *kPLSColorAttachmentReserved = "Color attachment is actively reserved for pixel local storage.";
 inline constexpr const char *kPLSCapNotAllowed = "Cap 0x%04X cannot be enabled or disabled while pixel local storage is active.";
 inline constexpr const char *kPLSDefaultFramebufferBound = "Default framebuffer object name 0 does not support pixel local storage.";
 inline constexpr const char *kPLSDimensionsDontMatchRenderingArea = "Pixel local storage backing texture dimensions not equal to the rendering area.";

src/libANGLE/validationES.cpp

@@ -5763,7 +5763,6 @@ bool ValidateGetFramebufferAttachmentParameterivBase(const Context *context,
             }
             break;
 
-        case GL_COLOR_ATTACHMENT0:
         case GL_DEPTH_ATTACHMENT:
         case GL_STENCIL_ATTACHMENT:
             break;
@@ -5777,6 +5776,15 @@ bool ValidateGetFramebufferAttachmentParameterivBase(const Context *context,
                 ANGLE_VALIDATION_ERROR(GL_INVALID_ENUM, kInvalidAttachment);
                 return false;
             }
+            [[fallthrough]];
+
+        case GL_COLOR_ATTACHMENT0:
+            if (context->getPrivateState().isActivelyOverriddenPLSDrawBuffer(
+                    attachment - GL_COLOR_ATTACHMENT0_EXT))
+            {
+                ANGLE_VALIDATION_ERROR(GL_INVALID_OPERATION, kPLSColorAttachmentReserved);
+                return false;
+            }
             break;
     }
 

src/tests/gl_tests/PixelLocalStorageTest.cpp

@@ -6204,6 +6204,90 @@ TEST_P(PixelLocalStorageValidationTest, BlendFuncDuringPLS)
     }
 }
 
+// Check that glGetFramebufferAttachmentParameteriv() generates errors when
+// querying overridden PLS attachments.
+TEST_P(PixelLocalStorageValidationTest, FramebufferQueries)
+{
+    ANGLE_SKIP_TEST_IF(!EnsureGLExtensionEnabled("GL_ANGLE_shader_pixel_local_storage"));
+
+    PLSTestTexture pls0(GL_RGBA8);
+    PLSTestTexture pls1(GL_RGBA8);
+    GLFramebuffer fbo;
+    glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+    glFramebufferTexturePixelLocalStorageANGLE(0, pls0, 0, 0);
+    glFramebufferTexturePixelLocalStorageANGLE(1, pls1, 0, 0);
+
+    glBeginPixelLocalStorageANGLE(2, GLenumArray({GL_DONT_CARE, GL_DONT_CARE}));
+    EXPECT_GL_NO_ERROR();
+
+    int firstOverriddenDrawBuffer =
+        std::min(MAX_COLOR_ATTACHMENTS_WITH_ACTIVE_PIXEL_LOCAL_STORAGE,
+                 MAX_COMBINED_DRAW_BUFFERS_AND_PIXEL_LOCAL_STORAGE_PLANES - 2);
+    GLint intValue;
+
+    glGetFramebufferAttachmentParameteriv(GL_FRAMEBUFFER,
+                                          GL_COLOR_ATTACHMENT0 + firstOverriddenDrawBuffer,
+                                          GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME, &intValue);
+    EXPECT_GL_SINGLE_ERROR(GL_INVALID_OPERATION);
+    EXPECT_GL_SINGLE_ERROR_MSG("Color attachment is actively reserved for pixel local storage.");
+
+    glGetFramebufferAttachmentParameteriv(
+        GL_FRAMEBUFFER,
+        GL_COLOR_ATTACHMENT0 + MAX_COMBINED_DRAW_BUFFERS_AND_PIXEL_LOCAL_STORAGE_PLANES - 2,
+        GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME, &intValue);
+    EXPECT_GL_SINGLE_ERROR(GL_INVALID_OPERATION);
+    EXPECT_GL_SINGLE_ERROR_MSG("Color attachment is actively reserved for pixel local storage.");
+
+    glGetFramebufferAttachmentParameteriv(
+        GL_FRAMEBUFFER,
+        GL_COLOR_ATTACHMENT0 + MAX_COMBINED_DRAW_BUFFERS_AND_PIXEL_LOCAL_STORAGE_PLANES - 1,
+        GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME, &intValue);
+    EXPECT_GL_SINGLE_ERROR(GL_INVALID_OPERATION);
+    EXPECT_GL_SINGLE_ERROR_MSG("Color attachment is actively reserved for pixel local storage.");
+
+    glGetFramebufferAttachmentParameteriv(GL_FRAMEBUFFER,
+                                          GL_COLOR_ATTACHMENT0 + MAX_COLOR_ATTACHMENTS - 1,
+                                          GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME, &intValue);
+    EXPECT_GL_SINGLE_ERROR(GL_INVALID_OPERATION);
+    EXPECT_GL_SINGLE_ERROR_MSG("Color attachment is actively reserved for pixel local storage.");
+
+    if (firstOverriddenDrawBuffer != 0)
+    {
+        glGetFramebufferAttachmentParameteriv(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
+                                              GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME, &intValue);
+        EXPECT_GL_NO_ERROR();
+        glGetFramebufferAttachmentParameteriv(GL_FRAMEBUFFER,
+                                              GL_COLOR_ATTACHMENT0 + firstOverriddenDrawBuffer - 1,
+                                              GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME, &intValue);
+        EXPECT_GL_NO_ERROR();
+    }
+
+    glEndPixelLocalStorageANGLE(2, GLenumArray({GL_DONT_CARE, GL_DONT_CARE}));
+    EXPECT_GL_NO_ERROR();
+
+    glGetFramebufferAttachmentParameteriv(GL_FRAMEBUFFER,
+                                          GL_COLOR_ATTACHMENT0 + firstOverriddenDrawBuffer,
+                                          GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME, &intValue);
+    EXPECT_GL_NO_ERROR();
+
+    glGetFramebufferAttachmentParameteriv(
+        GL_FRAMEBUFFER,
+        GL_COLOR_ATTACHMENT0 + MAX_COMBINED_DRAW_BUFFERS_AND_PIXEL_LOCAL_STORAGE_PLANES - 2,
+        GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME, &intValue);
+    EXPECT_GL_NO_ERROR();
+
+    glGetFramebufferAttachmentParameteriv(
+        GL_FRAMEBUFFER,
+        GL_COLOR_ATTACHMENT0 + MAX_COMBINED_DRAW_BUFFERS_AND_PIXEL_LOCAL_STORAGE_PLANES - 1,
+        GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME, &intValue);
+    EXPECT_GL_NO_ERROR();
+
+    glGetFramebufferAttachmentParameteriv(GL_FRAMEBUFFER,
+                                          GL_COLOR_ATTACHMENT0 + MAX_COLOR_ATTACHMENTS - 1,
+                                          GL_FRAMEBUFFER_ATTACHMENT_OBJECT_NAME, &intValue);
+    EXPECT_GL_NO_ERROR();
+}
+
 // Check that glEnable(GL_BLEND) and glColorMask() do not generate errors, and are ignored for
 // overridden draw buffers during PLS.
 TEST_P(PixelLocalStorageValidationTest, BlendMaskDuringPLS)