Implement instanced rendering for simple shapes

src/gpu/GrInstancedRenderingTypes.h

+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef GrInstancedRenderingTypes_DEFINED
+#define GrInstancedRenderingTypes_DEFINED
+
+#include "GrTypes.h"
+#include "SkRRect.h"
+
+/**
+ * Internal structs and enums for classes involved in instanced rendering. This class is intended
+ * to be inherited non-publicly.
+ */
+class GrInstancedRenderingTypes {

[bsalomon, 2016/04/25 13:22:39]

Why this pattern of inheriting from this class which contains no interface? Why
aren't these just defined in GrInstancedRendering?

[Chris Dalton, 2016/04/25 17:01:19]

Mainly because then GrInstanceProcessor.cpp gets littered with
"GrInstancedRendering::" all over, or if we put them in GrInstanceProcessor,
GrInstanceRendering gets littered with "GrInstanceProcessor::" everywhere :)

But also, I don't see these types as specifically belonging to one class or the
other. They're really just part of the instanced rendering subsystem. Maybe a
namespace would be more appropriate but I don't see much precedent for that in
Skia.

[bsalomon, 2016/04/27 14:10:32]

I'd prefer a namespace and using decls.

What do you think about organizing all the instanced classes into their own
subdir, say src/gpu/instanced or something like that?

[csmartdalton, 2016/05/20 01:09:45]

sgtm

