AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Don't compute path keys for volatile paths in GrShape.

Browse Source 
Otherwise, we will compute cache keys for internally transformed paths that don't repeat (e.g. clip paths transformed into device space with a changing view matrix).

BUG=chromium:649562
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2369513002

Review-Url: https://codereview.chromium.org/2369513002
This commit is contained in:
bsalomon 2016-09-23 12:09:16 -07:00 committed by Commit bot
parent bf41fa841b
commit aa840647fc
3 changed files with 34 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/gpu/GrShape.cpp
View File

@ -132,13 +132,13 @@ int GrShape::unstyledKeySize() const {
// 4 for the end points and 1 for the inverseness
return 5;
case Type::kPath: {
if (0 == fPathData.fGenID) {
return -1;
}
int dataKeySize = path_key_from_data_size(fPathData.fPath);
if (dataKeySize >= 0) {
return dataKeySize;
}
if (0 == fPathData.fGenID) {
return -1;
}
// The key is the path ID and fill type.
return 2;
}
@ -172,12 +172,12 @@ void GrShape::writeUnstyledKey(uint32_t* key) const {
*key++ = fLineData.fInverted ? 1 : 0;
break;
case Type::kPath: {
SkASSERT(fPathData.fGenID);
int dataKeySize = path_key_from_data_size(fPathData.fPath);
if (dataKeySize >= 0) {
write_path_key_from_data(fPathData.fPath, key);
return;
}
SkASSERT(fPathData.fGenID);
*key++ = fPathData.fGenID;
// We could canonicalize the fill rule for paths that don't differentiate between
// even/odd or winding fill (e.g. convex).

18
tests/GpuDrawPathTest.cpp
View File

@ -104,18 +104,15 @@ DEF_GPUTEST(GrPathKeys, reporter, /*factory*/) {
for (const GrStyle& style : styles) {
// Keys should not ignore conic weights.
SkPath path1, path2;
path1.setIsVolatile(true);
path2.setIsVolatile(true);
SkPoint p0 = SkPoint::Make(100, 0);
SkPoint p1 = SkPoint::Make(100, 100);
path1.conicTo(p0, p1, .5f);
path2.conicTo(p0, p1, .7f);
bool isVolatile;
GrUniqueKey key1, key2;
// Even though the paths are marked volatile, they should have keys based on the path data
// because they have a small amount of data.
// We expect these small paths to be keyed based on their data.
bool isVolatile;
GrPath::ComputeKey(GrShape(path1, GrStyle::SimpleFill()), &key1, &isVolatile);
REPORTER_ASSERT(reporter, !isVolatile);
REPORTER_ASSERT(reporter, key1.isValid());
@ -123,12 +120,19 @@ DEF_GPUTEST(GrPathKeys, reporter, /*factory*/) {
REPORTER_ASSERT(reporter, !isVolatile);
REPORTER_ASSERT(reporter, key1.isValid());
REPORTER_ASSERT(reporter, key1 != key2);
{
GrUniqueKey tempKey;
path1.setIsVolatile(true);
GrPath::ComputeKey(GrShape(path1, style), &key1, &isVolatile);
REPORTER_ASSERT(reporter, isVolatile);
REPORTER_ASSERT(reporter, !tempKey.isValid());
}
// Ensure that recreating the GrShape doesn't change the key.
{
GrUniqueKey tempKey;
GrPath::ComputeKey(GrShape(path1, GrStyle::SimpleFill()), &tempKey, &isVolatile);
REPORTER_ASSERT(reporter, key1 == tempKey);
GrPath::ComputeKey(GrShape(path2, GrStyle::SimpleFill()), &tempKey, &isVolatile);
REPORTER_ASSERT(reporter, key2 == tempKey);
}
// Try a large path that is too big to be keyed off its data.

44
tests/GrShapeTest.cpp
View File

@ -1161,14 +1161,8 @@ void test_make_hairline_path_effect(skiatest::Reporter* reporter, const Geo& geo
// If the resulting path is small enough then it will have a key.
REPORTER_ASSERT(reporter, paths_fill_same(a, b));
REPORTER_ASSERT(reporter, paths_fill_same(a, c));
if (c.countVerbs() <= GrShape::kMaxKeyFromDataVerbCnt) {
REPORTER_ASSERT(reporter, !peCase.appliedPathEffectKey().empty());
REPORTER_ASSERT(reporter, peCase.appliedPathEffectKey() ==
peCase.appliedFullStyleKey());
} else {
REPORTER_ASSERT(reporter, peCase.appliedPathEffectKey().empty());
REPORTER_ASSERT(reporter, peCase.appliedFullStyleKey().empty());
}
REPORTER_ASSERT(reporter, peCase.appliedPathEffectKey().empty());
REPORTER_ASSERT(reporter, peCase.appliedFullStyleKey().empty());
}
REPORTER_ASSERT(reporter, peCase.appliedPathEffectShape().style().isSimpleHairline());
REPORTER_ASSERT(reporter, peCase.appliedFullStyleShape().style().isSimpleHairline());
@ -1184,8 +1178,8 @@ void test_volatile_path(skiatest::Reporter* reporter, const Geo& geo) {
dashAndStroke.setStyle(SkPaint::kStroke_Style);
TestCase volatileCase(reporter, vPath, dashAndStroke);
// We expect a shape made from a volatile path to have a key iff the shape is recognized
// as a specialized geometry or it has a small verb count.
if (geo.isNonPath(dashAndStroke) || vPath.countVerbs() <= GrShape::kMaxKeyFromDataVerbCnt) {
// as a specialized geometry.
if (geo.isNonPath(dashAndStroke)) {
REPORTER_ASSERT(reporter, SkToBool(volatileCase.baseKey().count()));
// In this case all the keys should be identical to the non-volatile case.
TestCase nonVolatileCase(reporter, geo.path(), dashAndStroke);
@ -1788,19 +1782,19 @@ static void test_short_path_keys(skiatest::Reporter* r) {
paints[3].setStyle(SkPaint::kStrokeAndFill_Style);
paints[3].setStrokeWidth(5.f);
auto compare = [r, &paints] (SkPath* pathA, SkPath* pathB,
auto compare = [r, &paints] (const SkPath& pathA, const SkPath& pathB,
TestCase::ComparisonExpecation expectation) {
SkPath volatileA = pathA;
SkPath volatileB = pathB;
volatileA.setIsVolatile(true);
volatileB.setIsVolatile(true);
for (const SkPaint& paint : paints) {
REPORTER_ASSERT(r, !GrShape(volatileA, paint).hasUnstyledKey());
REPORTER_ASSERT(r, !GrShape(volatileB, paint).hasUnstyledKey());
for (PathGeo::Invert invert : {PathGeo::Invert::kNo, PathGeo::Invert::kYes}) {
for (bool aIsVolatile : {false, true}) {
for (bool bIsVolatile : {false, true}) {
pathA->setIsVolatile(aIsVolatile);
pathB->setIsVolatile(bIsVolatile);
TestCase caseA(PathGeo(*pathA, invert), paint, r);
TestCase caseB(PathGeo(*pathB, invert), paint, r);
caseA.compare(r, caseB, expectation);
}
}
TestCase caseA(PathGeo(pathA, invert), paint, r);
TestCase caseB(PathGeo(pathB, invert), paint, r);
caseA.compare(r, caseB, expectation);
}
}
};
@ -1814,33 +1808,33 @@ static void test_short_path_keys(skiatest::Reporter* r) {
pathB.lineTo(10.f, 10.f);
pathB.conicTo(20.f, 20.f, 20.f, 30.f, 0.7f);
compare(&pathA, &pathB, TestCase::kAllSame_ComparisonExpecation);
compare(pathA, pathB, TestCase::kAllSame_ComparisonExpecation);
// Give path b a different point
pathB.reset();
pathB.lineTo(10.f, 10.f);
pathB.conicTo(21.f, 20.f, 20.f, 30.f, 0.7f);
compare(&pathA, &pathB, TestCase::kAllDifferent_ComparisonExpecation);
compare(pathA, pathB, TestCase::kAllDifferent_ComparisonExpecation);
// Give path b a different conic weight
pathB.reset();
pathB.lineTo(10.f, 10.f);
pathB.conicTo(20.f, 20.f, 20.f, 30.f, 0.6f);
compare(&pathA, &pathB, TestCase::kAllDifferent_ComparisonExpecation);
compare(pathA, pathB, TestCase::kAllDifferent_ComparisonExpecation);
// Give path b an extra lineTo verb
pathB.reset();
pathB.lineTo(10.f, 10.f);
pathB.conicTo(20.f, 20.f, 20.f, 30.f, 0.6f);
pathB.lineTo(50.f, 50.f);
compare(&pathA, &pathB, TestCase::kAllDifferent_ComparisonExpecation);
compare(pathA, pathB, TestCase::kAllDifferent_ComparisonExpecation);
// Give path b a close
pathB.reset();
pathB.lineTo(10.f, 10.f);
pathB.conicTo(20.f, 20.f, 20.f, 30.f, 0.7f);
pathB.close();
compare(&pathA, &pathB, TestCase::kAllDifferent_ComparisonExpecation);
compare(pathA, pathB, TestCase::kAllDifferent_ComparisonExpecation);
}
DEF_TEST(GrShape, reporter) {