Create GrDraw and start fast pathing src over rects

src/gpu/draws/GrDrawRect.cpp

+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "GrDrawRect.h"
+
+#include "GrContext.h"
+#include "GrClip.h"
+#include "GrDrawTarget.h"
+#include "GrRenderTarget.h"
+#include "GrStrokeInfo.h"
+#include "SkColorFilter.h"
+#include "SkImageFilter.h"
+#include "SkGr.h"
+#include "SkMatrix.h"
+#include "SkMaskFilter.h"
+#include "SkPaint.h"
+#include "SkRect.h"
+#include "SkShader.h"
+#include "SkXfermode.h"
+#include "batches/GrRectBatchFactory.h"
+

[robertphillips, 2015/09/25 12:48:49]

AAStroke
AAFill
BWStroke
BWFill

?

[joshualitt, 2015/09/25 18:17:13]

Brian doesn't like BW

[robertphillips, 2015/09/25 20:00:35]

I proposing a reordering not a renaming.

+enum RectBatchType {
+    kAAStroke_RectBatchType,
+    kAAFill_RectBatchType,
+    kNonAAFill_RectBatchType,
+    kNonAAStroke_RectBatchType,
+};
+
+class GrDrawRectSnap : public GrDrawSnap {
+public:
+    GrDrawRectSnap(const SkPaint& paint,
+                   const GrClip& clip,
+                   RectBatchType type,
+                   bool snapToPixelCenter)
+        : fShader(SkSafeRef(paint.getShader()))
+        , fMaskFilter(SkSafeRef(paint.getMaskFilter()))

[bsalomon, 2015/09/23 13:49:38]

Maybe just punt if the originating paint has a IM, MF, or CF? Going to be very
rare.

[joshualitt, 2015/09/25 18:17:13]

Acknowledged.

+        , fImageFilter(SkSafeRef(paint.getImageFilter()))
+        , fColorFilter(SkSafeRef(paint.getColorFilter()))
+        , fClipGenID(clip.clipType() != GrClip::kClipStack_ClipType ?
+                     -1 :
+                     clip.clipStack()->getTopmostGenID())
+        , fType(type)
+        , fSnapToPixelCenters(snapToPixelCenter)
+        , fIsDither(paint.isDither()) {
+        fClassID = GrDrawRect::ClassID();
+    }
+
+    bool canBatch(const SkPaint& paint,
+                  const GrClip& clip,
+                  RectBatchType type,
+                  bool snapToPixelCenters) const {
+        return paint.getShader() == fShader &&
+               paint.getMaskFilter() == fMaskFilter &&
+               paint.getImageFilter() == fImageFilter &&
+               paint.getColorFilter() == fColorFilter &&
+               paint.isDither() == fIsDither &&
+               fClipGenID != -1 && clip.clipType() == GrClip::kClipStack_ClipType &&
+               fClipGenID == clip.clipStack()->getTopmostGenID() &&
+               fType == type &&
+               fSnapToPixelCenters == snapToPixelCenters;
+    }
+
+    // we only bother fast pathing kSrcOver_Mode, though we could do quite a bit more
+    static bool CanSnap(const SkPaint& paint) {

[bsalomon, 2015/09/23 13:49:38]

kSrcOver_Mode is always a nullptr for paint.getXfermode(). Maybe save a function
call here and just return nullptr == paint.getXfermode()?

[joshualitt, 2015/09/25 18:17:13]

Acknowledged.

+        return SkXfermode::IsMode(paint.getXfermode(), SkXfermode::kSrcOver_Mode);
+    }
+
+private:
+    SkAutoTUnref<SkShader> fShader;
+    SkAutoTUnref<SkMaskFilter> fMaskFilter;
+    SkAutoTUnref<SkImageFilter> fImageFilter;
+    SkAutoTUnref<SkColorFilter> fColorFilter;
+    int32_t fClipGenID;
+    RectBatchType fType;
+    bool fSnapToPixelCenters;
+    bool fIsDither;
+
+    typedef GrDrawSnap INHERITED;
+};
+
+inline static RectBatchType compute_rect_batch_type(bool useAA, SkScalar width) {
+    if (useAA) {
+        if (width >= 0) {
+            return kAAStroke_RectBatchType;
+        } else {
+            return kAAFill_RectBatchType;
+        }
+    } else if (width >= 0) {
+        return kNonAAStroke_RectBatchType;
+    } else {
+        return kNonAAFill_RectBatchType;
+    }
+}
+
+inline static bool can_fastpath(GrDrawTarget* drawTarget,
+                                const SkPaint& paint,
+                                const GrClip& clip,
+                                GrColor color,
+                                const SkMatrix& viewMatrix,
+                                const SkRect& rect,
+                                const GrStrokeInfo& strokeInfo,
+                                RectBatchType batchType,
+                                bool snapToPixelCenters) {
+    GrBatch* lastBatch = drawTarget->lastBatch();
+    if (!lastBatch) {
+        return false;
+    }
+

[robertphillips, 2015/09/25 12:48:49]

drawSnap -> getLastSnap ?

calling drawSnap here sounds like we're doing something else 

[joshualitt, 2015/09/25 18:17:13]

Acknowledged.

+    const GrDrawSnap* lastSnap = lastBatch->drawSnap();
+    if (!lastSnap) {
+        return false;
+    }
+
+    if (lastSnap->classID() != GrDrawRect::ClassID()) {
+        return false;
+    }
+
+    const GrDrawRectSnap* lastSnapCast = static_cast<const GrDrawRectSnap*>(lastSnap);
+    if (lastSnapCast->canBatch(paint, clip, batchType, snapToPixelCenters)) {
+        switch (batchType) {
+            case kAAStroke_RectBatchType:

[bsalomon, 2015/09/23 13:49:38]

Would it make sense to store an Append()/Create() proc ptr pair rather than have
an enum?

[robertphillips, 2015/09/25 12:48:49]

We would still have to deal with all the different parameters.

[joshualitt, 2015/09/25 18:17:13]

yea, I think in the short term this is the best way to handle things.  I think
we can come up with something better, but I'm not sure what that is at this
point in time.

+                if (GrAAStrokeRectBatch::Append(lastBatch, color, viewMatrix, rect,
+                                                strokeInfo)) {
+                    return true;
+                }
+                break;
+            case kAAFill_RectBatchType:
+                GrAAFillRectBatch::Append(lastBatch, color, viewMatrix, rect);
+                return true;
+            case kNonAAFill_RectBatchType:
+                if (GrNonAAFillRectBatch::Append(lastBatch, color, viewMatrix, rect,
+                                                 nullptr, nullptr)) {
+                    return true;
+                }
+                break;
+            case kNonAAStroke_RectBatchType:
+                // GrNonAAStrokeRectBatch doesn't batch yet
+                break;
+        }
+    }
+
+    return false;
+}
+
+void GrDrawRect::execute(GrDrawTarget* drawTarget) const {
+    const SkPaint& paint = *fPaint;
+    const GrClip& clip = *fClip;
+    const SkMatrix& viewMatrix = *fViewMatrix;
+    const SkRect& rect = *fRect;
+

[robertphillips, 2015/09/25 12:48:49]

Didn't we pull all the info for fStrokeInfo out of the SkPaint ?

[joshualitt, 2015/09/25 18:17:13]

yes, but why pull it out again if we don't have to?

+    SkScalar width = nullptr == fStrokeInfo ? -1 : fStrokeInfo->getWidth();
+
+    bool needAA = fPaint->isAntiAlias() && !fRenderTarget->isUnifiedMultisampled();
+
+    // The fill path can handle rotation but not skew
+    // The stroke path needs the rect to remain axis aligned (no rotation or skew)
+    // None of our AA draw rect calls can handle perspective yet
+    bool canApplyAA = width >= 0 ? viewMatrix.rectStaysRect() : viewMatrix.preservesRightAngles();
+    bool useAA = needAA && canApplyAA;
+
+    // Non-AA hairlines are snapped to pixel centers to make which pixels are hit deterministic
+    bool snapToPixelCenters = !useAA && (0 == width && !fRenderTarget->isUnifiedMultisampled());
+
+    // TODO we also do this on the paint, do we need to?

[bsalomon, 2015/09/23 13:49:38]

I'd just punt on CF. I'm also changing how CF works in skptogrp

[joshualitt, 2015/09/25 18:17:13]

I need to handle the color here either way unfortunately.

+    SkColor paintColor = paint.getColor();
+    if (fPaint->getColorFilter() && !paint.getShader()) {
+        paintColor = paint.getColorFilter()->filterColor(paintColor);
+    }
+    GrColor color = SkColor2GrColor(paintColor);
+
+    RectBatchType batchType = compute_rect_batch_type(useAA, width);

[robertphillips, 2015/09/25 12:48:49]

can_fastpath -> attempt_fastpath ?

[joshualitt, 2015/09/25 18:17:13]

Acknowledged.

+    if (can_fastpath(drawTarget, *fPaint, clip, color, viewMatrix, rect, *fStrokeInfo, batchType,
+                     snapToPixelCenters)) {
+        return;
+    }
+
+    SkAutoTUnref<GrDrawBatch> batch;
+    switch (batchType) {
+        case kAAStroke_RectBatchType:
+            batch.reset(GrRectBatchFactory::CreateAAStroke(color, viewMatrix, rect, *fStrokeInfo));
+            break;
+        case kAAFill_RectBatchType:
+            batch.reset(GrRectBatchFactory::CreateAAFill(color, viewMatrix, rect));
+            break;
+        case kNonAAFill_RectBatchType:
+            batch.reset(GrRectBatchFactory::CreateNonAAFill(color, viewMatrix, rect, nullptr,
+                                                            nullptr));
+            break;
+        case kNonAAStroke_RectBatchType:
+            batch.reset(GrRectBatchFactory::CreateNonAAStroke(color, viewMatrix, rect, width,
+                                                              snapToPixelCenters));
+            break;
+    }
+
+    GrPaint grPaint;
+    if (!SkPaint2GrPaint(fContext, paint, viewMatrix, true, &grPaint)) {
+        return;
+    }
+
+    GrPipelineBuilder pipelineBuilder(grPaint, fRenderTarget, clip);
+
+    // Depending on sub-pixel coordinates and the particular GPU, we may lose a corner of
+    // hairline rects. We jam all the vertices to pixel centers to avoid this, but not when MSAA
+    // is enabled because it can cause ugly artifacts.
+    pipelineBuilder.setState(GrPipelineBuilder::kSnapVerticesToPixelCenters_Flag,
+                             snapToPixelCenters);
+    drawTarget->drawBatch(pipelineBuilder, batch);

[robertphillips, 2015/09/25 12:48:49]

So if we can't snap we just don't draw ?

[joshualitt, 2015/09/25 18:17:13]

If we can't snap, we *do* draw(drawTarget->drawBatch above), we just leave the
snap on the batch null

+    if (GrDrawRectSnap::CanSnap(paint)) {
+        GrDrawRectSnap* drawRectSnap = batch->getSnapStorage<GrDrawRectSnap>();
+        new (drawRectSnap) GrDrawRectSnap(paint, clip, batchType, snapToPixelCenters);
+    }
+}