Revert of Fix NVPR assert for equivalent ovals (patchset #1 id:1 of https://codereview.chromium.org/1457073002/ )
Reason for revert: Causes failures on Android and Win8: ... ( 137/1245MB 9) 73.9ms unit test GpuLayerCachec:\0\build\slave\workdir\build\skia\include\private\skuniqueptr.h:164: failed assertion "get() != pointer()" Caught exception 2147483651 EXCEPTION_BREAKPOINT ... Original issue's description: > Fix NVPR assert for equivalent ovals > > For oval paths, GrPath ignores the point order and only uses the bounds > when building its key. This is problematic because > > 1) point order is important when dashing > 2) GrStencilAndCoverPathRenderer asserts that the lookup SkPath is equal > to the cached SkPath - which is not the case for ovals with different > directions/different point order. > > With this CL we no longer use the reduced oval key when dashing, and > instead fall through to the more general path cases. The assert is > adjusted to accommodate "equivalent" ovals (when not dashing). > > Also re-enabled & updated the GpuDrawPath unit test (disabled in > https://codereview.chromium.org/1456463003/, presumably due to the use > of uninitialized SkRects). > > R=bsalomon@google.com,robertphillips@google.com,cdalton@nvidia.com > > Committed: https://skia.googlesource.com/skia/+/f9b1577d763988ebc043ddabf80674f71571ecff TBR=bsalomon@google.com,cdalton@nvidia.com,robertphillips@google.com,fmalita@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Review URL: https://codereview.chromium.org/1461913002
This commit is contained in:
parent
5aaef1ff1a
commit
1ac3f40b43
@ -36,8 +36,7 @@ inline static bool compute_key_for_line_path(const SkPath& path, const GrStrokeI
|
||||
inline static bool compute_key_for_oval_path(const SkPath& path, const GrStrokeInfo& stroke,
|
||||
GrUniqueKey* key) {
|
||||
SkRect rect;
|
||||
// Point order is significant when dashing, so we cannot devolve to a rect key.
|
||||
if (stroke.isDashed() || !path.isOval(&rect)) {
|
||||
if (!path.isOval(&rect)) {
|
||||
return false;
|
||||
}
|
||||
static_assert((sizeof(rect) % sizeof(uint32_t)) == 0 && sizeof(rect) > sizeof(uint32_t),
|
||||
@ -172,20 +171,3 @@ void GrPath::ComputeKey(const SkPath& path, const GrStrokeInfo& stroke, GrUnique
|
||||
*outIsVolatile = path.isVolatile();
|
||||
}
|
||||
|
||||
#ifdef SK_DEBUG
|
||||
bool GrPath::isEqualTo(const SkPath& path, const GrStrokeInfo& stroke) const {
|
||||
if (!fStroke.hasEqualEffect(stroke)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// We treat same-rect ovals as identical - but only when not dashing.
|
||||
SkRect ovalBounds;
|
||||
if (!fStroke.isDashed() && fSkPath.isOval(&ovalBounds)) {
|
||||
SkRect otherOvalBounds;
|
||||
return path.isOval(&otherOvalBounds) && ovalBounds == otherOvalBounds;
|
||||
}
|
||||
|
||||
return fSkPath == path;
|
||||
}
|
||||
#endif
|
||||
|
||||
|
@ -36,7 +36,9 @@ public:
|
||||
const SkRect& getBounds() const { return fBounds; }
|
||||
|
||||
#ifdef SK_DEBUG
|
||||
bool isEqualTo(const SkPath& path, const GrStrokeInfo& stroke) const;
|
||||
bool isEqualTo(const SkPath& path, const GrStrokeInfo& stroke) {
|
||||
return fSkPath == path && fStroke.hasEqualEffect(stroke);
|
||||
}
|
||||
#endif
|
||||
|
||||
protected:
|
||||
|
@ -15,8 +15,6 @@
|
||||
#include "SkCanvas.h"
|
||||
#include "SkColor.h"
|
||||
#include "SkPaint.h"
|
||||
#include "SkPath.h"
|
||||
#include "SkDashPathEffect.h"
|
||||
#include "SkRRect.h"
|
||||
#include "SkRect.h"
|
||||
#include "SkSurface.h"
|
||||
@ -25,12 +23,11 @@
|
||||
static void test_drawPathEmpty(skiatest::Reporter*, SkCanvas* canvas) {
|
||||
// Filling an empty path should not crash.
|
||||
SkPaint paint;
|
||||
SkRect emptyRect = SkRect::MakeEmpty();
|
||||
canvas->drawRect(emptyRect, paint);
|
||||
canvas->drawRect(SkRect(), paint);
|
||||
canvas->drawPath(SkPath(), paint);
|
||||
canvas->drawOval(emptyRect, paint);
|
||||
canvas->drawRect(emptyRect, paint);
|
||||
canvas->drawRRect(SkRRect::MakeRect(emptyRect), paint);
|
||||
canvas->drawOval(SkRect(), paint);
|
||||
canvas->drawRect(SkRect(), paint);
|
||||
canvas->drawRRect(SkRRect(), paint);
|
||||
|
||||
// Stroking an empty path should not crash.
|
||||
paint.setAntiAlias(true);
|
||||
@ -38,43 +35,17 @@ static void test_drawPathEmpty(skiatest::Reporter*, SkCanvas* canvas) {
|
||||
paint.setColor(SK_ColorGRAY);
|
||||
paint.setStrokeWidth(SkIntToScalar(20));
|
||||
paint.setStrokeJoin(SkPaint::kRound_Join);
|
||||
canvas->drawRect(emptyRect, paint);
|
||||
canvas->drawRect(SkRect(), paint);
|
||||
canvas->drawPath(SkPath(), paint);
|
||||
canvas->drawOval(emptyRect, paint);
|
||||
canvas->drawRect(emptyRect, paint);
|
||||
canvas->drawRRect(SkRRect::MakeRect(emptyRect), paint);
|
||||
canvas->drawOval(SkRect(), paint);
|
||||
canvas->drawRect(SkRect(), paint);
|
||||
canvas->drawRRect(SkRRect(), paint);
|
||||
}
|
||||
|
||||
static void fill_and_stroke(SkCanvas* canvas, const SkPath& p1, const SkPath& p2,
|
||||
SkPathEffect* effect) {
|
||||
SkPaint paint;
|
||||
paint.setAntiAlias(true);
|
||||
paint.setPathEffect(effect);
|
||||
|
||||
canvas->drawPath(p1, paint);
|
||||
canvas->drawPath(p2, paint);
|
||||
|
||||
paint.setStyle(SkPaint::kStroke_Style);
|
||||
canvas->drawPath(p1, paint);
|
||||
canvas->drawPath(p2, paint);
|
||||
}
|
||||
|
||||
static void test_drawSameRectOvals(skiatest::Reporter*, SkCanvas* canvas) {
|
||||
// Drawing ovals with similar bounds but different points order should not crash.
|
||||
|
||||
SkPath oval1, oval2;
|
||||
const SkRect rect = SkRect::MakeWH(100, 50);
|
||||
oval1.addOval(rect, SkPath::kCW_Direction);
|
||||
oval2.addOval(rect, SkPath::kCCW_Direction);
|
||||
|
||||
fill_and_stroke(canvas, oval1, oval2, nullptr);
|
||||
|
||||
const SkScalar intervals[] = { 1, 1 };
|
||||
SkAutoTUnref<SkPathEffect> dashEffect(SkDashPathEffect::Create(intervals, 2, 0));
|
||||
fill_and_stroke(canvas, oval1, oval2, dashEffect);
|
||||
}
|
||||
|
||||
DEF_GPUTEST(GpuDrawPath, reporter, factory) {
|
||||
return;
|
||||
|
||||
for (int type = 0; type < GrContextFactory::kLastGLContextType; ++type) {
|
||||
GrContextFactory::GLContextType glType = static_cast<GrContextFactory::GLContextType>(type);
|
||||
|
||||
@ -91,7 +62,6 @@ DEF_GPUTEST(GpuDrawPath, reporter, factory) {
|
||||
SkSurface::NewRenderTarget(grContext, SkSurface::kNo_Budgeted, info,
|
||||
sampleCounts[i], nullptr));
|
||||
test_drawPathEmpty(reporter, surface->getCanvas());
|
||||
test_drawSameRectOvals(reporter, surface->getCanvas());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user