Remove incremental draw optimization from SkDebugCanvas
The SkDebugCanvas can be (or is currently) being used to draw to multiple
different canvases. If this use-case is intended, then storing draw
-related state in the canvas causes bugs.
Remove incremental draw from SkDebugCanvas. It can only optimize the
case where draw command index is advanced, no other changes are done
and no visualization is used. This case is not that critical, as it
happens only once per new frame at most. It causes bugs, because
one SkDebugCanvas is used to draw to multiple canvases.
Leave the draw to canvas in same state as it was passed in.
This fixes the debugger bugs where:
* Old, stale image would stay on the raster canvas when a new image is
loaded. Also happened with resizes.
* Proper image for overdraw filter would be visible only for the first
frame
Review URL: https://codereview.chromium.org/844493003
diff --git a/debugger/SkDebugger.cpp b/debugger/SkDebugger.cpp
index 43823c1..fc61dda 100644
--- a/debugger/SkDebugger.cpp
+++ b/debugger/SkDebugger.cpp
@@ -49,19 +49,11 @@
fDebugCanvas->setOverdrawViz(false);
bool pathOps = fDebugCanvas->getAllowSimplifyClip();
fDebugCanvas->setAllowSimplifyClip(false);
- int saveCount = fDebugCanvas->getOutstandingSaveCount();
- fDebugCanvas->setOutstandingSaveCount(0);
fDebugCanvas->draw(canvas);
- int temp = fDebugCanvas->getOutstandingSaveCount();
- for (int i = 0; i < temp; ++i) {
- canvas->restore();
- }
-
fDebugCanvas->setMegaVizMode(vizMode);
fDebugCanvas->setOverdrawViz(overDraw);
- fDebugCanvas->setOutstandingSaveCount(saveCount);
fDebugCanvas->setAllowSimplifyClip(pathOps);
return recorder.endRecording();
diff --git a/src/utils/debugger/SkDebugCanvas.cpp b/src/utils/debugger/SkDebugCanvas.cpp
index 6a04006..9d80955 100644
--- a/src/utils/debugger/SkDebugCanvas.cpp
+++ b/src/utils/debugger/SkDebugCanvas.cpp
@@ -19,14 +19,11 @@
, fPicture(NULL)
, fFilter(false)
, fMegaVizMode(false)
- , fIndex(0)
, fOverdrawViz(false)
, fOverdrawFilter(NULL)
, fOverrideTexFiltering(false)
- , fTexOverrideFilter(NULL)
- , fOutstandingSaveCount(0) {
+ , fTexOverrideFilter(NULL) {
fUserMatrix.reset();
- fDrawNeedsReset = false;
// SkPicturePlayback uses the base-class' quickReject calls to cull clipped
// operations. This can lead to problems in the debugger which expects all
@@ -59,8 +56,6 @@
}
void SkDebugCanvas::draw(SkCanvas* canvas) {
- fDrawNeedsReset = true;
-
if (!fCommandVector.isEmpty()) {
this->drawTo(canvas, fCommandVector.count() - 1);
}
@@ -231,32 +226,20 @@
void SkDebugCanvas::drawTo(SkCanvas* canvas, int index) {
SkASSERT(!fCommandVector.isEmpty());
SkASSERT(index < fCommandVector.count());
- int i = 0;
+
+ int saveCount = canvas->save();
+
SkRect windowRect = SkRect::MakeWH(SkIntToScalar(canvas->getBaseLayerSize().width()),
SkIntToScalar(canvas->getBaseLayerSize().height()));
bool pathOpsMode = getAllowSimplifyClip();
canvas->setAllowSimplifyClip(pathOpsMode);
- // This only works assuming the canvas and device are the same ones that
- // were previously drawn into because they need to preserve all saves
- // and restores.
- // The visibility filter also requires a full re-draw - otherwise we can
- // end up drawing the filter repeatedly.
- if (fIndex < index && !fFilter && !fMegaVizMode && !pathOpsMode && !fDrawNeedsReset) {
- i = fIndex + 1;
- } else {
- for (int j = 0; j < fOutstandingSaveCount; j++) {
- canvas->restore();
- }
- canvas->clear(SK_ColorTRANSPARENT);
- canvas->resetMatrix();
- if (!windowRect.isEmpty()) {
- canvas->clipRect(windowRect, SkRegion::kReplace_Op);
- }
- this->applyUserTransform(canvas);
- fDrawNeedsReset = false;
- fOutstandingSaveCount = 0;
+ canvas->clear(SK_ColorTRANSPARENT);
+ canvas->resetMatrix();
+ if (!windowRect.isEmpty()) {
+ canvas->clipRect(windowRect, SkRegion::kReplace_Op);
}
+ this->applyUserTransform(canvas);
// The setting of the draw filter has to go here (rather than in
// SkRasterWidget) due to the canvas restores this class performs.
@@ -286,7 +269,7 @@
this->markActiveCommands(index);
}
- for (; i <= index; i++) {
+ for (int i = 0; i <= index; i++) {
if (i == index && fFilter) {
canvas->clear(0xAAFFFFFF);
}
@@ -302,8 +285,6 @@
fCommandVector[i]->setUserMatrix(fUserMatrix);
fCommandVector[i]->execute(canvas);
}
-
- fCommandVector[i]->trackSaveState(&fOutstandingSaveCount);
}
}
@@ -350,7 +331,8 @@
if (!canvas->getClipDeviceBounds(&fClip)) {
fClip.setEmpty();
}
- fIndex = index;
+
+ canvas->restoreToCount(saveCount);
}
void SkDebugCanvas::deleteDrawCommandAt(int index) {
diff --git a/src/utils/debugger/SkDebugCanvas.h b/src/utils/debugger/SkDebugCanvas.h
index 3d9983b..e1ad077 100644
--- a/src/utils/debugger/SkDebugCanvas.h
+++ b/src/utils/debugger/SkDebugCanvas.h
@@ -35,9 +35,6 @@
void setOverdrawViz(bool overdrawViz) { fOverdrawViz = overdrawViz; }
bool getOverdrawViz() const { return fOverdrawViz; }
- void setOutstandingSaveCount(int saveCount) { fOutstandingSaveCount = saveCount; }
- int getOutstandingSaveCount() const { return fOutstandingSaveCount; }
-
bool getAllowSimplifyClip() const { return fAllowSimplifyClip; }
void setPicture(SkPicture* picture) { fPicture = picture; }
@@ -137,7 +134,6 @@
void setUserMatrix(SkMatrix matrix) {
fUserMatrix = matrix;
- fDrawNeedsReset = true;
}
SkString clipStackData() const { return fClipStackData; }
@@ -225,9 +221,7 @@
SkPicture* fPicture;
bool fFilter;
bool fMegaVizMode;
- int fIndex;
SkMatrix fUserMatrix;
- bool fDrawNeedsReset; // fUserMatrix has changed so the incremental draw won't work
SkMatrix fMatrix;
SkIRect fClip;
@@ -242,14 +236,6 @@
SkTexOverrideFilter* fTexOverrideFilter;
/**
- Number of unmatched save() calls at any point during a draw.
- If there are any saveLayer() calls outstanding, we need to resolve
- all of them, which in practice means resolving all save() calls,
- to avoid corruption of our canvas.
- */
- int fOutstandingSaveCount;
-
- /**
The active saveLayer commands at a given point in the renderering.
Only used when "mega" visualization is enabled.
*/
diff --git a/src/utils/debugger/SkDrawCommand.cpp b/src/utils/debugger/SkDrawCommand.cpp
index be7fdfc..47b29f7 100644
--- a/src/utils/debugger/SkDrawCommand.cpp
+++ b/src/utils/debugger/SkDrawCommand.cpp
@@ -831,10 +831,6 @@
canvas->restore();
}
-void SkRestoreCommand::trackSaveState(int* state) {
- (*state)--;
-}
-
SkRotateCommand::SkRotateCommand(SkScalar degrees)
: INHERITED(ROTATE) {
fDegrees = degrees;
@@ -854,10 +850,6 @@
canvas->save();
}
-void SkSaveCommand::trackSaveState(int* state) {
- (*state)++;
-}
-
SkSaveLayerCommand::SkSaveLayerCommand(const SkRect* bounds, const SkPaint* paint,
SkCanvas::SaveFlags flags)
: INHERITED(SAVE_LAYER) {
@@ -894,10 +886,6 @@
canvas->save();
}
-void SkSaveLayerCommand::trackSaveState(int* state) {
- (*state)++;
-}
-
SkScaleCommand::SkScaleCommand(SkScalar sx, SkScalar sy)
: INHERITED(SCALE) {
fSx = sx;
diff --git a/src/utils/debugger/SkDrawCommand.h b/src/utils/debugger/SkDrawCommand.h
index 22ee700..746bf59 100644
--- a/src/utils/debugger/SkDrawCommand.h
+++ b/src/utils/debugger/SkDrawCommand.h
@@ -42,10 +42,6 @@
virtual void setUserMatrix(const SkMatrix&) {}
- /** Does nothing by default, but used by save() and restore()-type
- subclasses to track unresolved save() calls. */
- virtual void trackSaveState(int* state) {}
-
// The next "active" system is only used by save, saveLayer, and restore.
// It is used to determine which saveLayers are currently active (at a
// given point in the rendering).
@@ -80,7 +76,6 @@
public:
SkRestoreCommand();
void execute(SkCanvas* canvas) const SK_OVERRIDE;
- void trackSaveState(int* state) SK_OVERRIDE;
Action action() const SK_OVERRIDE { return kPopLayer_Action; }
private:
@@ -511,7 +506,6 @@
public:
SkSaveCommand();
void execute(SkCanvas* canvas) const SK_OVERRIDE;
- void trackSaveState(int* state) SK_OVERRIDE;
Action action() const SK_OVERRIDE { return kPushLayer_Action; }
private:
typedef SkDrawCommand INHERITED;
@@ -523,7 +517,6 @@
SkCanvas::SaveFlags flags);
void execute(SkCanvas* canvas) const SK_OVERRIDE;
void vizExecute(SkCanvas* canvas) const SK_OVERRIDE;
- void trackSaveState(int* state) SK_OVERRIDE;
Action action() const SK_OVERRIDE{ return kPushLayer_Action; }
void setActive(bool active) SK_OVERRIDE { fActive = active; }
bool active() const SK_OVERRIDE { return fActive; }