Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ae10e136 authored by Chris Craik's avatar Chris Craik Committed by Android (Google) Code Review
Browse files

Merge "Fix clip serialization crash"

parents 2e263c4a 15f04686
Loading
Loading
Loading
Loading
+2 −0
Original line number Diff line number Diff line
@@ -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;
+4 −4
Original line number Diff line number Diff line
@@ -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);

        // 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
+4 −1
Original line number Diff line number Diff line
@@ -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);
+19 −21
Original line number Diff line number Diff line
@@ -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
    }
    {
    LinearAllocator allocator;
    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, 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";

        EXPECT_NE(nullptr, bakedState); // NOT rejected by clip, so will be constructed
        EXPECT_LE(64u, allocator.usedSize()); // 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";
    }
}