diff options
-rw-r--r-- | libs/hwui/BakedOpRenderer.cpp | 2 | ||||
-rw-r--r-- | libs/hwui/BakedOpState.cpp | 8 | ||||
-rw-r--r-- | libs/hwui/BakedOpState.h | 5 | ||||
-rw-r--r-- | libs/hwui/tests/unit/BakedOpStateTests.cpp | 40 |
4 files changed, 29 insertions, 26 deletions
diff --git a/libs/hwui/BakedOpRenderer.cpp b/libs/hwui/BakedOpRenderer.cpp index b9c13e6cf847..c1f19a341956 100644 --- a/libs/hwui/BakedOpRenderer.cpp +++ b/libs/hwui/BakedOpRenderer.cpp @@ -201,6 +201,7 @@ void BakedOpRenderer::setupStencilQuads(std::vector<Vertex>& quadVertices, } void BakedOpRenderer::setupStencilRectList(const ClipBase* clip) { + LOG_ALWAYS_FATAL_IF(clip->mode != ClipMode::RectangleList, "can't rectlist clip without rectlist"); auto&& rectList = reinterpret_cast<const ClipRectList*>(clip)->rectList; int quadCount = rectList.getTransformedRectanglesCount(); std::vector<Vertex> rectangleVertices; @@ -234,6 +235,7 @@ void BakedOpRenderer::setupStencilRectList(const ClipBase* clip) { } void BakedOpRenderer::setupStencilRegion(const ClipBase* clip) { + LOG_ALWAYS_FATAL_IF(clip->mode != ClipMode::Region, "can't region clip without region"); auto&& region = reinterpret_cast<const ClipRegion*>(clip)->region; std::vector<Vertex> regionVertices; diff --git a/libs/hwui/BakedOpState.cpp b/libs/hwui/BakedOpState.cpp index f1cc846593db..f0406fa13a3d 100644 --- a/libs/hwui/BakedOpState.cpp +++ b/libs/hwui/BakedOpState.cpp @@ -48,10 +48,10 @@ ResolvedRenderState::ResolvedRenderState(LinearAllocator& allocator, Snapshot& s const Rect& clipRect = clipState->rect; if (CC_UNLIKELY(clipRect.isEmpty() || !clippedBounds.intersects(clipRect))) { // Rejected based on either empty clip, or bounds not intersecting with clip - if (clipState) { - allocator.rewindIfLastAlloc(clipState); - clipState = nullptr; - } + + // Note: we could rewind the clipState object in situations where the clipRect is empty, + // but *only* if the caching logic within ClipArea was aware of the rewind. + clipState = nullptr; clippedBounds.setEmpty(); } else { // Not rejected! compute true clippedBounds and clipSideFlags diff --git a/libs/hwui/BakedOpState.h b/libs/hwui/BakedOpState.h index 5c7b43f5a034..3db28c982469 100644 --- a/libs/hwui/BakedOpState.h +++ b/libs/hwui/BakedOpState.h @@ -99,6 +99,7 @@ class BakedOpState { public: static BakedOpState* tryConstruct(LinearAllocator& allocator, Snapshot& snapshot, const RecordedOp& recordedOp) { + if (CC_UNLIKELY(snapshot.getRenderTargetClip().isEmpty())) return nullptr; BakedOpState* bakedState = new (allocator) BakedOpState( allocator, snapshot, recordedOp, false); if (bakedState->computedState.clippedBounds.isEmpty()) { @@ -118,6 +119,7 @@ public: static BakedOpState* tryStrokeableOpConstruct(LinearAllocator& allocator, Snapshot& snapshot, const RecordedOp& recordedOp, StrokeBehavior strokeBehavior) { + if (CC_UNLIKELY(snapshot.getRenderTargetClip().isEmpty())) return nullptr; bool expandForStroke = (strokeBehavior == StrokeBehavior::StyleDefined) ? (recordedOp.paint && recordedOp.paint->getStyle() != SkPaint::kFill_Style) : true; @@ -126,6 +128,7 @@ public: allocator, snapshot, recordedOp, expandForStroke); if (bakedState->computedState.clippedBounds.isEmpty()) { // bounds are empty, so op is rejected + // NOTE: this won't succeed if a clip was allocated allocator.rewindIfLastAlloc(bakedState); return nullptr; } @@ -134,7 +137,7 @@ public: static BakedOpState* tryShadowOpConstruct(LinearAllocator& allocator, Snapshot& snapshot, const ShadowOp* shadowOpPtr) { - if (snapshot.getRenderTargetClip().isEmpty()) return nullptr; + if (CC_UNLIKELY(snapshot.getRenderTargetClip().isEmpty())) return nullptr; // clip isn't empty, so construct the op return new (allocator) BakedOpState(allocator, snapshot, shadowOpPtr); diff --git a/libs/hwui/tests/unit/BakedOpStateTests.cpp b/libs/hwui/tests/unit/BakedOpStateTests.cpp index 3fd822d71310..b3506869e0dc 100644 --- a/libs/hwui/tests/unit/BakedOpStateTests.cpp +++ b/libs/hwui/tests/unit/BakedOpStateTests.cpp @@ -176,29 +176,26 @@ TEST(ResolvedRenderState, construct_expandForStroke) { } TEST(BakedOpState, tryConstruct) { - LinearAllocator allocator; - Matrix4 translate100x0; translate100x0.loadTranslate(100, 0, 0); SkPaint paint; ClipRect clip(Rect(100, 200)); - { - RectOp rejectOp(Rect(30, 40, 100, 200), translate100x0, &clip, &paint); - auto snapshot = TestUtils::makeSnapshot(Matrix4::identity(), Rect(100, 200)); - BakedOpState* bakedState = BakedOpState::tryConstruct(allocator, *snapshot, rejectOp); - - EXPECT_EQ(nullptr, bakedState); // rejected by clip, so not constructed - EXPECT_GT(8u, allocator.usedSize()); // no significant allocation space used for rejected op - } - { - RectOp successOp(Rect(30, 40, 100, 200), Matrix4::identity(), &clip, &paint); - auto snapshot = TestUtils::makeSnapshot(Matrix4::identity(), Rect(100, 200)); - BakedOpState* bakedState = BakedOpState::tryConstruct(allocator, *snapshot, successOp); - EXPECT_NE(nullptr, bakedState); // NOT rejected by clip, so will be constructed - EXPECT_LE(64u, allocator.usedSize()); // relatively large alloc for non-rejected op - } + LinearAllocator allocator; + RectOp successOp(Rect(30, 40, 100, 200), Matrix4::identity(), &clip, &paint); + auto snapshot = TestUtils::makeSnapshot(Matrix4::identity(), Rect(100, 200)); + EXPECT_NE(nullptr, BakedOpState::tryConstruct(allocator, *snapshot, successOp)) + << "successOp NOT rejected by clip, so should be constructed"; + size_t successAllocSize = allocator.usedSize(); + EXPECT_LE(64u, successAllocSize) << "relatively large alloc for non-rejected op"; + + RectOp rejectOp(Rect(30, 40, 100, 200), translate100x0, &clip, &paint); + EXPECT_EQ(nullptr, BakedOpState::tryConstruct(allocator, *snapshot, rejectOp)) + << "rejectOp rejected by clip, so should not be constructed"; + + // NOTE: this relies on the clip having already been serialized by the op above + EXPECT_EQ(successAllocSize, allocator.usedSize()) << "no extra allocation used for rejected op"; } TEST(BakedOpState, tryShadowOpConstruct) { @@ -207,15 +204,16 @@ TEST(BakedOpState, tryShadowOpConstruct) { auto snapshot = TestUtils::makeSnapshot(Matrix4::identity(), Rect()); // Note: empty clip BakedOpState* bakedState = BakedOpState::tryShadowOpConstruct(allocator, *snapshot, (ShadowOp*)0x1234); - EXPECT_EQ(nullptr, bakedState); // rejected by clip, so not constructed - EXPECT_GT(8u, allocator.usedSize()); // no significant allocation space used for rejected op + EXPECT_EQ(nullptr, bakedState) << "op should be rejected by clip, so not constructed"; + EXPECT_EQ(0u, allocator.usedSize()) << "no serialization, even for clip," + "since op is quick rejected based on snapshot clip"; } { auto snapshot = TestUtils::makeSnapshot(Matrix4::identity(), Rect(100, 200)); BakedOpState* bakedState = BakedOpState::tryShadowOpConstruct(allocator, *snapshot, (ShadowOp*)0x1234); - ASSERT_NE(nullptr, bakedState); // NOT rejected by clip, so will be constructed - EXPECT_LE(64u, allocator.usedSize()); // relatively large alloc for non-rejected op + ASSERT_NE(nullptr, bakedState) << "NOT rejected by clip, so op should be constructed"; + EXPECT_LE(64u, allocator.usedSize()) << "relatively large alloc for non-rejected op"; } } |