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

Commit 4876de16 authored by Chris Craik's avatar Chris Craik
Browse files

Properly reject empty unclipped savelayers

bug:27225580
bug:27281241

Empty unclipped savelayers (clipped at defer time, often by dirty rect)
were resulting in invalid layer clear rectangles. Simplify by just
rejecting these unclipped savelayers entirely at defer.

Also, use repaint rect as base clip for constructed ops within
LayerBuilder.

Change-Id: I5c466199e85201a2f68f5cdc60b29187c849961b
parent 1b7db400
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -347,7 +347,7 @@ void BakedOpRenderer::dirtyRenderTarget(const Rect& uiDirty) {
        if (valid) {
            dirty = android::Rect(uiDirty.left, uiDirty.top, uiDirty.right, uiDirty.bottom);
        } else {
            dirty = android::Rect(0, 0,
            dirty = android::Rect(
                    mRenderTarget.offscreenBuffer->viewportWidth,
                    mRenderTarget.offscreenBuffer->viewportHeight);
        }
+15 −7
Original line number Diff line number Diff line
@@ -21,6 +21,15 @@
namespace android {
namespace uirenderer {

static int computeClipSideFlags(const Rect& clip, const Rect& bounds) {
    int clipSideFlags = 0;
    if (clip.left > bounds.left) clipSideFlags |= OpClipSideFlags::Left;
    if (clip.top > bounds.top) clipSideFlags |= OpClipSideFlags::Top;
    if (clip.right < bounds.right) clipSideFlags |= OpClipSideFlags::Right;
    if (clip.bottom < bounds.bottom) clipSideFlags |= OpClipSideFlags::Bottom;
    return clipSideFlags;
}

ResolvedRenderState::ResolvedRenderState(LinearAllocator& allocator, Snapshot& snapshot,
        const RecordedOp& recordedOp, bool expandForStroke) {
    // resolvedMatrix = parentMatrix * localMatrix
@@ -55,10 +64,7 @@ ResolvedRenderState::ResolvedRenderState(LinearAllocator& allocator, Snapshot& s
        clippedBounds.setEmpty();
    } else {
        // Not rejected! compute true clippedBounds and clipSideFlags
        if (clipRect.left > clippedBounds.left) clipSideFlags |= OpClipSideFlags::Left;
        if (clipRect.top > clippedBounds.top) clipSideFlags |= OpClipSideFlags::Top;
        if (clipRect.right < clippedBounds.right) clipSideFlags |= OpClipSideFlags::Right;
        if (clipRect.bottom < clippedBounds.bottom) clipSideFlags |= OpClipSideFlags::Bottom;
        clipSideFlags = computeClipSideFlags(clipRect, clippedBounds);
        clippedBounds.doIntersect(clipRect);
    }
}
@@ -69,11 +75,13 @@ ResolvedRenderState::ResolvedRenderState(LinearAllocator& allocator, Snapshot& s
        , clippedBounds(clipState->rect)
        , clipSideFlags(OpClipSideFlags::Full) {}

ResolvedRenderState::ResolvedRenderState(const ClipRect* viewportRect, const Rect& dstRect)
ResolvedRenderState::ResolvedRenderState(const ClipRect* clipRect, const Rect& dstRect)
        : transform(Matrix4::identity())
        , clipState(viewportRect)
        , clipState(clipRect)
        , clippedBounds(dstRect)
        , clipSideFlags(OpClipSideFlags::None) {}
        , clipSideFlags(computeClipSideFlags(clipRect->rect, dstRect)) {
    clippedBounds.doIntersect(clipRect->rect);
}

} // namespace uirenderer
} // namespace android
+2 −2
Original line number Diff line number Diff line
@@ -175,8 +175,8 @@ private:
            , projectionPathMask(snapshot.projectionPathMask)
            , op(shadowOpPtr) {}

    BakedOpState(const ClipRect* viewportRect, const Rect& dstRect, const RecordedOp& recordedOp)
            : computedState(viewportRect, dstRect)
    BakedOpState(const ClipRect* clipRect, const Rect& dstRect, const RecordedOp& recordedOp)
            : computedState(clipRect, dstRect)
            , alpha(1.0f)
            , roundRectClipState(nullptr)
            , projectionPathMask(nullptr)
+33 −26
Original line number Diff line number Diff line
@@ -782,6 +782,10 @@ void FrameBuilder::deferBeginUnclippedLayerOp(const BeginUnclippedLayerOp& op) {
    boundsTransform.mapRect(dstRect);
    dstRect.doIntersect(mCanvasState.currentSnapshot()->getRenderTargetClip());

    if (dstRect.isEmpty()) {
        // Unclipped layer rejected - push a null op, so next EndUnclippedLayerOp is ignored
        currentLayer().activeUnclippedSaveLayers.push_back(nullptr);
    } else {
        // Allocate a holding position for the layer object (copyTo will produce, copyFrom will consume)
        OffscreenBuffer** layerHandle = mAllocator.create<OffscreenBuffer*>(nullptr);

@@ -790,7 +794,7 @@ void FrameBuilder::deferBeginUnclippedLayerOp(const BeginUnclippedLayerOp& op) {
         */
        auto copyToOp = mAllocator.create_trivial<CopyToLayerOp>(op, layerHandle);
        BakedOpState* bakedState = BakedOpState::directConstruct(mAllocator,
            &(currentLayer().viewportClip), dstRect, *copyToOp);
                &(currentLayer().repaintClip), dstRect, *copyToOp);
        currentLayer().deferUnmergeableOp(mAllocator, bakedState, OpBatchType::CopyToLayer);

        /**
@@ -805,16 +809,19 @@ void FrameBuilder::deferBeginUnclippedLayerOp(const BeginUnclippedLayerOp& op) {
         */
        auto copyFromOp = mAllocator.create_trivial<CopyFromLayerOp>(op, layerHandle);
        bakedState = BakedOpState::directConstruct(mAllocator,
            &(currentLayer().viewportClip), dstRect, *copyFromOp);
                &(currentLayer().repaintClip), dstRect, *copyFromOp);
        currentLayer().activeUnclippedSaveLayers.push_back(bakedState);
    }
}

void FrameBuilder::deferEndUnclippedLayerOp(const EndUnclippedLayerOp& /* ignored */) {
    LOG_ALWAYS_FATAL_IF(currentLayer().activeUnclippedSaveLayers.empty(), "no layer to end!");

    BakedOpState* copyFromLayerOp = currentLayer().activeUnclippedSaveLayers.back();
    currentLayer().deferUnmergeableOp(mAllocator, copyFromLayerOp, OpBatchType::CopyFromLayer);
    currentLayer().activeUnclippedSaveLayers.pop_back();
    if (copyFromLayerOp) {
        currentLayer().deferUnmergeableOp(mAllocator, copyFromLayerOp, OpBatchType::CopyFromLayer);
    }
}

} // namespace uirenderer
+3 −3
Original line number Diff line number Diff line
@@ -199,10 +199,10 @@ LayerBuilder::LayerBuilder(uint32_t width, uint32_t height,
        : width(width)
        , height(height)
        , repaintRect(repaintRect)
        , repaintClip(repaintRect)
        , offscreenBuffer(renderNode ? renderNode->getLayer() : nullptr)
        , beginLayerOp(beginLayerOp)
        , renderNode(renderNode)
        , viewportClip(Rect(width, height)) {}
        , renderNode(renderNode) {}

// iterate back toward target to see if anything drawn since should overlap the new op
// if no target, merging ops still iterate to find similar batch to insert after
@@ -260,7 +260,7 @@ void LayerBuilder::flushLayerClears(LinearAllocator& allocator) {
                Matrix4::identity(), nullptr, paint,
                verts, vertCount);
        BakedOpState* bakedState = BakedOpState::directConstruct(allocator,
                &viewportClip, bounds, *op);
                &repaintClip, bounds, *op);
        deferUnmergeableOp(allocator, bakedState, OpBatchType::Vertices);
    }
}
Loading