Revert "Add GM to verify that drawX == (path.addX, drawPath)"

This reverts commit 78d1b428a8e8a0b76e88e9266d2893136acd5906.

Reason for revert: break preabandongpu bot

Original change's description:
> Add GM to verify that drawX == (path.addX, drawPath)
>
> This demonstrates a new kind of hybrid unit test/GM.
> It creates a grid of cells. In each cell, we do two
> renders that are expected to produce the same result.
> For each cell, we render the two results overlaid,
> and highlight any differing pixels in red. Assuming
> there is a diff, the area around the largest diff
> is drawn zoomed in from both images.
>
> Matching cells are outlined in green, failing cells
> are outlined in red. Triaging this GM just involves
> answering the question: "Are there any red boxes?"
>
> "Good" example: https://screenshot.googleplex.com/909P3tvS55f.png
> "Bad" example: https://screenshot.googleplex.com/oXBWbEKw5ur.png
>
> To get more tests to pass, (and fix an assert
> in Ganesh), I've gone ahead and enforced that user
> supplied rects (in drawRect and drawOval) are
> always sorted once they hit the canvas virtuals.
> Currently, drawArc rejects empty ovals, but I added
> the same assert to onDrawArc, if we decide to change
> the strategy there.
>
> Bug: skia:
> Change-Id: I021a18c85e234298e1d29f333662683d996dd42c
> Reviewed-on: https://skia-review.googlesource.com/16983
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Mike Klein <mtklein@chromium.org>
> Reviewed-by: Mike Reed <reed@google.com>
>

TBR=mtklein@chromium.org,mtklein@google.com,brianosman@google.com,reed@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Bug: skia:

Change-Id: Id1ead4e22115c49cad5d0adb6151ede81734b4d3
Reviewed-on: https://skia-review.googlesource.com/17269
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>

