Revert "Add genIDs from all contributing elements to GrReducedClip's mask key."
This reverts commit 1354048c8fa885b83e414532c011d710590d6b46.
Reason for revert: tsan/valgrind failures
Original change's description:
> Add genIDs from all contributing elements to GrReducedClip's mask key.
>
> Change-Id: I3fed124ba3fefd1ef82acdb4ace9531d0c89ad8b
> Reviewed-on: https://skia-review.googlesource.com/138586
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
TBR=bsalomon@google.com,robertphillips@google.com,csmartdalton@google.com
Change-Id: Ia5bc098309cd02baf46f03d8ff17fabbd383481e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/139920
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
diff --git a/tests/ClipStackTest.cpp b/tests/ClipStackTest.cpp
index 60a8626..498e60d 100644
--- a/tests/ClipStackTest.cpp
+++ b/tests/ClipStackTest.cpp
@@ -944,18 +944,6 @@
}
}
-// When the entire clip is an intersection of rectangles we can wind up with a mask with one
-// AA rect element that doesn't have a known top-most contributing element.
-static bool mask_allowed_no_top_most_gen_id(const GrReducedClip& reduced) {
- return 1 == reduced.maskElements().count() &&
- reduced.maskElements().head()->fElement.getDeviceSpaceType() ==
- SkClipStack::Element::DeviceSpaceType::kRect &&
- reduced.maskElements().head()->fElement.isAA() &&
- !reduced.maskElements().head()->fElement.isInverseFilled() &&
- reduced.maskElements().head()->fElement.getOp() ==
- static_cast<SkClipOp>(SkRegion::kReplace_Op);
-}
-
static void test_reduced_clip_stack(skiatest::Reporter* reporter) {
// We construct random clip stacks, reduce them, and then rasterize both versions to verify that
// they are equal.
@@ -1055,35 +1043,41 @@
auto context = GrContext::MakeMock(nullptr);
const GrCaps* caps = context->contextPriv().caps();
+ // Zero the memory we will new the GrReducedClip into. This ensures the elements gen ID
+ // will be kInvalidGenID if left uninitialized.
+ SkAlignedSTStorage<1, GrReducedClip> storage;
+ memset(storage.get(), 0, sizeof(GrReducedClip));
+ GR_STATIC_ASSERT(0 == SkClipStack::kInvalidGenID);
+
// Get the reduced version of the stack.
SkRect queryBounds = kBounds;
queryBounds.outset(kBounds.width() / 2, kBounds.height() / 2);
- const GrReducedClip reduced(stack, queryBounds, caps);
+ const GrReducedClip* reduced = new (storage.get()) GrReducedClip(stack, queryBounds, caps);
- if (!reduced.maskElements().isEmpty() && !mask_allowed_no_top_most_gen_id(reduced)) {
- REPORTER_ASSERT(reporter, SK_InvalidGenID != reduced.topMaskElementID(),
- testCase.c_str());
- }
+ REPORTER_ASSERT(reporter,
+ reduced->maskElements().isEmpty() ||
+ SkClipStack::kInvalidGenID != reduced->maskGenID(),
+ testCase.c_str());
- if (!reduced.maskElements().isEmpty()) {
- REPORTER_ASSERT(reporter, reduced.hasScissor(), testCase.c_str());
+ if (!reduced->maskElements().isEmpty()) {
+ REPORTER_ASSERT(reporter, reduced->hasScissor(), testCase.c_str());
SkRect stackBounds;
SkClipStack::BoundsType stackBoundsType;
stack.getBounds(&stackBounds, &stackBoundsType);
- REPORTER_ASSERT(reporter, reduced.maskRequiresAA() == doAA, testCase.c_str());
+ REPORTER_ASSERT(reporter, reduced->maskRequiresAA() == doAA, testCase.c_str());
}
// Build a new clip stack based on the reduced clip elements
SkClipStack reducedStack;
- if (GrReducedClip::InitialState::kAllOut == reduced.initialState()) {
+ if (GrReducedClip::InitialState::kAllOut == reduced->initialState()) {
// whether the result is bounded or not, the whole plane should start outside the clip.
reducedStack.clipEmpty();
}
- for (ElementList::Iter iter(reduced.maskElements()); iter.get(); iter.next()) {
- add_elem_to_stack(iter.get()->fElement, &reducedStack);
+ for (ElementList::Iter iter(reduced->maskElements()); iter.get(); iter.next()) {
+ add_elem_to_stack(*iter.get(), &reducedStack);
}
- SkIRect scissor = reduced.hasScissor() ? reduced.scissor() : kIBounds;
+ SkIRect scissor = reduced->hasScissor() ? reduced->scissor() : kIBounds;
// GrReducedClipStack assumes that the final result is clipped to the returned bounds
reducedStack.clipDevRect(scissor, kIntersect_SkClipOp);
@@ -1097,6 +1091,8 @@
set_region_to_stack(reducedStack, scissor, &reducedRegion);
REPORTER_ASSERT(reporter, region == reducedRegion, testCase.c_str());
+
+ reduced->~GrReducedClip();
}
}
@@ -1106,7 +1102,7 @@
#define SUPPRESS_VISIBILITY_WARNING __attribute__((visibility("hidden")))
#endif
-static void test_reduced_clip_stack_mask_key(skiatest::Reporter* reporter) {
+static void test_reduced_clip_stack_genid(skiatest::Reporter* reporter) {
{
SkClipStack stack;
stack.clipRect(SkRect::MakeXYWH(0, 0, 100, 100), SkMatrix::I(), kReplace_SkClipOp,
@@ -1118,9 +1114,16 @@
auto context = GrContext::MakeMock(nullptr);
const GrCaps* caps = context->contextPriv().caps();
- GrReducedClip reduced(stack, bounds, caps);
- REPORTER_ASSERT(reporter, reduced.maskElements().count() == 1);
- REPORTER_ASSERT(reporter, reduced.maskUniqueKey().isValid());
+ SkAlignedSTStorage<1, GrReducedClip> storage;
+ memset(storage.get(), 0, sizeof(GrReducedClip));
+ GR_STATIC_ASSERT(0 == SkClipStack::kInvalidGenID);
+ const GrReducedClip* reduced = new (storage.get()) GrReducedClip(stack, bounds, caps);
+
+ REPORTER_ASSERT(reporter, reduced->maskElements().count() == 1);
+ // Clips will be cached based on the generation id. Make sure the gen id is valid.
+ REPORTER_ASSERT(reporter, SkClipStack::kInvalidGenID != reduced->maskGenID());
+
+ reduced->~GrReducedClip();
}
{
SkClipStack stack;
@@ -1159,7 +1162,7 @@
static const struct SUPPRESS_VISIBILITY_WARNING {
SkRect testBounds;
int reducedClipCount;
- uint32_t topStackGenID;
+ uint32_t reducedGenID;
InitialState initialState;
SkIRect clipIRect;
// parameter.
@@ -1204,14 +1207,17 @@
const GrReducedClip reduced(stack, testCases[i].testBounds, caps);
REPORTER_ASSERT(reporter, reduced.maskElements().count() ==
testCases[i].reducedClipCount);
- if (reduced.maskElements().count() && !mask_allowed_no_top_most_gen_id(reduced)) {
- REPORTER_ASSERT(reporter, reduced.topMaskElementID() == testCases[i].topStackGenID);
- REPORTER_ASSERT(reporter, reduced.maskUniqueKey().isValid());
+ SkASSERT(reduced.maskElements().count() == testCases[i].reducedClipCount);
+ if (reduced.maskElements().count()) {
+ REPORTER_ASSERT(reporter, reduced.maskGenID() == testCases[i].reducedGenID);
+ SkASSERT(reduced.maskGenID() == testCases[i].reducedGenID);
}
-
REPORTER_ASSERT(reporter, reduced.initialState() == testCases[i].initialState);
+ SkASSERT(reduced.initialState() == testCases[i].initialState);
REPORTER_ASSERT(reporter, reduced.hasScissor());
+ SkASSERT(reduced.hasScissor());
REPORTER_ASSERT(reporter, reduced.scissor() == testCases[i].clipIRect);
+ SkASSERT(reduced.scissor() == testCases[i].clipIRect);
}
}
}
@@ -1468,7 +1474,7 @@
test_invfill_diff_bug(reporter);
test_reduced_clip_stack(reporter);
- test_reduced_clip_stack_mask_key(reporter);
+ test_reduced_clip_stack_genid(reporter);
test_reduced_clip_stack_no_aa_crash(reporter);
test_reduced_clip_stack_aa(reporter);
test_tiny_query_bounds_assertion_bug(reporter);
@@ -1493,7 +1499,7 @@
path.addCircle(15, 15, 8);
path.setFillType(SkPath::kEvenOdd_FillType);
- static const char* kTag = GrReducedClip::kMaskTestTag;
+ static const char* kTag = GrClipStackClip::kMaskTestTag;
GrResourceCache* cache = context->contextPriv().getResourceCache();
static constexpr int kN = 5;