Implement support for mixed sampled render targets

include/gpu/GrRenderTarget.h

Index: include/gpu/GrRenderTarget.h
diff --git a/include/gpu/GrRenderTarget.h b/include/gpu/GrRenderTarget.h
index 40e006aa8aec8edb70b28cec5a3274f5b7f0e110..933e0923b5057b2d99feea641bd3be705507f2d1 100644
--- a/include/gpu/GrRenderTarget.h
+++ b/include/gpu/GrRenderTarget.h
@@ -31,14 +31,55 @@ public:
 
     // GrRenderTarget
     /**
-     * @return true if the surface is multisampled, false otherwise
+     * On some hardware it is possible for a render target to have multisampling
+     * only in certain stages of the graphics pipeline. This enum allows
+     * the caller to query about specific stages in isMultisampled() and
+     * numSamples().
      */
-    bool isMultisampled() const { return 0 != fDesc.fSampleCnt; }
+    enum PipelineStage {

[bsalomon, 2015/03/31 16:37:33]

I'm not sure about this name... 

enum BufferType {
kColor_BufferType,
kStencil_BufferType
};

?

Also, should it be a bitfield?

uint32_t buffersWithMSAA() const { return fMSAABufferBitfield; }

/** Param must only have one flag bit set */
int sampleCount(BufferType buffer) const {
   if (this->buffersWithMSAA() & buffer) {
       return fDesc.fSampleCnt;
   }
   return 0;
}

something like that?

[Chris Dalton, 2015/03/31 19:47:30]

I'm going to try replying inline. Hopefully it works ok.

On 2015/03/31 16:37:33, bsalomon wrote:
One thing I should clarify is that this enum by design was not related to the
internal data structure of the RT. It was just intended as a tool for client
code to query about the multisampling.

The motivation was that, while internally our msaa was defined by GrSampleConfig
and a sample count, it was cleaner for the client code to make queries that
looked like the actual information it wanted to know:

  rt->isMultisampled(ColorBuffer);
  rt->isMultisampled(Anywhere);
  rt->numSamples(StencilBuffer);
  rt->isMultisampled(Rasterizer);  // Rasterizer for the future

I recommended the name "PipelineStage" for Seva because in the future we will
want to query about msaa in more than just buffers (e.g. the rasterizer), but I
think I've changed my mind. It would probably be best to limit it to buffers for
now like you suggest. We can worry about the other pipeline stages when we need
to.
For the internal data structure, we intentionally tried to avoid something that
could allow for configurations that aren't supported (e.g. msaa in color but not
stencil), which is why we went the GrSampleConfig route.

A bitfield is a good idea though. Going off your suggestion, what about
something along these lines?

 enum BufferBit {
     kColor_BufferBit = 1,
     kStencil_BufferBit = (1 << 1)
 };
 typedef uint32_t BufferBits;

 enum SampleConfig {
     kUnified_SampleConfig = kColor_BufferBit + kStencil_BufferBit,
     kStencil_SampleConfig = kStencil_BufferBit
 };

 // Constructor accepts a SampleConfing...

 SampleConfig fSampleConfig; // Only Unified or Stencil configs are allowed.

 enum {
     kAny_BufferBits = 0,
     kEvery_BufferBit = kColor_BufferBit + kSample_BufferBit
 };

 /* Returns true if we are multisampled in all the buffers, blah blah blah */
 bool isMultisampled(BufferBits buffers) {
     if ((fSampleConfig & buffers) != buffers) {
         return false;
     }
     return 0 != fDesc.fSampleCnt;
 }

 // numSamples stays the same as in the patch

Obviously not polished, but this way we could get a cool bitfield while still
enforcing only two legal sample configs in the data structure (and more
importantly, the constructor.)

[Chris Dalton, 2015/03/31 22:24:57]