diff --git a/gm/shapes_as_paths.cpp b/gm/shapes_as_paths.cpp
deleted file mode 100644 (file)
index 20c097f..0000000
--- a/gm/shapes_as_paths.cpp
+++ /dev/null
@@ -1,239 +0,0 @@
-/*
- * Copyright 2017 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "gm.h"
-
-#include "SkAutoPixmapStorage.h"
-#include "SkImage.h"
-#include "SkPath.h"
-#include "SkSurface.h"
-
-namespace skiagm {
-
-static void draw_diff(SkCanvas* canvas, SkImage* imgA, SkImage* imgB) {
-    SkASSERT(imgA->dimensions() == imgB->dimensions());
-
-    int w = imgA->width(), h = imgA->height();
-
-    // First, draw the two images faintly overlaid
-    SkPaint paint;
-    paint.setAlpha(64);
-    paint.setBlendMode(SkBlendMode::kPlus);
-    canvas->drawImage(imgA, 0, 0, &paint);
-    canvas->drawImage(imgB, 0, 0, &paint);
-
-    // Next, read the pixels back, figure out if there are any differences
-    SkImageInfo info = SkImageInfo::MakeN32Premul(w, h);
-    SkAutoPixmapStorage pmapA;
-    SkAutoPixmapStorage pmapB;
-    pmapA.alloc(info);
-    pmapB.alloc(info);
-    if (!imgA->readPixels(pmapA, 0, 0) || !imgB->readPixels(pmapB, 0, 0)) {
-        return;
-    }
-
-    int maxDiffX = 0, maxDiffY = 0, maxDiff = 0;
-    SkBitmap highlight;
-    highlight.allocN32Pixels(w, h);
-    highlight.eraseColor(SK_ColorTRANSPARENT);
-
-    for (int y = 0; y < h; ++y) {
-        for (int x = 0; x < w; ++x) {
-            uint32_t pixelA = *pmapA.addr32(x, y);
-            uint32_t pixelB = *pmapB.addr32(x, y);
-            if (pixelA != pixelB) {
-                int diff =
-                    SkTAbs((int)(SkColorGetR(pixelA) - SkColorGetR(pixelB))) +
-                    SkTAbs((int)(SkColorGetG(pixelA) - SkColorGetG(pixelB))) +
-                    SkTAbs((int)(SkColorGetB(pixelA) - SkColorGetB(pixelB))) +
-                    SkTAbs((int)(SkColorGetA(pixelA) - SkColorGetA(pixelB)));
-                if (diff > maxDiff) {
-                    maxDiffX = x;
-                    maxDiffY = y;
-                    maxDiff = diff;
-                }
-                *highlight.getAddr32(x, y) = SkPackARGB32(0xA0, 0xA0, 0x00, 0x00);
-            }
-        }
-    }
-
-    SkPaint outline;
-    outline.setStyle(SkPaint::kStroke_Style);
-    outline.setColor(maxDiff == 0 ? 0xFF007F00 : 0xFF7F0000);
-
-    if (maxDiff > 0) {
-        // Call extra attention to the region we're going to zoom
-        SkPMColor yellow = SkPackARGB32(0xFF, 0xFF, 0xFF, 0x00);
-        *highlight.getAddr32(maxDiffX, maxDiffY) = yellow;
-        *highlight.getAddr32(SkTMax(maxDiffX - 1, 0), maxDiffY) = yellow;
-        *highlight.getAddr32(maxDiffX, SkTMax(maxDiffY - 1, 0)) = yellow;
-        *highlight.getAddr32(SkTMin(maxDiffX + 1, w - 1), maxDiffY) = yellow;
-        *highlight.getAddr32(maxDiffX, SkTMin(maxDiffY + 1, h - 1)) = yellow;
-
-        // Draw the overlay
-        canvas->drawBitmap(highlight, 0, 0);
-
-        // Draw zoom of largest pixel diff
-        canvas->drawImageRect(imgA, SkRect::MakeXYWH(maxDiffX - 5, maxDiffY - 5, 10, 10),
-                              SkRect::MakeXYWH(w, 0, w, h), nullptr);
-        canvas->drawImageRect(imgB, SkRect::MakeXYWH(maxDiffX - 5, maxDiffY - 5, 10, 10),
-                              SkRect::MakeXYWH(2 * w, 0, w, h), nullptr);
-
-        // Add lines to separate zoom boxes
-        canvas->drawLine(w, 0, w, h, outline);
-        canvas->drawLine(2 * w, 0, 2 * w, h, outline);
-    }
-
-    // Draw outline of whole test region
-    canvas->drawRect(SkRect::MakeWH(3 * w, h), outline);
-}
-
-namespace {
-typedef std::function<void(SkCanvas*, const SkRect&, const SkPaint&)> ShapeDrawFunc;
-}
-
-/**
- *  Iterates over a variety of rect shapes, paint parameters, and matrices, calling two different
- *  user-supplied draw callbacks. Produces a grid clearly showing if the two callbacks produce the
- *  same visual results in all cases.
- */
-static void draw_rect_geom_diff_grid(SkCanvas* canvas, ShapeDrawFunc f1, ShapeDrawFunc f2) {
-    // Variables:
-    // - Fill, hairline, wide stroke
-    // - Axis aligned, rotated, scaled, scaled negative, perspective
-    // - Source geometry (normal, collapsed, inverted)
-    //
-    // Things not (yet?) tested:
-    // - AntiAlias on/off
-    // - StrokeAndFill
-    // - Cap/join
-    // - Anything even more elaborate...
-
-    const SkRect kRects[] = {
-        SkRect::MakeXYWH(10, 10, 30, 30),  // Normal
-        SkRect::MakeXYWH(10, 25, 30, 0),   // Collapsed
-        SkRect::MakeXYWH(10, 40, 30, -30), // Inverted
-    };
-
-    const struct { SkPaint::Style fStyle; SkScalar fStrokeWidth; } kStyles[] = {
-        { SkPaint::kFill_Style, 0 },   // Filled
-        { SkPaint::kStroke_Style, 0 }, // Hairline
-        { SkPaint::kStroke_Style, 5 }, // Wide stroke
-    };
-
-    SkMatrix mI = SkMatrix::I();
-    SkMatrix mRot;
-    mRot.setRotate(30, 25, 25);
-    SkMatrix mScale;
-    mScale.setScaleTranslate(0.5f, 1, 12.5f, 0);
-    SkMatrix mFlipX;
-    mFlipX.setScaleTranslate(-1, 1, 50, 0);
-    SkMatrix mFlipY;
-    mFlipY.setScaleTranslate(1, -1, 0, 50);
-    SkMatrix mFlipXY;
-    mFlipXY.setScaleTranslate(-1, -1, 50, 50);
-    SkMatrix mPersp;
-    mPersp.setIdentity();
-    mPersp.setPerspY(0.002f);
-
-    const SkMatrix* kMatrices[] = { &mI, &mRot, &mScale, &mFlipX, &mFlipY, &mFlipXY, &mPersp, };
-
-    canvas->translate(10, 10);
-
-    SkImageInfo info = canvas->imageInfo().makeWH(50, 50);
-    if (kUnknown_SkColorType == info.colorType()) {
-        info = SkImageInfo::MakeN32Premul(50, 50);
-    }
-    auto surface = canvas->makeSurface(info);
-    if (!surface) {
-        surface = SkSurface::MakeRaster(info);
-    }
-
-    for (const SkRect& rect : kRects) {
-        for (const auto& style : kStyles) {
-            canvas->save();
-
-            for (const SkMatrix* mat : kMatrices) {
-                SkPaint paint;
-                paint.setColor(SK_ColorWHITE);
-                paint.setAntiAlias(true);
-                paint.setStyle(style.fStyle);
-                paint.setStrokeWidth(style.fStrokeWidth);
-
-                // Do first draw
-                surface->getCanvas()->clear(SK_ColorBLACK);
-                surface->getCanvas()->save();
-                surface->getCanvas()->concat(*mat);
-                f1(surface->getCanvas(), rect, paint);
-                surface->getCanvas()->restore();
-                auto imgA = surface->makeImageSnapshot();
-
-                // Do second draw
-                surface->getCanvas()->clear(SK_ColorBLACK);
-                surface->getCanvas()->save();
-                surface->getCanvas()->concat(*mat);
-                f2(surface->getCanvas(), rect, paint);
-                surface->getCanvas()->restore();
-                auto imgB = surface->makeImageSnapshot();
-
-                draw_diff(canvas, imgA.get(), imgB.get());
-                canvas->translate(160, 0);
-            }
-            canvas->restore();
-            canvas->translate(0, 60);
-        }
-    }
-}
-
-static const int kNumRows = 9;
-static const int kNumColumns = 7;
-static const int kTotalWidth = kNumColumns * 160 + 10;
-static const int kTotalHeight = kNumRows * 60 + 10;
-
-DEF_SIMPLE_GM_BG(rects_as_paths, canvas, kTotalWidth, kTotalHeight, SK_ColorBLACK) {
-    // Drawing a rect vs. adding it to a path and drawing the path, should produce same results.
-    auto rectDrawFunc = [](SkCanvas* canvas, const SkRect& rect, const SkPaint& paint) {
-        canvas->drawRect(rect, paint);
-    };
-    auto pathDrawFunc = [](SkCanvas* canvas, const SkRect& rect, const SkPaint& paint) {
-        SkPath path;
-        path.addRect(rect);
-        canvas->drawPath(path, paint);
-    };
-
-    draw_rect_geom_diff_grid(canvas, rectDrawFunc, pathDrawFunc);
-}
-
-DEF_SIMPLE_GM_BG(ovals_as_paths, canvas, kTotalWidth, kTotalHeight, SK_ColorBLACK) {
-    // Drawing an oval vs. adding it to a path and drawing the path, should produce same results.
-    auto ovalDrawFunc = [](SkCanvas* canvas, const SkRect& rect, const SkPaint& paint) {
-        canvas->drawOval(rect, paint);
-    };
-    auto pathDrawFunc = [](SkCanvas* canvas, const SkRect& rect, const SkPaint& paint) {
-        SkPath path;
-        path.addOval(rect);
-        canvas->drawPath(path, paint);
-    };
-
-    draw_rect_geom_diff_grid(canvas, ovalDrawFunc, pathDrawFunc);
-}
-
-DEF_SIMPLE_GM_BG(arcs_as_paths, canvas, kTotalWidth, kTotalHeight, SK_ColorBLACK) {
-    // Drawing an arc vs. adding it to a path and drawing the path, should produce same results.
-    auto arcDrawFunc = [](SkCanvas* canvas, const SkRect& rect, const SkPaint& paint) {
-        canvas->drawArc(rect, 10, 200, false, paint);
-    };
-    auto pathDrawFunc = [](SkCanvas* canvas, const SkRect& rect, const SkPaint& paint) {
-        SkPath path;
-        path.addArc(rect, 10, 200);
-        canvas->drawPath(path, paint);
-    };
-
-    draw_rect_geom_diff_grid(canvas, arcDrawFunc, pathDrawFunc);
-}
-
-}

