Fix key computation for GrPaths

Improve tests to ensure paths are receiving valid keys
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2342873002

Review-Url: https://codereview.chromium.org/2342873002
This commit is contained in:
bsalomon 2016-09-15 13:55:33 -07:00 committed by Commit bot
parent fe6ca0f798
commit 7bffcd2673
4 changed files with 109 additions and 108 deletions

View File

@ -6,9 +6,8 @@
*/
#include "GrPath.h"
#include "GrStyle.h"
#include "GrShape.h"
namespace {
// Verb count limit for generating path key from content of a volatile path.
// The value should accomodate at least simple rects and rrects.
static const int kSimpleVolatilePathVerbLimit = 10;
@ -26,58 +25,14 @@ static inline void write_style_key(uint32_t* dst, const GrStyle& style) {
GrStyle::WriteKey(dst, style, GrStyle::Apply::kPathEffectAndStrokeRec, SK_Scalar1);
}
inline static bool compute_key_for_line_path(const SkPath& path, const GrStyle& style,
GrUniqueKey* key) {
SkPoint pts[2];
if (!path.isLine(pts)) {
return false;
}
static_assert((sizeof(pts) % sizeof(uint32_t)) == 0 && sizeof(pts) > sizeof(uint32_t),
"pts_needs_padding");
int styleDataCnt = style_data_cnt(style);
const int kBaseData32Cnt = 1 + sizeof(pts) / sizeof(uint32_t);
static const GrUniqueKey::Domain kOvalPathDomain = GrUniqueKey::GenerateDomain();
GrUniqueKey::Builder builder(key, kOvalPathDomain, kBaseData32Cnt + styleDataCnt);
builder[0] = path.getFillType();
memcpy(&builder[1], &pts, sizeof(pts));
if (styleDataCnt > 0) {
write_style_key(&builder[kBaseData32Cnt], style);
}
return true;
}
inline static bool compute_key_for_oval_path(const SkPath& path, const GrStyle& style,
GrUniqueKey* key) {
SkRect rect;
// Point order is significant when dashing, so we cannot devolve to a rect key.
if (style.pathEffect() || !path.isOval(&rect)) {
return false;
}
static_assert((sizeof(rect) % sizeof(uint32_t)) == 0 && sizeof(rect) > sizeof(uint32_t),
"rect_needs_padding");
const int kBaseData32Cnt = 1 + sizeof(rect) / sizeof(uint32_t);
int styleDataCnt = style_data_cnt(style);
static const GrUniqueKey::Domain kOvalPathDomain = GrUniqueKey::GenerateDomain();
GrUniqueKey::Builder builder(key, kOvalPathDomain, kBaseData32Cnt + styleDataCnt);
builder[0] = path.getFillType();
memcpy(&builder[1], &rect, sizeof(rect));
if (styleDataCnt > 0) {
write_style_key(&builder[kBaseData32Cnt], style);
}
return true;
}
// Encodes the full path data to the unique key for very small, volatile paths. This is typically
// hit when clipping stencils the clip stack. Intention is that this handles rects too, since
// SkPath::isRect seems to do non-trivial amount of work.
inline static bool compute_key_for_simple_path(const SkPath& path, const GrStyle& style,
GrUniqueKey* key) {
if (!path.isVolatile()) {
// Encodes the full path data to the unique key for very small paths that wouldn't otherwise have a
// key. This is typically hit when clipping stencils the clip stack.
inline static bool compute_key_for_simple_path(const GrShape& shape, GrUniqueKey* key) {
if (shape.hasUnstyledKey()) {
return false;
}
SkPath path;
shape.asPath(&path);
// The check below should take care of negative values casted positive.
const int verbCnt = path.countVerbs();
if (verbCnt > kSimpleVolatilePathVerbLimit) {
@ -124,7 +79,7 @@ inline static bool compute_key_for_simple_path(const SkPath& path, const GrStyle
// 2) stroke data (varying size)
const int baseData32Cnt = 2 + verbData32Cnt + pointData32Cnt + conicWeightData32Cnt;
const int styleDataCnt = style_data_cnt(style);
const int styleDataCnt = style_data_cnt(shape.style());
static const GrUniqueKey::Domain kSimpleVolatilePathDomain = GrUniqueKey::GenerateDomain();
GrUniqueKey::Builder builder(key, kSimpleVolatilePathDomain, baseData32Cnt + styleDataCnt);
int i = 0;
@ -169,45 +124,33 @@ inline static bool compute_key_for_simple_path(const SkPath& path, const GrStyle
}
SkASSERT(i == baseData32Cnt);
if (styleDataCnt > 0) {
write_style_key(&builder[baseData32Cnt], style);
write_style_key(&builder[baseData32Cnt], shape.style());
}
return true;
}
inline static void compute_key_for_general_path(const SkPath& path, const GrStyle& style,
GrUniqueKey* key) {
const int kBaseData32Cnt = 2;
int styleDataCnt = style_data_cnt(style);
inline static bool compute_key_for_general_shape(const GrShape& shape, GrUniqueKey* key) {
int geoCnt = shape.unstyledKeySize();
int styleCnt = style_data_cnt(shape.style());
if (styleCnt < 0 || geoCnt < 0) {
return false;
}
static const GrUniqueKey::Domain kGeneralPathDomain = GrUniqueKey::GenerateDomain();
GrUniqueKey::Builder builder(key, kGeneralPathDomain, kBaseData32Cnt + styleDataCnt);
builder[0] = path.getGenerationID();
builder[1] = path.getFillType();
if (styleDataCnt > 0) {
write_style_key(&builder[kBaseData32Cnt], style);
GrUniqueKey::Builder builder(key, kGeneralPathDomain, geoCnt + styleCnt);
shape.writeUnstyledKey(&builder[0]);
if (styleCnt) {
write_style_key(&builder[geoCnt], shape.style());
}
return true;
}
}
void GrPath::ComputeKey(const GrShape& shape, GrUniqueKey* key, bool* outIsVolatile) {
void GrPath::ComputeKey(const SkPath& path, const GrStyle& style, GrUniqueKey* key,
bool* outIsVolatile) {
if (compute_key_for_line_path(path, style, key)) {
if (compute_key_for_simple_path(shape, key)) {
*outIsVolatile = false;
return;
}
if (compute_key_for_oval_path(path, style, key)) {
*outIsVolatile = false;
return;
}
if (compute_key_for_simple_path(path, style, key)) {
*outIsVolatile = false;
return;
}
compute_key_for_general_path(path, style, key);
*outIsVolatile = path.isVolatile();
*outIsVolatile = !compute_key_for_general_shape(shape, key);
}
#ifdef SK_DEBUG

View File

@ -14,6 +14,8 @@
#include "SkPath.h"
#include "SkRect.h"
class GrShape;
class GrPath : public GrGpuResource {
public:
/**
@ -30,8 +32,7 @@ public:
{
}
static void ComputeKey(const SkPath& path, const GrStyle& style, GrUniqueKey* key,
bool* outIsVolatile);
static void ComputeKey(const GrShape&, GrUniqueKey* key, bool* outIsVolatile);
const SkRect& getBounds() const { return fBounds; }

View File

@ -51,20 +51,28 @@ bool GrStencilAndCoverPathRenderer::onCanDrawPath(const CanDrawPathArgs& args) c
}
}
static GrPath* get_gr_path(GrResourceProvider* resourceProvider, const SkPath& skPath,
const GrStyle& style) {
static GrPath* get_gr_path(GrResourceProvider* resourceProvider, const GrShape& shape) {
GrUniqueKey key;
bool isVolatile;
GrPath::ComputeKey(skPath, style, &key, &isVolatile);
SkAutoTUnref<GrPath> path(
static_cast<GrPath*>(resourceProvider->findAndRefResourceByUniqueKey(key)));
GrPath::ComputeKey(shape, &key, &isVolatile);
sk_sp<GrPath> path;
if (!isVolatile) {
path.reset(
static_cast<GrPath*>(resourceProvider->findAndRefResourceByUniqueKey(key)));
}
if (!path) {
path.reset(resourceProvider->createPath(skPath, style));
SkPath skPath;
shape.asPath(&skPath);
path.reset(resourceProvider->createPath(skPath, shape.style()));
if (!isVolatile) {
resourceProvider->assignUniqueKeyToResource(key, path);
resourceProvider->assignUniqueKeyToResource(key, path.get());
}
} else {
SkASSERT(path->isEqualTo(skPath, style));
#ifdef SK_DEBUG
SkPath skPath;
shape.asPath(&skPath);
SkASSERT(path->isEqualTo(skPath, shape.style()));
#endif
}
return path.release();
}
@ -73,10 +81,8 @@ void GrStencilAndCoverPathRenderer::onStencilPath(const StencilPathArgs& args) {
GR_AUDIT_TRAIL_AUTO_FRAME(args.fDrawContext->auditTrail(),
"GrStencilAndCoverPathRenderer::onStencilPath");
SkASSERT(!args.fIsAA || args.fDrawContext->isStencilBufferMultisampled());
SkPath path;
args.fShape->asPath(&path);
SkAutoTUnref<GrPath> p(get_gr_path(fResourceProvider, path, GrStyle::SimpleFill()));
SkAutoTUnref<GrPath> p(get_gr_path(fResourceProvider, *args.fShape));
args.fDrawContext->drawContextPriv().stencilPath(*args.fClip, args.fIsAA, *args.fViewMatrix, p);
}
@ -91,7 +97,7 @@ bool GrStencilAndCoverPathRenderer::onDrawPath(const DrawPathArgs& args) {
SkPath path;
args.fShape->asPath(&path);
SkAutoTUnref<GrPath> p(get_gr_path(fResourceProvider, path, args.fShape->style()));
SkAutoTUnref<GrPath> p(get_gr_path(fResourceProvider, *args.fShape));
if (path.isInverseFillType()) {
SkMatrix invert = SkMatrix::I();

View File

@ -11,7 +11,7 @@
#include "GrContext.h"
#include "GrPath.h"
#include "GrStyle.h"
#include "GrShape.h"
#include "SkBitmap.h"
#include "SkCanvas.h"
#include "SkColor.h"
@ -92,21 +92,72 @@ DEF_GPUTEST_FOR_ALL_GL_CONTEXTS(GpuDrawPath, reporter, ctxInfo) {
}
DEF_GPUTEST(GrPathKeys, reporter, /*factory*/) {
// 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);
SkPaint strokePaint;
strokePaint.setStyle(SkPaint::kStroke_Style);
strokePaint.setStrokeWidth(10.f);
GrStyle styles[] = {
GrStyle::SimpleFill(),
GrStyle::SimpleHairline(),
GrStyle(strokePaint)
};
path1.conicTo(p0, p1, .5f);
path2.conicTo(p0, p1, .7f);
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);
bool isVolatile;
GrUniqueKey key1, key2;
GrPath::ComputeKey(path1, GrStyle::SimpleFill(), &key1, &isVolatile);
GrPath::ComputeKey(path2, GrStyle::SimpleFill(), &key2, &isVolatile);
REPORTER_ASSERT(reporter, key1 != key2);
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.
GrPath::ComputeKey(GrShape(path1, GrStyle::SimpleFill()), &key1, &isVolatile);
REPORTER_ASSERT(reporter, !isVolatile);
REPORTER_ASSERT(reporter, key1.isValid());
GrPath::ComputeKey(GrShape(path2, GrStyle::SimpleFill()), &key2, &isVolatile);
REPORTER_ASSERT(reporter, !isVolatile);
REPORTER_ASSERT(reporter, key1.isValid());
REPORTER_ASSERT(reporter, key1 != key2);
// 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);
}
// Try a large path that is too big to be keyed off its data.
SkPath path3;
SkPath path4;
for (int i = 0; i < 1000; ++i) {
SkScalar s = SkIntToScalar(i);
path3.conicTo(s, 3.f * s / 4, s + 1.f, s, 0.5f + s / 2000.f);
path4.conicTo(s, 3.f * s / 4, s + 1.f, s, 0.3f + s / 2000.f);
}
GrUniqueKey key3, key4;
// These aren't marked volatile and so should have keys
GrPath::ComputeKey(GrShape(path3, style), &key3, &isVolatile);
REPORTER_ASSERT(reporter, !isVolatile);
REPORTER_ASSERT(reporter, key3.isValid());
GrPath::ComputeKey(GrShape(path4, style), &key4, &isVolatile);
REPORTER_ASSERT(reporter, !isVolatile);
REPORTER_ASSERT(reporter, key4.isValid());
REPORTER_ASSERT(reporter, key3 != key4);
{
GrUniqueKey tempKey;
path3.setIsVolatile(true);
GrPath::ComputeKey(GrShape(path3, style), &key1, &isVolatile);
REPORTER_ASSERT(reporter, isVolatile);
REPORTER_ASSERT(reporter, !tempKey.isValid());
}
}
}
#endif