Fix stroked oval and rrect rendering on Ubuntu-x86-Release.

src/gpu/GrOvalRenderer.cpp

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "GrOvalRenderer.h"
 
 #include "GrEffect.h"
@@ skipping 341 matching lines @@
     if (!geo.succeeded()) {
         GrPrintf("Failed to get space for vertices!\n");
         return;
     }
 
     CircleVertex* verts = reinterpret_cast<CircleVertex*>(geo.vertices());
 
     SkStrokeRec::Style style = stroke.getStyle();
     bool isStroked = (SkStrokeRec::kStroke_Style == style || SkStrokeRec::kHairline_Style == style);
 
-    GrEffectRef* effect = CircleEdgeEffect::Create(isStroked);
-    static const int kCircleEdgeAttrIndex = 1;
-    drawState->addCoverageEffect(effect, kCircleEdgeAttrIndex)->unref();
-
     SkScalar innerRadius = 0.0f;
     SkScalar outerRadius = radius;
     SkScalar halfWidth = 0;
     if (style != SkStrokeRec::kFill_Style) {
         if (SkScalarNearlyZero(strokeWidth)) {
             halfWidth = SK_ScalarHalf;
         } else {
             halfWidth = SkScalarHalf(strokeWidth);
         }
 
         outerRadius += halfWidth;
         if (isStroked) {
             innerRadius = radius - halfWidth;

[robertphillips, 2013/08/29 17:01:14]

Did you try leaving the "innerRadius = radius - halfWidth;" line here and just
moving the creation and addCoverEffect of the CircleEdgeEffect down?

It seems like the order this was done in before this patch was wrong.

[jvanverth1, 2013/08/29 18:16:53]

Yes, that's why I moved it down.

-            isStroked = (innerRadius > 0);
         }
     }
 
+    GrEffectRef* effect = CircleEdgeEffect::Create(isStroked && innerRadius > 0);
+    static const int kCircleEdgeAttrIndex = 1;
+    drawState->addCoverageEffect(effect, kCircleEdgeAttrIndex)->unref();
+
     // The radii are outset for two reasons. First, it allows the shader to simply perform
     // clamp(distance-to-center - radius, 0, 1). Second, the outer radius is used to compute the
     // verts of the bounding box that is rendered and the outset ensures the box will cover all
     // pixels partially covered by the circle.
     outerRadius += SK_ScalarHalf;
     innerRadius -= SK_ScalarHalf;
 
     SkRect bounds = SkRect::MakeLTRB(
         center.fX - outerRadius,
         center.fY - outerRadius,
@@ skipping 89 matching lines @@
         // we don't handle it if curvature of the stroke is less than curvature of the ellipse
         if (scaledStroke.fX*(yRadius*yRadius) < (scaledStroke.fY*scaledStroke.fY)*xRadius ||
             scaledStroke.fY*(xRadius*xRadius) < (scaledStroke.fX*scaledStroke.fX)*yRadius) {
             return false;
         }
 
         // this is legit only if scale & translation (which should be the case at the moment)
         if (isStroked) {
             innerXRadius = xRadius - scaledStroke.fX;
             innerYRadius = yRadius - scaledStroke.fY;
-            isStroked = (innerXRadius > 0 && innerYRadius > 0);
         }
 
         xRadius += scaledStroke.fX;
         yRadius += scaledStroke.fY;
     }
 
     GrDrawState::AutoViewMatrixRestore avmr;
     if (!avmr.setIdentity(drawState)) {
         return false;
     }
 
     drawState->setVertexAttribs<gEllipseVertexAttribs>(SK_ARRAY_COUNT(gEllipseVertexAttribs));
     SkASSERT(sizeof(EllipseVertex) == drawState->getVertexSize());
 
     GrDrawTarget::AutoReleaseGeometry geo(target, 4, 0);
     if (!geo.succeeded()) {
         GrPrintf("Failed to get space for vertices!\n");
         return false;
     }
 
     EllipseVertex* verts = reinterpret_cast<EllipseVertex*>(geo.vertices());
 
-    GrEffectRef* effect = EllipseEdgeEffect::Create(isStroked);
+    GrEffectRef* effect = EllipseEdgeEffect::Create(isStroked &&
+                                                    innerXRadius > 0 && innerYRadius > 0);
 
     static const int kEllipseCenterAttrIndex = 1;
     static const int kEllipseEdgeAttrIndex = 2;
     drawState->addCoverageEffect(effect, kEllipseCenterAttrIndex, kEllipseEdgeAttrIndex)->unref();
 
     // Compute the reciprocals of the radii here to save time in the shader
     SkScalar xRadRecip = SkScalarInvert(xRadius);
     SkScalar yRadRecip = SkScalarInvert(yRadius);
     SkScalar xInnerRadRecip = SkScalarInvert(innerXRadius);
     SkScalar yInnerRadRecip = SkScalarInvert(innerYRadius);