Sorry to pollute your inboxes :) I've thought about it more and I think I really
like this hybrid bitfield approach!

 enum BufferBits {
     kAny_BufferBits = 0,
     kColor_BufferBit = 1,
     kStencil_BufferBit = (1 << 1),
     kAll_BufferBits = kColor_BufferBit + kStencil_BufferBit
 };

 /* Enorce only two legal configs, blah blah blah */
 enum SampleConfig {
     kUnified_SampleConfig = kColor_BufferBit + kStencil_BufferBit,
     kStencil_SampleConfig = kStencil_BufferBit
 };

 /*Returns true if fSampleCnt > 0 and all the buffers are msaa, blah blah blah*/
 bool isMultisampled(BufferBits buffers = kAll_BufferBits) {
     if (buffers != (fSampleConfig & buffers)) {
         // One of the buffers is not in our msaa config.
         return false;
     }
     return 0 != fDesc.fSampleCnt;
 }

 ...

 SampleConfig fSampleConfig;
};

GR_MAKE_BITFIELD_OPS(GrRenderTarget::BufferBits)

Seva, Brian, thoughts?

[vbuzinov, 2015/04/02 13:10:37]

This bitfield implementation is certainly easier to read and understand, so I am
all for it.

+        kColorBuffer_PipelineStage,
+        kStencilBuffer_PipelineStage,
+        // We only support two sample configs at the moment, unified and
+        // stencil. So we can query multisample info for everywhere/anywhere
+        // by checking the correct buffer.
+        kEverywhere_PipelineStage = kColorBuffer_PipelineStage,
+        kAnywhere_PipelineStage = kStencilBuffer_PipelineStage
+    };
+
+    /**
+     * @param  pipelineStage  specifies the pipeline stage for which
+     *                        multisampling is queried
+     * @return true if the surface is multisampled in the specified pipeline
+     *         stage(s), false otherwise
+     */
+    bool isMultisampled(PipelineStage pipelineStage =
+                                      kEverywhere_PipelineStage) const {
+        if (kStencil_GrSampleConfig == fSampleConfig &&
+            kColorBuffer_PipelineStage == pipelineStage) {
+            return false;
+        }
+        return 0 != fDesc.fSampleCnt;
+     }
+
+    /**
+     * @param  pipelineStage  specifies the pipeline stage for which
+     *                        multisampling is queried
+     * @return the number of samples-per-pixel, or zero if the render target
+     *         is not multisampled in the specified pipeline stage(s)
+     */
+    int numSamples(PipelineStage pipelineStage =
+                                 kEverywhere_PipelineStage) const {
+        return this->isMultisampled(pipelineStage) ? fDesc.fSampleCnt : 0;
+    }
 
     /**
-     * @return the number of samples-per-pixel or zero if non-MSAA.
+     * @return sample config: kUnified_GrSampleConfig if both color and stencil
+               buffers are MSAA, or kStencil_GrSampleConfig if only stencil
+               buffer is MSAA.
      */
-    int numSamples() const { return fDesc.fSampleCnt; }
+    GrSampleConfig sampleConfig() const {
+        return fSampleConfig;
+    }
 
     /**
      * Call to indicate the multisample contents were modified such that the
@@ -94,8 +135,10 @@ public:
     const GrRenderTargetPriv renderTargetPriv() const;
 
 protected:
-    GrRenderTarget(GrGpu* gpu, LifeCycle lifeCycle, const GrSurfaceDesc& desc)
+    GrRenderTarget(GrGpu* gpu, LifeCycle lifeCycle, const GrSurfaceDesc& desc,
+                   GrSampleConfig sampleConfig)
         : INHERITED(gpu, lifeCycle, desc)
+        , fSampleConfig(sampleConfig)
         , fStencilBuffer(NULL) {
         fResolveRect.setLargestInverted();
     }
@@ -110,6 +153,8 @@ private:
 
     friend class GrRenderTargetPriv;
 
+    GrSampleConfig    fSampleConfig;
+
     GrStencilBuffer*  fStencilBuffer;
 
     SkIRect           fResolveRect;