Enable stencil clipping in mixed sampled render targets

src/gpu/gl/GrGLGpu.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 
 #include "GrGLGpu.h"
 #include "GrGLGLSL.h"
@@ skipping 139 matching lines @@
 
     GR_STATIC_ASSERT(14 == kS2C_GrBlendCoeff);
     GR_STATIC_ASSERT(15 == kIS2C_GrBlendCoeff);
     GR_STATIC_ASSERT(16 == kS2A_GrBlendCoeff);
     GR_STATIC_ASSERT(17 == kIS2A_GrBlendCoeff);
 
     // assertion for gXfermodeCoeff2Blend have to be in GrGpu scope
     GR_STATIC_ASSERT(kGrBlendCoeffCnt == SK_ARRAY_COUNT(gXfermodeCoeff2Blend));
 }
 
+// Sample locations that are colocated at pixel center

[Mark Kilgard, 2015/09/21 17:03:50]

for the sake of documentation, I recommend 2*16 instead of 32

will you need 0.5f to quite warnings on any platform Skia cares about?

+static const GrGLfloat gCenteredSampleLocations[32] = { 0.5, 0.5, 0.5, 0.5, 0.5, 0.5, 0.5, 0.5,
+                                                        0.5, 0.5, 0.5, 0.5, 0.5, 0.5, 0.5, 0.5,
+                                                        0.5, 0.5, 0.5, 0.5, 0.5, 0.5, 0.5, 0.5,
+                                                        0.5, 0.5, 0.5, 0.5, 0.5, 0.5, 0.5, 0.5 };
+
 ///////////////////////////////////////////////////////////////////////////////
 
 // Used in the map of pixel configs to stencil format indices. This value is used to
 // indicate that a stencil format has not yet been set for the given config.
 static const int kUnknownStencilIndex = -1;
 // This value is used as the stencil index when no stencil configs are supported with the
 // given pixel config.
 static const int kUnsupportedStencilIndex = -2;
 
 ///////////////////////////////////////////////////////////////////////////////
@@ skipping 1723 matching lines @@
             if (!flipY) {
                 dst += rowBytes;
             } else {
                 dst -= rowBytes;
             }
         }
     }
     return true;
 }
 
