Widen color state support on ES.

Indexed draw buffer (color state) support on ES is not core until 3.2.
Previously, we were failing (asserting) if color state was changed for
a non-zero attachment. This CL compares the state, and only asserts if the
color state actually differs per-attachment.

It also implements a disable_indexed_draw_buffers toggle, which allows a
test to check for the functionality in a platform-independent manner.

BUG=dawn:580, dawn:582

Change-Id: I11c67b0dd72f73e7302c06cad24e8a268fb37a76
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/34981
Commit-Queue: Stephen White <senorblanco@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/Toggles.cpp

@@ -120,6 +120,11 @@ namespace dawn_native {
               "Disables the use of non-zero base instance which is unsupported on some "
               "platforms.",
               "https://crbug.com/dawn/343"}},
+            {Toggle::DisableIndexedDrawBuffers,
+             {"disable_indexed_draw_buffers",
+              "Disables the use of indexed draw buffer state which is unsupported on some "
+              "platforms.",
+              "https://crbug.com/dawn/582"}},
             {Toggle::UseD3D12SmallShaderVisibleHeapForTesting,
              {"use_d3d12_small_shader_visible_heap",
               "Enable use of a small D3D12 shader visible heap, instead of using a large one by "

src/dawn_native/Toggles.h

@@ -39,6 +39,7 @@ namespace dawn_native {
         MetalUseSharedModeForCounterSampleBuffer,
         DisableBaseVertex,
         DisableBaseInstance,
+        DisableIndexedDrawBuffers,
         UseD3D12SmallShaderVisibleHeapForTesting,
         UseDXC,
         DisableRobustness,

src/dawn_native/opengl/DeviceGL.cpp

@@ -65,6 +65,9 @@ namespace dawn_native { namespace opengl {
 
         bool supportsBaseInstance = gl.IsAtLeastGLES(3, 2) || gl.IsAtLeastGL(4, 2);
 
+        // TODO(crbug.com/dawn/582): Use OES_draw_buffers_indexed where available.
+        bool supportsIndexedDrawBuffers = gl.IsAtLeastGLES(3, 2) || gl.IsAtLeastGL(3, 0);
+
         // TODO(crbug.com/dawn/343): We can support the extension variants, but need to load the EXT
         // procs without the extension suffix.
         // We'll also need emulation of shader builtins gl_BaseVertex and gl_BaseInstance.
@@ -82,6 +85,7 @@ namespace dawn_native { namespace opengl {
         // TODO(crbug.com/dawn/343): Investigate emulation.
         SetToggle(Toggle::DisableBaseVertex, !supportsBaseVertex);
         SetToggle(Toggle::DisableBaseInstance, !supportsBaseInstance);
+        SetToggle(Toggle::DisableIndexedDrawBuffers, !supportsIndexedDrawBuffers);
     }
 
     const GLFormat& Device::GetGLFormat(const Format& format) {

src/dawn_native/opengl/RenderPipelineGL.cpp

@@ -106,44 +106,46 @@ namespace dawn_native { namespace opengl {
                              ColorAttachmentIndex attachment,
                              const ColorStateDescriptor* descriptor) {
             GLuint colorBuffer = static_cast<GLuint>(static_cast<uint8_t>(attachment));
-            if (gl.IsAtLeastGL(3, 0) || gl.IsAtLeastGLES(3, 2)) {
+            if (BlendEnabled(descriptor)) {
-                if (BlendEnabled(descriptor)) {
+                gl.Enablei(GL_BLEND, colorBuffer);
-                    gl.Enablei(GL_BLEND, colorBuffer);
+                gl.BlendEquationSeparatei(colorBuffer,
-                    gl.BlendEquationSeparatei(colorBuffer,
+                                          GLBlendMode(descriptor->colorBlend.operation),
-                                              GLBlendMode(descriptor->colorBlend.operation),
+                                          GLBlendMode(descriptor->alphaBlend.operation));
-                                              GLBlendMode(descriptor->alphaBlend.operation));
+                gl.BlendFuncSeparatei(colorBuffer,
-                    gl.BlendFuncSeparatei(colorBuffer,
+                                      GLBlendFactor(descriptor->colorBlend.srcFactor, false),
-                                          GLBlendFactor(descriptor->colorBlend.srcFactor, false),
+                                      GLBlendFactor(descriptor->colorBlend.dstFactor, false),
-                                          GLBlendFactor(descriptor->colorBlend.dstFactor, false),
+                                      GLBlendFactor(descriptor->alphaBlend.srcFactor, true),
-                                          GLBlendFactor(descriptor->alphaBlend.srcFactor, true),
+                                      GLBlendFactor(descriptor->alphaBlend.dstFactor, true));
-                                          GLBlendFactor(descriptor->alphaBlend.dstFactor, true));
-                } else {
-                    gl.Disablei(GL_BLEND, colorBuffer);
-                }
-                gl.ColorMaski(colorBuffer, descriptor->writeMask & wgpu::ColorWriteMask::Red,
-                              descriptor->writeMask & wgpu::ColorWriteMask::Green,
-                              descriptor->writeMask & wgpu::ColorWriteMask::Blue,
-                              descriptor->writeMask & wgpu::ColorWriteMask::Alpha);
             } else {
-                // TODO(crbug.com/dawn/582): Add validation to prevent this as it is not supported
+                gl.Disablei(GL_BLEND, colorBuffer);
-                // on GLES < 3.2.
-                DAWN_ASSERT(colorBuffer == 0);
-                if (BlendEnabled(descriptor)) {
-                    gl.Enable(GL_BLEND);
-                    gl.BlendEquationSeparate(GLBlendMode(descriptor->colorBlend.operation),
-                                             GLBlendMode(descriptor->alphaBlend.operation));
-                    gl.BlendFuncSeparate(GLBlendFactor(descriptor->colorBlend.srcFactor, false),
-                                         GLBlendFactor(descriptor->colorBlend.dstFactor, false),
-                                         GLBlendFactor(descriptor->alphaBlend.srcFactor, true),
-                                         GLBlendFactor(descriptor->alphaBlend.dstFactor, true));
-                } else {
-                    gl.Disable(GL_BLEND);
-                }
-                gl.ColorMask(descriptor->writeMask & wgpu::ColorWriteMask::Red,
-                             descriptor->writeMask & wgpu::ColorWriteMask::Green,
-                             descriptor->writeMask & wgpu::ColorWriteMask::Blue,
-                             descriptor->writeMask & wgpu::ColorWriteMask::Alpha);
             }
+            gl.ColorMaski(colorBuffer, descriptor->writeMask & wgpu::ColorWriteMask::Red,
+                          descriptor->writeMask & wgpu::ColorWriteMask::Green,
+                          descriptor->writeMask & wgpu::ColorWriteMask::Blue,
+                          descriptor->writeMask & wgpu::ColorWriteMask::Alpha);
+        }
+
+        void ApplyColorState(const OpenGLFunctions& gl, const ColorStateDescriptor* descriptor) {
+            if (BlendEnabled(descriptor)) {
+                gl.Enable(GL_BLEND);
+                gl.BlendEquationSeparate(GLBlendMode(descriptor->colorBlend.operation),
+                                         GLBlendMode(descriptor->alphaBlend.operation));
+                gl.BlendFuncSeparate(GLBlendFactor(descriptor->colorBlend.srcFactor, false),
+                                     GLBlendFactor(descriptor->colorBlend.dstFactor, false),
+                                     GLBlendFactor(descriptor->alphaBlend.srcFactor, true),
+                                     GLBlendFactor(descriptor->alphaBlend.dstFactor, true));
+            } else {
+                gl.Disable(GL_BLEND);
+            }
+            gl.ColorMask(descriptor->writeMask & wgpu::ColorWriteMask::Red,
+                         descriptor->writeMask & wgpu::ColorWriteMask::Green,
+                         descriptor->writeMask & wgpu::ColorWriteMask::Blue,
+                         descriptor->writeMask & wgpu::ColorWriteMask::Alpha);
+        }
+
+        bool Equal(const BlendDescriptor& lhs, const BlendDescriptor& rhs) {
+            return lhs.operation == rhs.operation && lhs.srcFactor == rhs.srcFactor &&
+                   lhs.dstFactor == rhs.dstFactor;
         }
 
         GLuint OpenGLStencilOperation(wgpu::StencilOperation stencilOperation) {
@@ -298,8 +300,25 @@ namespace dawn_native { namespace opengl {
             gl.Disable(GL_POLYGON_OFFSET_FILL);
         }
 
-        for (ColorAttachmentIndex attachmentSlot : IterateBitSet(GetColorAttachmentsMask())) {
+        if (!GetDevice()->IsToggleEnabled(Toggle::DisableIndexedDrawBuffers)) {
-            ApplyColorState(gl, attachmentSlot, GetColorStateDescriptor(attachmentSlot));
+            for (ColorAttachmentIndex attachmentSlot : IterateBitSet(GetColorAttachmentsMask())) {
+                ApplyColorState(gl, attachmentSlot, GetColorStateDescriptor(attachmentSlot));
+            }
+        } else {
+            const ColorStateDescriptor* prevDescriptor = nullptr;
+            for (ColorAttachmentIndex attachmentSlot : IterateBitSet(GetColorAttachmentsMask())) {
+                const ColorStateDescriptor* descriptor = GetColorStateDescriptor(attachmentSlot);
+                if (!prevDescriptor) {
+                    ApplyColorState(gl, descriptor);
+                    prevDescriptor = descriptor;
+                } else if (!Equal(descriptor->alphaBlend, prevDescriptor->alphaBlend) ||
+                           !Equal(descriptor->colorBlend, prevDescriptor->colorBlend) ||
+                           descriptor->writeMask != prevDescriptor->writeMask) {
+                    // TODO(crbug.com/dawn/582): Add validation to prevent this as it is not
+                    // supported on GLES < 3.2.
+                    ASSERT(false);
+                }
+            }
         }
     }
 

src/tests/end2end/ColorStateTests.cpp

@@ -747,6 +747,8 @@ TEST_P(ColorStateTest, ColorWriteMaskBlendingDisabled) {
 TEST_P(ColorStateTest, IndependentColorState) {
     DAWN_SKIP_TEST_IF(IsWindows() && IsVulkan() && IsIntel());
 
+    DAWN_SKIP_TEST_IF(HasToggleEnabled("disable_indexed_draw_buffers"));
+
     std::array<wgpu::Texture, 4> renderTargets;
     std::array<wgpu::TextureView, 4> renderTargetViews;