Add test that SWPathRenderer GenIDChange listeners get removed.
When a cached path mask texture is destroyed it should remove the
gen ID change listener on from the SkPathRef:
https://skia-review.googlesource.com/c/skia/+/272654
Change-Id: I4d1781e578b29b801b1b5b97ba5119ac321de73c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/273004
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
diff --git a/include/private/SkPathRef.h b/include/private/SkPathRef.h
index ab476cc..1b55115 100644
--- a/include/private/SkPathRef.h
+++ b/include/private/SkPathRef.h
@@ -328,6 +328,7 @@
};
void addGenIDChangeListener(sk_sp<GenIDChangeListener>); // Threadsafe.
+ int genIDChangeListenerCount(); // Threadsafe
bool isValid() const;
SkDEBUGCODE(void validate() const { SkASSERT(this->isValid()); } )
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 87163a5..272f82e 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -3653,3 +3653,7 @@
clippedPath->reset();
return true;
}
+
+int SkPathPriv::GenIDChangeListenersCount(const SkPath& path) {
+ return path.fPathRef->genIDChangeListenerCount();
+}
diff --git a/src/core/SkPathPriv.h b/src/core/SkPathPriv.h
index 4f76d4e..79ba8fb 100644
--- a/src/core/SkPathPriv.h
+++ b/src/core/SkPathPriv.h
@@ -328,6 +328,13 @@
* If no clipping is needed, returns false and "result" is left unchanged.
*/
static bool PerspectiveClip(const SkPath& src, const SkMatrix&, SkPath* result);
+
+ /**
+ * Gets the number of GenIDChangeListeners. If another thread has access to this path then
+ * this may be stale before return and only indicates that the count was the return value
+ * at some point during the execution of the function.
+ */
+ static int GenIDChangeListenersCount(const SkPath&);
};
// Lightweight variant of SkPath::Iter that only returns segments (e.g. lines/conics).
diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp
index f665625..2bffe8d 100644
--- a/src/core/SkPathRef.cpp
+++ b/src/core/SkPathRef.cpp
@@ -496,6 +496,11 @@
*fGenIDChangeListeners.append() = listener.release();
}
+int SkPathRef::genIDChangeListenerCount() {
+ SkAutoMutexExclusive lock(fGenIDChangeListenersMutex);
+ return fGenIDChangeListeners.count();
+}
+
// we need to be called *before* the genID gets changed or zerod
void SkPathRef::callGenIDChangeListeners() {
auto visit = [this]() {
diff --git a/tests/PathRendererCacheTests.cpp b/tests/PathRendererCacheTests.cpp
index 676b803..edadc34 100644
--- a/tests/PathRendererCacheTests.cpp
+++ b/tests/PathRendererCacheTests.cpp
@@ -33,7 +33,8 @@
const SkPath& path,
GrPathRenderer* pr,
GrAAType aaType,
- const GrStyle& style) {
+ const GrStyle& style,
+ float scaleX = 1.f) {
GrPaint paint;
paint.setXPFactory(GrPorterDuffXPFactory::Get(SkBlendMode::kSrc));
@@ -45,6 +46,7 @@
shape = shape.applyStyle(GrStyle::Apply::kPathEffectAndStrokeRec, 1.0f);
}
SkMatrix matrix = SkMatrix::I();
+ matrix.setScaleX(scaleX);
GrPathRenderer::DrawPathArgs args{ctx,
std::move(paint),
&GrUserStencilSettings::kUnused,
@@ -72,6 +74,7 @@
std::function<SkPath(void)> createPath,
std::function<GrPathRenderer*(GrContext*)> createPathRenderer,
int expected,
+ bool checkListeners,
GrAAType aaType = GrAAType::kNone,
GrStyle style = GrStyle(SkStrokeRec::kFill_InitStyle)) {
sk_sp<GrContext> ctx = GrContext::MakeMock(nullptr);
@@ -106,6 +109,24 @@
path.reset();
cache->purgeAsNeeded();
REPORTER_ASSERT(reporter, cache_non_scratch_resources_equals(cache, expected - 1));
+
+ if (!checkListeners) {
+ return;
+ }
+
+ // Test that purging the cache of masks also removes listeners from the path.
+ path = createPath();
+ REPORTER_ASSERT(reporter, SkPathPriv::GenIDChangeListenersCount(path) == 0);
+ for (int i = 0; i < 20; ++i) {
+ float scaleX = 1 + ((float)i + 1)/20.f;
+ draw_path(ctx.get(), rtc.get(), path, pathRenderer.get(), aaType, style, scaleX);
+ }
+ ctx->flush();
+ REPORTER_ASSERT(reporter, SkPathPriv::GenIDChangeListenersCount(path) == 20);
+ cache->purgeAllUnlocked();
+ // The listeners don't actually purge until we try to add another one.
+ draw_path(ctx.get(), rtc.get(), path, pathRenderer.get(), aaType, style);
+ REPORTER_ASSERT(reporter, SkPathPriv::GenIDChangeListenersCount(path) == 1);
}
// Test that deleting the original path invalidates the VBs cached by the tessellating path renderer
@@ -118,7 +139,7 @@
// resources should be created.
const int kExpectedResources = 1;
- test_path(reporter, create_concave_path, createPR, kExpectedResources);
+ test_path(reporter, create_concave_path, createPR, kExpectedResources, false);
// Test with a style that alters the path geometry. This needs to attach the invalidation logic
// to the original path, not the modified path produced by the style.
@@ -126,8 +147,8 @@
paint.setStyle(SkPaint::kStroke_Style);
paint.setStrokeWidth(1);
GrStyle style(paint);
- test_path(
- reporter, create_concave_path, createPR, kExpectedResources, GrAAType::kNone, style);
+ test_path(reporter, create_concave_path, createPR, kExpectedResources, false, GrAAType::kNone,
+ style);
}
// Test that deleting the original path invalidates the textures cached by the SW path renderer
@@ -140,7 +161,8 @@
// only contains a single quad so GrFillRectOp doesn't need to use the shared index buffer.
const int kExpectedResources = 1;
- test_path(reporter, create_concave_path, createPR, kExpectedResources, GrAAType::kCoverage);
+ test_path(reporter, create_concave_path, createPR, kExpectedResources, true,
+ GrAAType::kCoverage);
// Test with a style that alters the path geometry. This needs to attach the invalidation logic
// to the original path, not the modified path produced by the style.
@@ -148,6 +170,6 @@
paint.setStyle(SkPaint::kStroke_Style);
paint.setStrokeWidth(1);
GrStyle style(paint);
- test_path(reporter, create_concave_path, createPR, kExpectedResources, GrAAType::kCoverage,
- style);
+ test_path(reporter, create_concave_path, createPR, kExpectedResources, true,
+ GrAAType::kCoverage, style);
}