+void GrGLGpu::setProgrammableSampleLocations(GrRenderTarget* rt, bool useSampleLocations) {
+    if (0 == rt->numRasterSamples())
+        return;

[Chris Dalton, 2015/09/22 08:35:24]

Skia style uses braces for one line if's.

+
+    GrGLRenderTarget* target = static_cast<GrGLRenderTarget*>(rt->asRenderTarget());
+    SkASSERT(0 != target->renderFBOID());
+
+    if (useSampleLocations == target->usesProgrammableSampleLocations())
+        return;
+
+    if (GrGLRenderTarget::kProgrammable_SampleLocationsConfig != target->sampleLocationsConfig()) {
+        uint32_t rtID = target->getUniqueID();

[Mark Kilgard, 2015/09/21 17:03:51]

is it worth considering using glNamedFramebufferParameteri (OpenGL 4.5) here to
eliminate the need for the glBindFramebuffer?

This would create a dependency on OpenGL 4.5 (or EXT_direct_state_access)...

Perhaps we are already bound to the framebuffer we need coming through here?

+        if (fHWBoundRenderTargetUniqueID != rtID) {
+            fStats.incRenderTargetBinds();
+            GL_CALL(BindFramebuffer(GR_GL_FRAMEBUFFER, target->renderFBOID()));
+            fHWBoundRenderTargetUniqueID = SK_InvalidUniqueID;
+        }
+
+        GL_CALL(FramebufferParameteri(GR_GL_FRAMEBUFFER, GR_GL_FRAMEBUFFER_PROGRAMMABLE_SAMPLE_LOCATIONS, true));
+        target->setSampleLocationsConfig(GrGLRenderTarget::kProgrammable_SampleLocationsConfig);
+
+        float defaultSampleLocations[32];

[Mark Kilgard, 2015/09/21 17:03:50]

worth adding

assert(rt->numRasterSamples()*2 == SK_ARRAY_COUNT(gCenteredSampleLocations));
assert(rt->numRasterSamples()*2 == SK_ARRAY_COUNT(defaultSampleLocations));

+        for (int i = 0; i < rt->numRasterSamples(); i++) {
+            GL_CALL(GetMultisamplefv(GR_GL_SAMPLE_LOCATION_NV, i, &defaultSampleLocations[i*2]));

[Mark Kilgard, 2015/09/21 17:03:51]

calling a glGetMultisamplefv here many times (as many as 16 times) is rather
expensive

it would make more sense to stash the default sample locations once (per
GrContext?) to avoid querying them.  This is particularly true given the command
buffer usage in Chrome.

Also, glGetMultisamplefv depends on the currently bound drawable FBO.  So you'd
need the glBindFramebuffer command above for this query to work, and hence
couldn't use the DSA version of glFramebufferParameteri.  Another reasons to
stash the default locations once per context.

Alternatively, you could just hard code the sample positions so they don't have
to be queried at all.  Microsoft specified the "default" sample positions for
1x, 2x, 4x, 8x, and 16x antialiasing.  The Microsoft locations are "good" in the
sense that the samples are well distributed for horizontal and vertical edges.

+        }
+        target->setDefaultSampleLocations(rt->numRasterSamples(), defaultSampleLocations);

[Chris Dalton, 2015/09/22 08:35:24]

We ought to set this up so it only queries the default sample locations once.
From the look of things, it will make this query every time we enable centered
sample locations, which would probably impact perf.

Also, do we need to remember the default locations? Or is it good enough to just
disable GL_FRAMEBUFFER_PROGRAMMABLE_SAMPLE_LOCATIONS?

+    }
+
+    GL_CALL(NamedFramebufferSampleLocationsfv(target->renderFBOID(), 0, rt->numRasterSamples(),
+                                              useSampleLocations ? gCenteredSampleLocations
+                                                                 : target->defaultSampleLocations()));
+    target->flagAsUsingProgrammableSampleLocations(useSampleLocations);
+}
+
 void GrGLGpu::flushRenderTarget(GrGLRenderTarget* target, const SkIRect* bound) {
 
     SkASSERT(target);
 
     uint32_t rtID = target->getUniqueID();
     if (fHWBoundRenderTargetUniqueID != rtID) {
         fStats.incRenderTargetBinds();
         GL_CALL(BindFramebuffer(GR_GL_FRAMEBUFFER, target->renderFBOID()));
 #ifdef SK_DEBUG
         // don't do this check in Chromium -- this is causing
@@ skipping 230 matching lines @@
                                GrStencilSettings::kFront_Face);
             }
         }
         fHWStencilSettings = stencilSettings;
     }
 }
 
 void GrGLGpu::flushHWAAState(GrRenderTarget* rt, bool useHWAA) {
     SkASSERT(!useHWAA || rt->isStencilBufferMultisampled());
 
+    if (rt->hasMixedSamples()) {
+        if (useHWAA) {
+            setProgrammableSampleLocations(rt, false);
+        } else {
+            setProgrammableSampleLocations(rt, true);
+        }
+        useHWAA = true;

[Chris Dalton, 2015/09/22 08:35:24]

I still think it's desirable to disable multisample rather than co-locating
sample positions if we don't need hwaa enabled for the stencil test. Can we find
a way to only co-locate the sample locations when we know there is a stencil
test?

+    }
+
     if (this->glCaps().multisampleDisableSupport()) {
         if (useHWAA) {
             if (kYes_TriState != fMSAAEnabled) {
                 GL_CALL(Enable(GR_GL_MULTISAMPLE));
                 fMSAAEnabled = kYes_TriState;
             }
         } else {
             if (kNo_TriState != fMSAAEnabled) {
                 GL_CALL(Disable(GR_GL_MULTISAMPLE));
                 fMSAAEnabled = kNo_TriState;
@@ skipping 997 matching lines @@
             this->setVertexArrayID(gpu, 0);
         }
         int attrCount = gpu->glCaps().maxVertexAttributes();
         if (fDefaultVertexArrayAttribState.count() != attrCount) {
             fDefaultVertexArrayAttribState.resize(attrCount);
         }
         attribState = &fDefaultVertexArrayAttribState;
     }
     return attribState;
 }