+public:
+    /**
+     * Per-vertex data. These values get fed into normal vertex attribs.
+     */
+    struct ShapeVertex {
+        float     fX, fY;  //!< Shape coordinates.
+        int32_t   fAttrs;  //!< Shape-specific vertex attributes, if needed.
+    };
+
+    /**
+     * Per-instance data. These values get fed into instanced vertex attribs.
+     */
+    struct Instance {
+        uint32_t   fInfo;               //!< Packed info about the instance. See InfoBits.
+        float      fShapeMatrix2x3[6];  //!< Maps canonical shape coords -> device space coords.
+        uint32_t   fColor;              //!< Color to be written out by the primitive processor.
+        float      fLocalRect[4];       //!< Local coords rect that spans [-1, +1] in shape coords.
+    };
+
+    /**
+     * Additional parameters required by some instances (e.g. round rect radii, perspective column,
+     * local matrix). These are accessed via texel buffer.
+     */
+    struct ParamsTexel {
+        union {
+            struct { float fX, fY, fZ, fW; };
+            float fValues[4];
+        };
+    };
+
+    GR_STATIC_ASSERT(0 == offsetof(ParamsTexel, fX));
+    GR_STATIC_ASSERT(4 * 4 == sizeof(ParamsTexel));
+
+    enum AttribIdx {
+        kShapeCoords_AttribIdx,
+        kVertexAttrs_AttribIdx,
+        kInstanceInfo_AttribIdx,
+        kShapeMatrixX_AttribIdx,
+        kShapeMatrixY_AttribIdx,
+        kColor_AttribIdx,
+        kLocalRect_AttribIdx,
+
+        kNumVertexAttribs
+    };
+
+    enum ShapeType {

[bsalomon, 2016/04/25 13:22:39]

I think use of the word "shape" may conflict with some work that is in progress
which will encapsulate rrect as well as non-rrect geometries using the word
shape. Could we call these something else? GeometryType? DrawType? RRectType?

Or can this just use the similar enum already defined in SkRRect?

[Chris Dalton, 2016/04/25 17:01:19]

SkRRect also has kEmpty, which I would prefer to not include in this enum.

Also, my vision for these classes is that they're a framework for drawing shapes
using instanced rendering (see the comment in GrInstancedRendering.h). It does
turn out that the shapes supported in v1.0 can all be generalized as round
rects, but if I get my way it won't stay like that forever.

Is it really a problem for a class to use the word "shape" internally?

[bsalomon, 2016/04/27 14:10:32]

I guess it is ok.

+        kRect_ShapeType,
+        kOval_ShapeType,
+        kSimpleRRect_ShapeType,
+        kNinePatch_ShapeType,
+        kComplexRRect_ShapeType,
+
+        kLast_ShapeType = kComplexRRect_ShapeType
+    };
+
+    static ShapeType RRectShapeType(const SkRRect& rrect) {
+        SkASSERT(rrect.getType() >= SkRRect::kRect_Type &&
+                 rrect.getType() <= SkRRect::kComplex_Type);
+        return static_cast<ShapeType>(rrect.getType() - 1);
+
+        GR_STATIC_ASSERT(kRect_ShapeType == SkRRect::kRect_Type - 1);
+        GR_STATIC_ASSERT(kOval_ShapeType == SkRRect::kOval_Type - 1);
+        GR_STATIC_ASSERT(kSimpleRRect_ShapeType == SkRRect::kSimple_Type - 1);
+        GR_STATIC_ASSERT(kNinePatch_ShapeType == SkRRect::kNinePatch_Type - 1);
+        GR_STATIC_ASSERT(kComplexRRect_ShapeType == SkRRect::kComplex_Type - 1);
+        GR_STATIC_ASSERT(kLast_ShapeType == SkRRect::kComplex_Type - 1);
+    }
+
+    enum ShapeFlag {
+        kRect_ShapeFlag          = (1 << kRect_ShapeType),
+        kOval_ShapeFlag          = (1 << kOval_ShapeType),
+        kSimpleRRect_ShapeFlag   = (1 << kSimpleRRect_ShapeType),
+        kNinePatch_ShapeFlag     = (1 << kNinePatch_ShapeType),
+        kComplexRRect_ShapeFlag  = (1 << kComplexRRect_ShapeType),
+
+        kRRect_ShapesMask = kSimpleRRect_ShapeFlag | kNinePatch_ShapeFlag | kComplexRRect_ShapeFlag
+    };
+
+    /**
+     * Defines what data is stored at which bits in the fInfo field of the instanced data.
+     */
+    enum InfoBits {
+        kShapeType_InfoBit       = 29,
+        kInnerShapeType_InfoBit  = 27,
+        kPerspective_InfoBit     = 26,
+        kLocalMatrix_InfoBit     = 25,
+        kParamsIdx_InfoBit       =  0
+    };
+
+    enum InfoMasks {
+        kShapeType_InfoMask       = 0u - (1 << kShapeType_InfoBit),
+        kInnerShapeType_InfoMask  = (1 << kShapeType_InfoBit) - (1 << kInnerShapeType_InfoBit),
+        kPerspective_InfoFlag     = (1 << kPerspective_InfoBit),
+        kLocalMatrix_InfoFlag     = (1 << kLocalMatrix_InfoBit),
+        kParamsIdx_InfoMask       = (1 << kLocalMatrix_InfoBit) - 1
+    };
+
+    GR_STATIC_ASSERT(kLast_ShapeType <= (uint32_t)kShapeType_InfoMask >> kShapeType_InfoBit);
+    GR_STATIC_ASSERT(kSimpleRRect_ShapeType <= kInnerShapeType_InfoMask >> kInnerShapeType_InfoBit);
+
+    struct BatchTracker {

[bsalomon, 2016/04/25 13:22:38]

We've been moving away from using "BatchTracker" in other batch subclasses and
just store the common properties in GrBatch member vars. However, this in this
instance it may make sense. I've never been a fan of the name, though. Could we
call this something like BatchProperties or CommonProperties? or ...

[Chris Dalton, 2016/04/25 17:01:19]

Agreed, I don't like the name either. We can come up with a better one.

[Chris Dalton, 2016/04/26 19:18:08]

Done.

+        BatchTracker() : fData(0) {}
+
+        bool isSimpleRects() const {
+            return !((fShapeTypes & ~kRect_ShapeFlag) | fInnerShapeTypes);
+        }
+
+        bool canJoin(BatchTracker that) const {
+            if (SkToBool(fInnerShapeTypes) != SkToBool(that.fInnerShapeTypes)) {
+                // GrInstanceProcessor can't currently combine draws with and without inner shapes.
+                return false;
+            }
+            if (fCannotDiscard != that.fCannotDiscard) {
+                // For stencil draws, the use of discard can be a requirement.
+                return false;
+            }
+            return true;
+        }
+
+        void join(BatchTracker that) {
+            SkASSERT(this->canJoin(that));
+            fData |= that.fData;
+        }
+
+        union {
+            struct {
+                uint8_t   fShapeTypes;
+                uint8_t   fInnerShapeTypes;
+                bool      fHasPerspective               : 1;
+                bool      fHasLocalMatrix               : 1;
+                bool      fHasParams                    : 1;
+                bool      fNonSquare                    : 1;
+                bool      fUsesColor                    : 1;

[bsalomon, 2016/04/25 13:22:39]

Uses color and uses coverage are rarely useful, do you need to track them? We've
been thinking about removing these.

[Chris Dalton, 2016/04/25 17:01:19]

Ok, I'll gladly take them out.

[Chris Dalton, 2016/04/26 19:18:08]

Done.

+                bool      fUsesCoverage                 : 1;
+                bool      fUsesLocalCoords              : 1;
+                bool      fCannotTweakAlphaForCoverage  : 1;
+                bool      fCannotDiscard                : 1;
+            };
+            uint32_t fData;
+        };
+    };
+
+    // This is required since all the data must fit into 32 bits of a shader key.
+    GR_STATIC_ASSERT(sizeof(uint32_t) == sizeof(BatchTracker));
+    GR_STATIC_ASSERT(kLast_ShapeType <= 8);
+
+    enum AntialiasMode {
+        kNone_AntialiasMode,
+        kCoverage_AntialiasMode,
+        kMSAA_AntialiasMode,
+        kMixedSamples_AntialiasMode,
+
+        kLast_AntialiasMode = kMixedSamples_AntialiasMode
+    };
+
+    enum AntialiasFlags {
+        kNone_AntialiasFlag          = (1 << kNone_AntialiasMode),
+        kCoverage_AntialiasFlag      = (1 << kCoverage_AntialiasMode),
+        kMSAA_AntialiasFlag          = (1 << kMSAA_AntialiasMode),
+        kMixedSamples_AntialiasFlag  = (1 << kMixedSamples_AntialiasMode)
+    };
+};
+
+#endif