Shrink saveLayer device bounds when it supplies an explicit bounds and has a complex paint
This CL shrinks the bound computed for saveLayers that possess both an explicit bound and a complex paint (e.g., one that affects transparent black). In this case the bound of the layer should be the clipped explicit bound rather then the clip prior/after the saveLayer/restore block. In the following the first bound is the currently computed bound while the second is the new/desired one: For a 100x100 picture saveLayer (no bound, no paint) [ 0 0 100 100 ] [ 50 50 100 100 ] setMatrix (translate 50, 50) [ 0 0 100 100 ] [ 50 50 100 100 ] saveLayer (bound of 0, 0, 50, 50 - complex paint) [ 0 0 100 100 ] [ 50 50 100 100 ] restore [ 0 0 100 100 ] [ 50 50 100 100 ] restore [ 0 0 100 100 ] [ 50 50 100 100 ] Review URL: https://codereview.chromium.org/696763002
This commit is contained in:
parent
66cad7669b
commit
4d52afef5c
@ -631,7 +631,7 @@ SkPicture::SkPicture(SkScalar width, SkScalar height, SkRecord* record, SkBBoxHi
|
||||
, fAnalysis(*fRecord) {
|
||||
// TODO: delay as much of this work until just before first playback?
|
||||
if (fBBH.get()) {
|
||||
SkRecordFillBounds(*fRecord, fBBH.get());
|
||||
SkRecordFillBounds(this->cullRect(), *fRecord, fBBH.get());
|
||||
}
|
||||
this->needsNewGenID();
|
||||
}
|
||||
|
@ -116,14 +116,6 @@ DRAW(DrawVertices, drawVertices(r.vmode, r.vertexCount, r.vertices, r.texs, r.co
|
||||
DRAW(DrawData, drawData(r.data, r.length));
|
||||
#undef DRAW
|
||||
|
||||
|
||||
// This looks silly, I know. Why not just use SkRect::MakeLargest()?
|
||||
// In practice, this is well large enough, and it has a few extra advantages:
|
||||
// it fits in an SkIRect, and we can munge it a little in both SkRect and
|
||||
// SKIRect space without worrying about overflow.
|
||||
static const SkRect kUnbounded = { -2e9f, -2e9f, 2e9f, 2e9f };
|
||||
|
||||
|
||||
// This is an SkRecord visitor that fills an SkBBoxHierarchy.
|
||||
//
|
||||
// The interesting part here is how to calculate bounds for ops which don't
|
||||
@ -144,11 +136,13 @@ static const SkRect kUnbounded = { -2e9f, -2e9f, 2e9f, 2e9f };
|
||||
// in for all the control ops we stashed away.
|
||||
class FillBounds : SkNoncopyable {
|
||||
public:
|
||||
FillBounds(const SkRecord& record, SkBBoxHierarchy* bbh) : fBounds(record.count()) {
|
||||
FillBounds(const SkRect& cullRect, const SkRecord& record, SkBBoxHierarchy* bbh)
|
||||
: fCullRect(cullRect)
|
||||
, fBounds(record.count()) {
|
||||
// Calculate bounds for all ops. This won't go quite in order, so we'll need
|
||||
// to store the bounds separately then feed them in to the BBH later in order.
|
||||
fCTM = &SkMatrix::I();
|
||||
fCurrentClipBounds = kUnbounded;
|
||||
fCurrentClipBounds = fCullRect;
|
||||
for (fCurrentOp = 0; fCurrentOp < record.count(); fCurrentOp++) {
|
||||
record.visit<void>(fCurrentOp, *this);
|
||||
}
|
||||
@ -161,7 +155,7 @@ public:
|
||||
|
||||
// Any control ops not part of any Save/Restore block draw everywhere.
|
||||
while (!fControlIndices.isEmpty()) {
|
||||
this->popControl(kUnbounded);
|
||||
this->popControl(fCullRect);
|
||||
}
|
||||
|
||||
// Finally feed all stored bounds into the BBH. They'll be returned in this order.
|
||||
@ -204,7 +198,7 @@ private:
|
||||
Bounds clip = SkRect::Make(devBounds);
|
||||
// We don't call adjustAndMap() because as its last step it would intersect the adjusted
|
||||
// clip bounds with the previous clip, exactly what we can't do when the clip grows.
|
||||
fCurrentClipBounds = this->adjustForSaveLayerPaints(&clip) ? clip : kUnbounded;
|
||||
fCurrentClipBounds = this->adjustForSaveLayerPaints(&clip) ? clip : fCullRect;
|
||||
}
|
||||
|
||||
// Restore holds the devBounds for the clip after the {save,saveLayer}/restore block completes.
|
||||
@ -216,7 +210,7 @@ private:
|
||||
const int kSavesToIgnore = 1;
|
||||
Bounds clip = SkRect::Make(op.devBounds);
|
||||
fCurrentClipBounds =
|
||||
this->adjustForSaveLayerPaints(&clip, kSavesToIgnore) ? clip : kUnbounded;
|
||||
this->adjustForSaveLayerPaints(&clip, kSavesToIgnore) ? clip : fCullRect;
|
||||
}
|
||||
|
||||
// We also take advantage of SaveLayer bounds when present to further cut the clip down.
|
||||
@ -253,7 +247,14 @@ private:
|
||||
|
||||
void pushSaveBlock(const SkPaint* paint) {
|
||||
// Starting a new Save block. Push a new entry to represent that.
|
||||
SaveBounds sb = { 0, Bounds::MakeEmpty(), paint };
|
||||
SaveBounds sb;
|
||||
sb.controlOps = 0;
|
||||
// If the paint affects transparent black, the bound shouldn't be smaller
|
||||
// than the current clip bounds.
|
||||
sb.bounds =
|
||||
PaintMayAffectTransparentBlack(paint) ? fCurrentClipBounds : Bounds::MakeEmpty();
|
||||
sb.paint = paint;
|
||||
|
||||
fSaveStack.push(sb);
|
||||
this->pushControl();
|
||||
}
|
||||
@ -303,19 +304,15 @@ private:
|
||||
SaveBounds sb;
|
||||
fSaveStack.pop(&sb);
|
||||
|
||||
// If the paint affects transparent black, we can't trust any of our calculated bounds.
|
||||
const Bounds& bounds =
|
||||
PaintMayAffectTransparentBlack(sb.paint) ? fCurrentClipBounds : sb.bounds;
|
||||
|
||||
while (sb.controlOps --> 0) {
|
||||
this->popControl(bounds);
|
||||
this->popControl(sb.bounds);
|
||||
}
|
||||
|
||||
// This whole Save block may be part another Save block.
|
||||
this->updateSaveBounds(bounds);
|
||||
this->updateSaveBounds(sb.bounds);
|
||||
|
||||
// If called from a real Restore (not a phony one for balance), it'll need the bounds.
|
||||
return bounds;
|
||||
return sb.bounds;
|
||||
}
|
||||
|
||||
void pushControl() {
|
||||
@ -340,7 +337,7 @@ private:
|
||||
// FIXME: this method could use better bounds
|
||||
Bounds bounds(const DrawText&) const { return fCurrentClipBounds; }
|
||||
|
||||
Bounds bounds(const Clear&) const { return kUnbounded; } // Ignores the clip.
|
||||
Bounds bounds(const Clear&) const { return fCullRect; } // Ignores the clip.
|
||||
Bounds bounds(const DrawPaint&) const { return fCurrentClipBounds; }
|
||||
Bounds bounds(const NoOp&) const { return Bounds::MakeEmpty(); } // NoOps don't draw.
|
||||
|
||||
@ -537,6 +534,9 @@ private:
|
||||
return rect;
|
||||
}
|
||||
|
||||
// We do not guarantee anything for operations outside of the cull rect
|
||||
const SkRect fCullRect;
|
||||
|
||||
// Conservative identity-space bounds for each op in the SkRecord.
|
||||
SkAutoTMalloc<Bounds> fBounds;
|
||||
|
||||
@ -554,6 +554,6 @@ private:
|
||||
|
||||
} // namespace SkRecords
|
||||
|
||||
void SkRecordFillBounds(const SkRecord& record, SkBBoxHierarchy* bbh) {
|
||||
SkRecords::FillBounds(record, bbh);
|
||||
void SkRecordFillBounds(const SkRect& cullRect, const SkRecord& record, SkBBoxHierarchy* bbh) {
|
||||
SkRecords::FillBounds(cullRect, record, bbh);
|
||||
}
|
||||
|
@ -15,7 +15,7 @@
|
||||
#include "SkRecord.h"
|
||||
|
||||
// Fill a BBH to be used by SkRecordDraw to accelerate playback.
|
||||
void SkRecordFillBounds(const SkRecord&, SkBBoxHierarchy*);
|
||||
void SkRecordFillBounds(const SkRect& cullRect, const SkRecord&, SkBBoxHierarchy*);
|
||||
|
||||
// Draw an SkRecord into an SkCanvas. A convenience wrapper around SkRecords::Draw.
|
||||
void SkRecordDraw(const SkRecord&, SkCanvas*, const SkBBoxHierarchy*, SkDrawPictureCallback*);
|
||||
|
@ -137,7 +137,7 @@ DEF_TEST(RecordDraw_BBH, r) {
|
||||
recorder.restore();
|
||||
|
||||
TestBBH bbh;
|
||||
SkRecordFillBounds(record, &bbh);
|
||||
SkRecordFillBounds(SkRect::MakeWH(SkIntToScalar(W), SkIntToScalar(H)), record, &bbh);
|
||||
|
||||
REPORTER_ASSERT(r, bbh.fEntries.count() == 5);
|
||||
for (int i = 0; i < bbh.fEntries.count(); i++) {
|
||||
@ -163,14 +163,14 @@ DEF_TEST(RecordDraw_TextBounds, r) {
|
||||
recorder.drawPosText(text, bytes, pos, SkPaint());
|
||||
|
||||
TestBBH bbh;
|
||||
SkRecordFillBounds(record, &bbh);
|
||||
SkRecordFillBounds(SkRect::MakeWH(SkIntToScalar(W), SkIntToScalar(H)), record, &bbh);
|
||||
REPORTER_ASSERT(r, bbh.fEntries.count() == 2);
|
||||
|
||||
// We can make these next assertions confidently because SkRecordFillBounds
|
||||
// builds its bounds by overestimating font metrics in a platform-independent way.
|
||||
// If that changes, these tests will need to be more flexible.
|
||||
REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[0].bounds, SkRect::MakeLTRB(-110, 0, 140, 60)));
|
||||
REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[1].bounds, SkRect::MakeLTRB(-80, 20, 180, 100)));
|
||||
REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[0].bounds, SkRect::MakeLTRB(0, 0, 140, 60)));
|
||||
REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[1].bounds, SkRect::MakeLTRB(0, 20, 180, 100)));
|
||||
}
|
||||
|
||||
// Base test to ensure start/stop range is respected
|
||||
@ -251,7 +251,7 @@ DEF_TEST(RecordDraw_SaveLayerAffectsClipBounds, r) {
|
||||
// The second bug showed up as adjusting the picture bounds (0,0,50,50) by the drop shadow too.
|
||||
// The saveLayer, clipRect, and restore bounds were incorrectly (0,0,70,50).
|
||||
TestBBH bbh;
|
||||
SkRecordFillBounds(record, &bbh);
|
||||
SkRecordFillBounds(SkRect::MakeWH(50, 50), record, &bbh);
|
||||
REPORTER_ASSERT(r, bbh.fEntries.count() == 4);
|
||||
REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[0].bounds, SkRect::MakeLTRB(0, 0, 50, 50)));
|
||||
REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[1].bounds, SkRect::MakeLTRB(0, 0, 50, 50)));
|
||||
@ -259,6 +259,29 @@ DEF_TEST(RecordDraw_SaveLayerAffectsClipBounds, r) {
|
||||
REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[3].bounds, SkRect::MakeLTRB(0, 0, 50, 50)));
|
||||
}
|
||||
|
||||
// When a saveLayer provides an explicit bound and has a complex paint (e.g., one that
|
||||
// affects transparent black), that bound should serve to shrink the area of the required
|
||||
// backing store.
|
||||
DEF_TEST(RecordDraw_SaveLayerBoundsAffectsClipBounds, r) {
|
||||
SkRecord record;
|
||||
SkRecorder recorder(&record, 50, 50);
|
||||
|
||||
SkPaint p;
|
||||
p.setXfermodeMode(SkXfermode::kSrc_Mode);
|
||||
|
||||
SkRect bounds = SkRect::MakeLTRB(10, 10, 40, 40);
|
||||
recorder.saveLayer(&bounds, &p);
|
||||
recorder.drawRect(SkRect::MakeLTRB(20, 20, 30, 30), SkPaint());
|
||||
recorder.restore();
|
||||
|
||||
TestBBH bbh;
|
||||
SkRecordFillBounds(SkRect::MakeWH(50, 50), record, &bbh);
|
||||
REPORTER_ASSERT(r, bbh.fEntries.count() == 3);
|
||||
REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[0].bounds, SkRect::MakeLTRB(10, 10, 40, 40)));
|
||||
REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[1].bounds, SkRect::MakeLTRB(20, 20, 30, 30)));
|
||||
REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[2].bounds, SkRect::MakeLTRB(10, 10, 40, 40)));
|
||||
}
|
||||
|
||||
DEF_TEST(RecordDraw_drawImage, r){
|
||||
class SkCanvasMock : public SkCanvas {
|
||||
public:
|
||||
|
Loading…
Reference in New Issue
Block a user