diff --git a/gn/gm.gni b/gn/gm.gni
index 0a6681b..5e33641 100644 (file)
--- a/gn/gm.gni
+++ b/gn/gm.gni
@@ -259,7 +259,6 @@ gm_sources = [
   "$_gm/shadowutils.cpp",
   "$_gm/shallowgradient.cpp",
   "$_gm/shapes.cpp",
-  "$_gm/shapes_as_paths.cpp",
   "$_gm/showmiplevels.cpp",
   "$_gm/simpleaaclip.cpp",
   "$_gm/simple_magnification.cpp",

diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 216d8fa..afc1203 100644 (file)
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -1711,9 +1711,7 @@ void SkCanvas::drawPaint(const SkPaint& paint) {
 }
 
 void SkCanvas::drawRect(const SkRect& r, const SkPaint& paint) {
-    // To avoid redundant logic in our culling code and various backends, we always sort rects
-    // before passing them along.
-    this->onDrawRect(r.makeSorted(), paint);
+    this->onDrawRect(r, paint);
 }
 
 void SkCanvas::drawRegion(const SkRegion& region, const SkPaint& paint) {
@@ -1729,9 +1727,7 @@ void SkCanvas::drawRegion(const SkRegion& region, const SkPaint& paint) {
 }
 
 void SkCanvas::drawOval(const SkRect& r, const SkPaint& paint) {
-    // To avoid redundant logic in our culling code and various backends, we always sort rects
-    // before passing them along.
-    this->onDrawOval(r.makeSorted(), paint);
+    this->onDrawOval(r, paint);
 }
 
 void SkCanvas::drawRRect(const SkRRect& rrect, const SkPaint& paint) {
@@ -1997,10 +1993,11 @@ static bool needs_autodrawlooper(SkCanvas* canvas, const SkPaint& paint) {
 
 void SkCanvas::onDrawRect(const SkRect& r, const SkPaint& paint) {
     TRACE_EVENT0("disabled-by-default-skia", "SkCanvas::drawRect()");
-    SkASSERT(r.isSorted());
     if (paint.canComputeFastBounds()) {
+        // Skia will draw an inverted rect, because it explicitly "sorts" it downstream.
+        // To prevent accidental rejecting at this stage, we have to sort it before we check.
         SkRect storage;
-        if (this->quickReject(paint.computeFastBounds(r, &storage))) {
+        if (this->quickReject(paint.computeFastBounds(r.makeSorted(), &storage))) {
             return;
         }
     }
@@ -2042,10 +2039,11 @@ void SkCanvas::onDrawRegion(const SkRegion& region, const SkPaint& paint) {
 
 void SkCanvas::onDrawOval(const SkRect& oval, const SkPaint& paint) {
     TRACE_EVENT0("disabled-by-default-skia", "SkCanvas::drawOval()");
-    SkASSERT(oval.isSorted());
     if (paint.canComputeFastBounds()) {
+        // Skia will draw an inverted rect, because it explicitly "sorts" it downstream.
+        // To prevent accidental rejecting at this stage, we have to sort it before we check.
         SkRect storage;
-        if (this->quickReject(paint.computeFastBounds(oval, &storage))) {
+        if (this->quickReject(paint.computeFastBounds(oval.makeSorted(), &storage))) {
             return;
         }
     }
@@ -2063,11 +2061,12 @@ void SkCanvas::onDrawArc(const SkRect& oval, SkScalar startAngle,
                          SkScalar sweepAngle, bool useCenter,
                          const SkPaint& paint) {
     TRACE_EVENT0("disabled-by-default-skia", "SkCanvas::drawArc()");
-    SkASSERT(oval.isSorted());
     if (paint.canComputeFastBounds()) {
+        // Skia will draw an inverted rect, because it explicitly "sorts" it downstream.
+        // To prevent accidental rejecting at this stage, we have to sort it before we check.
         SkRect storage;
         // Note we're using the entire oval as the bounds.
-        if (this->quickReject(paint.computeFastBounds(oval, &storage))) {
+        if (this->quickReject(paint.computeFastBounds(oval.makeSorted(), &storage))) {
             return;
         }
     }