@@ skipping 150 matching lines @@
         SkScalar halfWidth = 0;
         if (style != SkStrokeRec::kFill_Style) {
             if (SkScalarNearlyZero(scaledStroke.fX)) {
                 halfWidth = SK_ScalarHalf;
             } else {
                 halfWidth = SkScalarHalf(scaledStroke.fX);
             }
 
             if (isStroked) {
                 innerRadius = xRadius - halfWidth;
-                isStroked = (innerRadius > 0);
             }
             outerRadius += halfWidth;
             bounds.outset(halfWidth, halfWidth);
         }
 
-        GrEffectRef* effect = CircleEdgeEffect::Create(isStroked);
+        GrEffectRef* effect = CircleEdgeEffect::Create(isStroked && innerRadius > 0);
         static const int kCircleEdgeAttrIndex = 1;
         drawState->addCoverageEffect(effect, kCircleEdgeAttrIndex)->unref();
 
         // The radii are outset for two reasons. First, it allows the shader to simply perform
         // clamp(distance-to-center - radius, 0, 1). Second, the outer radius is used to compute the
         // verts of the bounding box that is rendered and the outset ensures the box will cover all
         // pixels partially covered by the circle.
         outerRadius += SK_ScalarHalf;
         innerRadius -= SK_ScalarHalf;
 
@@ skipping 66 matching lines @@
             // we don't handle it if curvature of the stroke is less than curvature of the ellipse
             if (scaledStroke.fX*(yRadius*yRadius) < (scaledStroke.fY*scaledStroke.fY)*xRadius ||
                 scaledStroke.fY*(xRadius*xRadius) < (scaledStroke.fX*scaledStroke.fX)*yRadius) {
                 return false;
             }
 
             // this is legit only if scale & translation (which should be the case at the moment)
             if (isStroked) {
                 innerXRadius = xRadius - scaledStroke.fX;
                 innerYRadius = yRadius - scaledStroke.fY;
-                isStroked = (innerXRadius > 0 && innerYRadius > 0);
             }
 
             xRadius += scaledStroke.fX;
             yRadius += scaledStroke.fY;
             bounds.outset(scaledStroke.fX, scaledStroke.fY);
         }
 
         GrDrawTarget::AutoReleaseGeometry geo(target, 16, 0);
         if (!geo.succeeded()) {
             GrPrintf("Failed to get space for vertices!\n");
             return false;
         }
         EllipseVertex* verts = reinterpret_cast<EllipseVertex*>(geo.vertices());
 
-        GrEffectRef* effect = EllipseEdgeEffect::Create(isStroked);
+        GrEffectRef* effect = EllipseEdgeEffect::Create(isStroked &&
+                                                        innerXRadius > 0 && innerYRadius > 0);
         static const int kEllipseOffsetAttrIndex = 1;
         static const int kEllipseRadiiAttrIndex = 2;
         drawState->addCoverageEffect(effect,
                                      kEllipseOffsetAttrIndex, kEllipseRadiiAttrIndex)->unref();
 
         // Compute the reciprocals of the radii here to save time in the shader
         SkScalar xRadRecip = SkScalarInvert(xRadius);
         SkScalar yRadRecip = SkScalarInvert(yRadius);
         SkScalar xInnerRadRecip = SkScalarInvert(innerXRadius);
         SkScalar yInnerRadRecip = SkScalarInvert(innerYRadius);
@@ skipping 37 matching lines @@
             verts->fInnerRadii = SkPoint::Make(xInnerRadRecip, yInnerRadRecip);
             verts++;
 
             verts->fPos = SkPoint::Make(bounds.fRight, yCoords[i]);
             verts->fOffset = SkPoint::Make(xOuterRadius, yOuterOffsets[i]);
             verts->fOuterRadii = SkPoint::Make(xRadRecip, yRadRecip);
             verts->fInnerRadii = SkPoint::Make(xInnerRadRecip, yInnerRadRecip);
             verts++;
         }
 
         // drop out the middle quad if we're stroked

[robertphillips, 2013/08/29 17:01:14]

It seems like this won't behave exactly the same? Is that okay?

[jvanverth1, 2013/08/29 18:16:53]

Nope, not okay. Good catch.

         int indexCnt = isStroked ? GR_ARRAY_COUNT(gRRectIndices)-6 : GR_ARRAY_COUNT(gRRectIndices);
         target->setIndexSourceToBuffer(indexBuffer);
         target->drawIndexed(kTriangles_GrPrimitiveType, 0, 0, 16, indexCnt, &bounds);
     }
 
     return true;
 }