fix addOval and addRRect on builder

Need to notify the underlying pathref if we've made oval or rrect

Bug: skia:9000
Change-Id: I57a801f1fb446b99634d7b028249a812a5a978f1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307516
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
This commit is contained in:
Mike Reed 2020-08-02 22:14:43 -04:00 committed by Skia Commit-Bot
parent a6df9da289
commit 74a7a81b98
5 changed files with 86 additions and 22 deletions

View File

@ -244,16 +244,15 @@ DEF_GM(return new ComboPathEfectsGM;)
DEF_SIMPLE_GM(stroke_and_fill_patheffect, canvas, 900, 450) {
const float kStrokeWidth = 20;
typedef void (*Maker)(SkPath*);
typedef SkPath (*Maker)();
const Maker makers[] = {
[](SkPath* path) {
path->addOval({0, 0, 100, 100}, SkPathDirection::kCW);
},
[](SkPath* path) {
path->addOval({0, 0, 100, 100}, SkPathDirection::kCCW);
},
[](SkPath* path) {
path->moveTo(0, 0).lineTo(100, 100).lineTo(0, 100).lineTo(100, 0).close();
[]() { return SkPath::Oval({0, 0, 100, 100}, SkPathDirection::kCW); },
[]() { return SkPath::Oval({0, 0, 100, 100}, SkPathDirection::kCCW); },
[]() {
const SkPoint pts[] = {
{0, 0}, {100, 100}, {0, 100}, {100, 0},
};
return SkPath::Polygon(pts, SK_ARRAY_COUNT(pts), true);
},
};
@ -274,9 +273,7 @@ DEF_SIMPLE_GM(stroke_and_fill_patheffect, canvas, 900, 450) {
SkPaint paint;
canvas->translate(20, 20);
for (auto maker : makers) {
SkPath path;
maker(&path);
const SkPath path = maker();
canvas->save();
for (const auto& r : rec) {
paint.setStyle(r.fStyle);

View File

@ -114,13 +114,27 @@ private:
SkPoint fLastMovePoint;
bool fNeedsMoveVerb;
enum IsA {
kIsA_JustMoves, // we only have 0 or more moves
kIsA_MoreThanMoves, // we have verbs other than just move
kIsA_Oval, // we are 0 or more moves followed by an oval
kIsA_RRect, // we are 0 or more moves followed by a rrect
};
IsA fIsA = kIsA_JustMoves;
int fIsAStart = -1; // tracks direction iff fIsA is not unknown
bool fIsACCW = false; // tracks direction iff fIsA is not unknown
int countVerbs() const { return fVerbs.count(); }
// called right before we add a (non-move) verb
void ensureMove() {
fIsA = kIsA_MoreThanMoves;
if (fNeedsMoveVerb) {
this->moveTo(fLastMovePoint);
}
}
SkPath make(sk_sp<SkPathRef>) const;
};
#endif

View File

@ -505,6 +505,7 @@ private:
friend class PathRefTest_Private;
friend class ForceIsRRect_Private; // unit test isRRect
friend class SkPath;
friend class SkPathBuilder;
friend class SkPathPriv;
};

View File

@ -141,17 +141,27 @@ SkPathBuilder& SkPathBuilder::rCubicTo(SkPoint p1, SkPoint p2, SkPoint p3) {
///////////////////////////////////////////////////////////////////////////////////////////
SkPath SkPathBuilder::make(sk_sp<SkPathRef> pr) const {
switch (fIsA) {
case kIsA_Oval: pr->setIsOval( true, fIsACCW, fIsAStart); break;
case kIsA_RRect: pr->setIsRRect(true, fIsACCW, fIsAStart); break;
default: break;
}
return SkPath(std::move(pr), fFillType, fIsVolatile);
}
SkPath SkPathBuilder::snapshot() {
return SkPath(sk_sp<SkPathRef>(new SkPathRef(fPts, fVerbs, fConicWeights, fSegmentMask)),
fFillType, fIsVolatile);
return this->make(sk_sp<SkPathRef>(new SkPathRef(fPts,
fVerbs,
fConicWeights,
fSegmentMask)));
}
SkPath SkPathBuilder::detach() {
auto path = SkPath(sk_sp<SkPathRef>(new SkPathRef(std::move(fPts),
std::move(fVerbs),
std::move(fConicWeights),
fSegmentMask)),
fFillType, fIsVolatile);
auto path = this->make(sk_sp<SkPathRef>(new SkPathRef(std::move(fPts),
std::move(fVerbs),
std::move(fConicWeights),
fSegmentMask)));
this->reset();
return path;
}
@ -250,6 +260,8 @@ SkPathBuilder& SkPathBuilder::addRect(const SkRect& rect, SkPathDirection dir, u
}
SkPathBuilder& SkPathBuilder::addOval(const SkRect& oval, SkPathDirection dir, unsigned index) {
const IsA prevIsA = fIsA;
const int kPts = 9; // moveTo + 4 conics(2 pts each)
const int kVerbs = 6; // moveTo + 4 conics + close
this->incReserve(kPts, kVerbs);
@ -263,10 +275,18 @@ SkPathBuilder& SkPathBuilder::addOval(const SkRect& oval, SkPathDirection dir, u
for (unsigned i = 0; i < 4; ++i) {
this->conicTo(rectIter.next(), ovalIter.next(), SK_ScalarRoot2Over2);
}
return this->close();
this->close();
if (prevIsA == kIsA_JustMoves) {
fIsA = kIsA_Oval;
fIsACCW = (dir == SkPathDirection::kCCW);
fIsAStart = index % 4;
}
return *this;
}
SkPathBuilder& SkPathBuilder::addRRect(const SkRRect& rrect, SkPathDirection dir, unsigned index) {
const IsA prevIsA = fIsA;
const SkRect& bounds = rrect.getBounds();
if (rrect.isRect() || rrect.isEmpty()) {
@ -307,6 +327,12 @@ SkPathBuilder& SkPathBuilder::addRRect(const SkRRect& rrect, SkPathDirection dir
}
this->close();
}
if (prevIsA == kIsA_JustMoves) {
fIsA = kIsA_RRect;
fIsACCW = (dir == SkPathDirection::kCCW);
fIsAStart = index % 8;
}
return *this;
}

View File

@ -115,6 +115,7 @@ DEF_TEST(pathbuilder_addRect, reporter) {
DEF_TEST(pathbuilder_addOval, reporter) {
const SkRect r = { 10, 20, 30, 40 };
SkRect tmp;
for (auto dir : {SkPathDirection::kCW, SkPathDirection::kCCW}) {
for (int i = 0; i < 4; ++i) {
@ -122,19 +123,30 @@ DEF_TEST(pathbuilder_addOval, reporter) {
SkPath p;
p.addOval(r, dir, i);
REPORTER_ASSERT(reporter, p == bp);
REPORTER_ASSERT(reporter, p.isOval(&tmp) && (tmp == r));
REPORTER_ASSERT(reporter, bp.isOval(&tmp) && (tmp == r));
}
auto bp = SkPathBuilder().addOval(r, dir).detach();
SkPath p;
p.addOval(r, dir);
REPORTER_ASSERT(reporter, p == bp);
REPORTER_ASSERT(reporter, p.isOval(&tmp) && (tmp == r));
REPORTER_ASSERT(reporter, bp.isOval(&tmp) && (tmp == r));
// test negative case -- can't have any other segments
bp = SkPathBuilder().addOval(r, dir).lineTo(10, 10).detach();
REPORTER_ASSERT(reporter, !bp.isOval(&tmp));
bp = SkPathBuilder().lineTo(10, 10).addOval(r, dir).detach();
REPORTER_ASSERT(reporter, !bp.isOval(&tmp));
}
}
DEF_TEST(pathbuilder_addRRect, reporter) {
const SkRRect rr = SkRRect::MakeRectXY({ 10, 20, 30, 40 }, 5, 6);
SkRRect tmp;
for (int i = 0; i < 4; ++i) {
for (auto dir : {SkPathDirection::kCW, SkPathDirection::kCCW}) {
for (auto dir : {SkPathDirection::kCW, SkPathDirection::kCCW}) {
for (int i = 0; i < 4; ++i) {
SkPathBuilder b;
b.addRRect(rr, dir, i);
auto bp = b.detach();
@ -142,7 +154,21 @@ DEF_TEST(pathbuilder_addRRect, reporter) {
SkPath p;
p.addRRect(rr, dir, i);
REPORTER_ASSERT(reporter, p == bp);
REPORTER_ASSERT(reporter, p.isRRect(&tmp) && (tmp == rr));
REPORTER_ASSERT(reporter, bp.isRRect(&tmp) && (tmp == rr));
}
auto bp = SkPathBuilder().addRRect(rr, dir).detach();
SkPath p;
p.addRRect(rr, dir);
REPORTER_ASSERT(reporter, p == bp);
REPORTER_ASSERT(reporter, p.isRRect(&tmp) && (tmp == rr));
REPORTER_ASSERT(reporter, bp.isRRect(&tmp) && (tmp == rr));
// test negative case -- can't have any other segments
bp = SkPathBuilder().addRRect(rr, dir).lineTo(10, 10).detach();
REPORTER_ASSERT(reporter, !bp.isRRect(&tmp));
bp = SkPathBuilder().lineTo(10, 10).addRRect(rr, dir).detach();
REPORTER_ASSERT(reporter, !bp.isRRect(&tmp));
}
}