Improve gpu path subdiv with perspective, remove tolerance scale, fix comment

Review URL: http://codereview.appspot.com/4993041/

git-svn-id: http://skia.googlecode.com/svn/trunk@2239 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/gpu/src/GrContext.cpp b/gpu/src/GrContext.cpp
index 639e25d68535f1bc04adb3a4dae6cadc1bcb105b..b857dda8f7b4ed1a969efe638e1a022c41d9d0fc 100644 (file)
--- a/gpu/src/GrContext.cpp
+++ b/gpu/src/GrContext.cpp
@@ -692,11 +692,6 @@ bool GrContext::prepareForOffscreenAA(GrDrawTarget* target,
         GR_STATIC_ASSERT(4 == OFFSCREEN_SSAA_SCALE);
         desc.fAALevel = kNone_GrAALevel;
     }
-    // Avoid overtesselating paths in AA buffers; may unduly reduce quality
-    // of simple circles?
-    if (pr) {
-        //pr->scaleCurveTolerance(GrIntToScalar(record->fScale));
-    }
     
     desc.fWidth *= record->fScale;
     desc.fHeight *= record->fScale;
@@ -881,10 +876,6 @@ void GrContext::doOffscreenAAPass2(GrDrawTarget* target,
 void GrContext::cleanupOffscreenAA(GrDrawTarget* target,
                                    GrPathRenderer* pr,
                                    OffscreenRecord* record) {
-    if (pr) {
-        // Counterpart of scale() in prepareForOffscreenAA()
-        //pr->scaleCurveTolerance(SkScalarInvert(SkIntToScalar(record->fScale)));
-    }
     target->restoreDrawState(record->fSavedState);
 }
 

diff --git a/gpu/src/GrDefaultPathRenderer.cpp b/gpu/src/GrDefaultPathRenderer.cpp
index b70b8631d50dbca2c5ab07a65eb1e260216cd67d..3323c91725164b623fd9fee16a23f9a7017a0aa2 100644 (file)
--- a/gpu/src/GrDefaultPathRenderer.cpp
+++ b/gpu/src/GrDefaultPathRenderer.cpp
@@ -374,18 +374,9 @@ void GrDefaultPathRenderer::onDrawPath(GrDrawTarget::StageBitfield stages,
                                        bool stencilOnly) {
 
     GrMatrix viewM = fTarget->getViewMatrix();
-    // In order to tesselate the path we get a bound on how much the matrix can
-    // stretch when mapping to screen coordinates.
-    GrScalar stretch = viewM.getMaxStretch();
-    bool useStretch = stretch > 0;
-    GrScalar tol = fCurveTolerance;
-
-    if (!useStretch) {
-        // TODO: deal with perspective in some better way.
-        tol /= 10;
-    } else {
-        tol = GrScalarDiv(tol, stretch);
-    }
+    GrScalar tol = GR_Scalar1;
+    tol = GrPathUtils::scaleToleranceToSrc(tol, viewM);
+
     // FIXME: It's really dumb that we recreate the verts for a new vertex
     // layout. We only do that because the GrDrawTarget API doesn't allow
     // us to change the vertex layout after reserveVertexSpace(). We won't

diff --git a/gpu/src/GrPathRenderer.cpp b/gpu/src/GrPathRenderer.cpp
index 06a00e44b76763f9dc273359971c73a1d7754e0e..929941a47eb4f3eaecad3d275f14e79af7731377 100644 (file)
--- a/gpu/src/GrPathRenderer.cpp
+++ b/gpu/src/GrPathRenderer.cpp
@@ -9,8 +9,7 @@
 #include "GrPathRenderer.h"
 
 GrPathRenderer::GrPathRenderer()
-    : fCurveTolerance (GR_Scalar1)
-    , fPath(NULL)
+    : fPath(NULL)
     , fTarget(NULL) {
 }
 

diff --git a/gpu/src/GrPathRenderer.h b/gpu/src/GrPathRenderer.h
index 4694de5252ab3ba893607a5d65f6b150acf21460..815da47de190e31f008a5e1180ccb984faa9c17c 100644 (file)
--- a/gpu/src/GrPathRenderer.h
+++ b/gpu/src/GrPathRenderer.h
@@ -36,8 +36,8 @@ public:
      * This is called to install custom path renderers in every GrContext at
      * create time. The default implementation in GrCreatePathRenderer_none.cpp
      * does not add any additional renderers. Link against another
-     * implementation to install your own. The most recently added is the
-     * most preferred path renderer.
+     * implementation to install your own. The first added is the most preferred
+     * path renderer, second is second most preferred, etc.
      *
      * @param context   the context that will use the path renderer
      * @param flags     flags indicating how path renderers will be used
@@ -169,16 +169,6 @@ public:
         GrCrash("Unexpected call to drawPathToStencil.");
     }
 
-    /**
-     * Multiply curve tolerance by the given value, increasing or decreasing
-     * the maximum error permitted in tesselating curves with short straight
-     * line segments.
-     */
-    void scaleCurveTolerance(GrScalar multiplier) {
-        GrAssert(multiplier > 0);
-        fCurveTolerance = SkScalarMul(fCurveTolerance, multiplier);
-    }
-
     /**
      * Helper that sets a path and automatically remove it in destructor.
      */
@@ -224,7 +214,6 @@ protected:
     virtual void pathWasSet() {}
     virtual void pathWillClear() {}
 
-    GrScalar fCurveTolerance;
     const SkPath*               fPath;
     GrDrawTarget*               fTarget;
     GrPathFill                  fFill;

diff --git a/gpu/src/GrPathUtils.cpp b/gpu/src/GrPathUtils.cpp
index 429b2945d3cc121e58c7965365367b3f3fe303f9..b7dc4b643ed321bb988f03da01e3e4be55b6e2e4 100644 (file)
--- a/gpu/src/GrPathUtils.cpp
+++ b/gpu/src/GrPathUtils.cpp
@@ -8,10 +8,27 @@
 
 
 #include "GrPathUtils.h"
+
 #include "GrPoint.h"
 
+GrScalar GrPathUtils::scaleToleranceToSrc(GrScalar devTol,
+                                          const GrMatrix& viewM) {
+    // In order to tesselate the path we get a bound on how much the matrix can
+    // stretch when mapping to screen coordinates.
+    GrScalar stretch = viewM.getMaxStretch();
+    GrScalar srcTol = devTol;
+
+    if (stretch < 0) {
+        // TODO: deal with perspective in some better way.
+        srcTol /= 5;
+        stretch = -stretch;
+    }
+    srcTol = GrScalarDiv(srcTol, stretch);
+    return srcTol;
+}
+
 static const int MAX_POINTS_PER_CURVE = 1 << 10;
-const GrScalar GrPathUtils::gMinCurveTol (GrFloatToScalar(0.0001f));
+static const GrScalar gMinCurveTol = GrFloatToScalar(0.0001f);
 
 uint32_t GrPathUtils::quadraticPointCount(const GrPoint points[],
                                           GrScalar tol) {

diff --git a/gpu/src/GrPathUtils.h b/gpu/src/GrPathUtils.h
index cde560139e9bc2edd713266e61b38e3fec40a44d..8d77982a7045067f66f6c8e118d74eb6e3376791 100644 (file)
--- a/gpu/src/GrPathUtils.h
+++ b/gpu/src/GrPathUtils.h
@@ -10,41 +10,42 @@
 #ifndef GrPathUtils_DEFINED
 #define GrPathUtils_DEFINED
 
-#include "GrNoncopyable.h"
-#include "GrPoint.h"
+#include "GrMatrix.h"
 #include "GrPath.h"
 
+class GrPoint;
+
 /**
  *  Utilities for evaluating paths.
  */
-class GrPathUtils : public GrNoncopyable {
-public:
+namespace GrPathUtils {
+    GrScalar scaleToleranceToSrc(GrScalar devTol,
+                                 const GrMatrix& viewM);
+
     /// Since we divide by tol if we're computing exact worst-case bounds,
     /// very small tolerances will be increased to gMinCurveTol.
-    static int worstCasePointCount(const GrPath&,
-                                   int* subpaths,
-                                   GrScalar tol);
+    int worstCasePointCount(const GrPath&,
+                            int* subpaths,
+                            GrScalar tol);
     /// Since we divide by tol if we're computing exact worst-case bounds,
     /// very small tolerances will be increased to gMinCurveTol.
-    static uint32_t quadraticPointCount(const GrPoint points[], GrScalar tol);
-    static uint32_t generateQuadraticPoints(const GrPoint& p0,
-                                            const GrPoint& p1,
-                                            const GrPoint& p2,
-                                            GrScalar tolSqd,
-                                            GrPoint** points,
-                                            uint32_t pointsLeft);
+    uint32_t quadraticPointCount(const GrPoint points[], GrScalar tol);
+    uint32_t generateQuadraticPoints(const GrPoint& p0,
+                                     const GrPoint& p1,
+                                     const GrPoint& p2,
+                                     GrScalar tolSqd,
+                                     GrPoint** points,
+                                     uint32_t pointsLeft);
     /// Since we divide by tol if we're computing exact worst-case bounds,
     /// very small tolerances will be increased to gMinCurveTol.
-    static uint32_t cubicPointCount(const GrPoint points[], GrScalar tol);
-    static uint32_t generateCubicPoints(const GrPoint& p0,
-                                        const GrPoint& p1,
-                                        const GrPoint& p2,
-                                        const GrPoint& p3,
-                                        GrScalar tolSqd,
-                                        GrPoint** points,
-                                        uint32_t pointsLeft);
-
-private:
-    static const GrScalar gMinCurveTol;
+    uint32_t cubicPointCount(const GrPoint points[], GrScalar tol);
+    uint32_t generateCubicPoints(const GrPoint& p0,
+                                 const GrPoint& p1,
+                                 const GrPoint& p2,
+                                 const GrPoint& p3,
+                                 GrScalar tolSqd,
+                                 GrPoint** points,
+                                 uint32_t pointsLeft);
+
 };
 #endif

diff --git a/gpu/src/GrTesselatedPathRenderer.cpp b/gpu/src/GrTesselatedPathRenderer.cpp
index 15e3cc0230181c037cd1cbf064f92d71f6e92d6a..dd281ed0b4f45728991f29289d116222e33f5ad1 100644 (file)
--- a/gpu/src/GrTesselatedPathRenderer.cpp
+++ b/gpu/src/GrTesselatedPathRenderer.cpp
@@ -351,18 +351,9 @@ void GrTesselatedPathRenderer::drawPath(GrDrawTarget::StageBitfield stages) {
     GrAssert(GrDrawTarget::kBoth_DrawFace == fTarget->getDrawFace());
 
     GrMatrix viewM = fTarget->getViewMatrix();
-    // In order to tesselate the path we get a bound on how much the matrix can
-    // stretch when mapping to screen coordinates.
-    GrScalar stretch = viewM.getMaxStretch();
-    bool useStretch = stretch > 0;
-    GrScalar tol = fCurveTolerance;
-
-    if (!useStretch) {
-        // TODO: deal with perspective in some better way.
-        tol /= 10;
-    } else {
-        tol = GrScalarDiv(tol, stretch);
-    }
+
+    GrScalar tol = GR_Scalar1;
+    tol = GrPathUtils::scaleToleranceToSrc(tol, viewM);
     GrScalar tolSqd = GrMul(tol, tol);
 
     int subpathCnt;

diff --git a/include/core/SkMatrix.h b/include/core/SkMatrix.h
index 7c2109c629b8af8b2555fb48a8c9c05f6f9328f5..333749c4420f170b648747aa6b7da8d2d5bb3ced 100644 (file)
--- a/include/core/SkMatrix.h
+++ b/include/core/SkMatrix.h
@@ -501,10 +501,12 @@ public:
     void toDumpString(SkString*) const;
 
     /**
-     * Calculates the maximum stretching factor of the matrix. Only defined if
-     * the matrix does not have perspective.
+     * Calculates the maximum stretching factor of the matrix. If the matrix has
+     * perspective the max stretch at the origin (in the pre-matrix space) is
+     * computed and returned as a negative.
      *
-     * @return maximum strecthing factor or negative if matrix has perspective.
+     * @return maximum strecthing factor or negative max stretching factor at
+     * the origin if matrix has perspective.
      */
     SkScalar getMaxStretch() const;
 

diff --git a/samplecode/SampleHairCurves.cpp b/samplecode/SampleHairCurves.cpp
index 152cbeb09c7872be36a59918b19233f5b24e10b8..a3ea5e103d458578304a8e91f2f3664ac65961b4 100644 (file)
--- a/samplecode/SampleHairCurves.cpp
+++ b/samplecode/SampleHairCurves.cpp
@@ -28,6 +28,11 @@ protected:
 
 
     virtual void onDrawContent(SkCanvas* canvas) {
+        SkMatrix m;
+        m.reset();
+        m.setPerspX(0.00020);
+        canvas->concat(m);
+
         SkPaint paint;
         paint.setAntiAlias(true);
         paint.setStyle(SkPaint::kStroke_Style);
@@ -38,6 +43,7 @@ protected:
         SkPath curves;
         SkPath hulls;
         SkPath ctrlPts;
+
         for (int i = 0; i < 100; ++i) {
             SkScalar pts[] = {
                 rand.nextUScalar1(), rand.nextUScalar1(),

diff --git a/src/core/SkMatrix.cpp b/src/core/SkMatrix.cpp
index a525f11a0197f58b4a8b7dfcc259f6efecd2f9e9..6fc6e5de30311a3bdc0e2b0df522332443e4d254 100644 (file)
--- a/src/core/SkMatrix.cpp
+++ b/src/core/SkMatrix.cpp
@@ -1674,10 +1674,6 @@ bool SkMatrix::setPolyToPoly(const SkPoint src[], const SkPoint dst[],
 SkScalar SkMatrix::getMaxStretch() const {
     TypeMask mask = this->getType();
 
-    if (mask & kPerspective_Mask) {
-        return -SK_Scalar1;
-    }
-    
     SkScalar stretch;
     
     if (this->isIdentity()) {
@@ -1702,7 +1698,8 @@ SkScalar SkMatrix::getMaxStretch() const {
         // and roots are guaraunteed to be pos and real).
         SkScalar largerRoot;
         SkScalar bSqd = SkScalarMul(b,b);
-        if (bSqd <= SkFloatToScalar(1e-10)) { // will be true if upper left 2x2 is orthogonal, which is common, so save some math
+        // if upper left 2x2 is orthogonal save some math
+        if (bSqd <= SK_ScalarNearlyZero) {
             largerRoot = SkMaxScalar(a, c);
         } else {
             SkScalar aminusc = a - c;
@@ -1710,8 +1707,17 @@ SkScalar SkMatrix::getMaxStretch() const {
             SkScalar x = SkScalarSqrt(SkScalarMul(aminusc, aminusc) + 4 * bSqd) / 2;
             largerRoot = apluscdiv2 + x;
         }
-        
         stretch = SkScalarSqrt(largerRoot);
+        if (mask & kPerspective_Mask) {
+            stretch = -stretch;
+            if (fMat[kMPersp2] != kMatrix22Elem) {
+#if defined(SK_SCALAR_IS_FLOAT)
+                stretch /= fMat[kMPersp2];
+#else
+                stretch = SkFractDiv(stretch, fMat[kMPersp2]);
+#endif
+            }
+        }
     }
 #if defined(SK_DEBUG) && 0
     // test a bunch of vectors. None should be scaled by more than stretch