Revert of Pre-crop filled rects to avoid scissor (patchset #6 id:100001 of https://codereview.chromium.org/2132073002/ )
Reason for revert: I think this is still causing a test failure on Chrome windows bots. https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/243928/steps/cc_unittests%20%28with%20patch%29%20on%20Windows-7-SP1/logs/stdio Original issue's description: > Pre-crop filled rects to avoid scissor > > Updates GrDrawContext to crop filled rects to the clip bounds before > creating batches for them. Also adds clipping logic to ignore scissor > when the draw falls completely inside. These two changes combined > reduce API traffic and improve batching. > > In the future this can and should be improved by switching to floating > point clip boundaries, thus allowing us to throw out non pixel aligned > rectangle clips as well. > > BUG=skia: > GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2132073002 > > Committed: https://skia.googlesource.com/skia/+/7969838702135b9f127bd738728da61bc49b050a > Committed: https://skia.googlesource.com/skia/+/86de59f4a99b5f54be0483c60ff0335be55b2bdf TBR=bsalomon@google.com,robertphillips@google.com,csmartdalton@google.com # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=skia: Review-Url: https://codereview.chromium.org/2140253004
This commit is contained in:
parent
634b430080
commit
ba3880fa6d
@ -1,108 +0,0 @@
|
||||
/*
|
||||
* Copyright 2016 Google Inc.
|
||||
*
|
||||
* Use of this source code is governed by a BSD-style license that can be
|
||||
* found in the LICENSE file.
|
||||
*/
|
||||
|
||||
#include "gm.h"
|
||||
#include "SkPath.h"
|
||||
#include "SkRandom.h"
|
||||
#include "SkRRect.h"
|
||||
#include "SkSurface.h"
|
||||
|
||||
namespace skiagm {
|
||||
|
||||
constexpr SkRect kSrcImageClip{75, 75, 275, 275};
|
||||
|
||||
/*
|
||||
* The purpose of this test is to exercise all three codepaths in GrDrawContext (drawFilledRect,
|
||||
* fillRectToRect, fillRectWithLocalMatrix) that pre-crop filled rects based on the clip.
|
||||
*
|
||||
* The test creates an image of a green square surrounded by red background, then draws this image
|
||||
* in various ways with the red clipped out. The test is successful if there is no visible red
|
||||
* background, scissor is never used, and ideally, all the rectangles draw in one batch.
|
||||
*/
|
||||
class CroppedRectsGM : public GM {
|
||||
private:
|
||||
SkString onShortName() override final { return SkString("croppedrects"); }
|
||||
SkISize onISize() override { return SkISize::Make(500, 500); }
|
||||
|
||||
void onOnceBeforeDraw() override {
|
||||
sk_sp<SkSurface> srcSurface = SkSurface::MakeRasterN32Premul(500, 500);
|
||||
SkCanvas* srcCanvas = srcSurface->getCanvas();
|
||||
|
||||
srcCanvas->clear(SK_ColorRED);
|
||||
|
||||
SkPaint paint;
|
||||
paint.setColor(0xff00ff00);
|
||||
srcCanvas->drawRect(kSrcImageClip, paint);
|
||||
|
||||
constexpr SkScalar kStrokeWidth = 10;
|
||||
SkPaint stroke;
|
||||
stroke.setStyle(SkPaint::kStroke_Style);
|
||||
stroke.setStrokeWidth(kStrokeWidth);
|
||||
stroke.setColor(0xff008800);
|
||||
srcCanvas->drawRect(kSrcImageClip.makeInset(kStrokeWidth / 2, kStrokeWidth / 2), stroke);
|
||||
|
||||
fSrcImage = srcSurface->makeImageSnapshot(SkBudgeted::kYes, SkSurface::kNo_ForceUnique);
|
||||
fSrcImageShader = fSrcImage->makeShader(SkShader::kClamp_TileMode,
|
||||
SkShader::kClamp_TileMode);
|
||||
}
|
||||
|
||||
void onDraw(SkCanvas* canvas) override {
|
||||
canvas->clear(SK_ColorWHITE);
|
||||
|
||||
{
|
||||
// GrDrawContext::drawFilledRect.
|
||||
SkAutoCanvasRestore acr(canvas, true);
|
||||
SkPaint paint;
|
||||
paint.setShader(fSrcImageShader);
|
||||
paint.setFilterQuality(kNone_SkFilterQuality);
|
||||
canvas->clipRect(kSrcImageClip);
|
||||
canvas->drawPaint(paint);
|
||||
}
|
||||
|
||||
{
|
||||
// GrDrawContext::fillRectToRect.
|
||||
SkAutoCanvasRestore acr(canvas, true);
|
||||
SkPaint paint;
|
||||
paint.setFilterQuality(kNone_SkFilterQuality);
|
||||
SkRect drawRect = SkRect::MakeXYWH(350, 100, 100, 300);
|
||||
canvas->clipRect(drawRect);
|
||||
canvas->drawImageRect(fSrcImage.get(),
|
||||
kSrcImageClip.makeOutset(0.5f * kSrcImageClip.width(),
|
||||
kSrcImageClip.height()),
|
||||
drawRect.makeOutset(0.5f * drawRect.width(), drawRect.height()),
|
||||
&paint);
|
||||
}
|
||||
|
||||
{
|
||||
// GrDrawContext::fillRectWithLocalMatrix.
|
||||
SkAutoCanvasRestore acr(canvas, true);
|
||||
SkPath path;
|
||||
path.moveTo(kSrcImageClip.fLeft - kSrcImageClip.width(), kSrcImageClip.centerY());
|
||||
path.lineTo(kSrcImageClip.fRight + 3 * kSrcImageClip.width(), kSrcImageClip.centerY());
|
||||
SkPaint paint;
|
||||
paint.setStyle(SkPaint::kStroke_Style);
|
||||
paint.setStrokeWidth(2 * kSrcImageClip.height());
|
||||
paint.setShader(fSrcImageShader);
|
||||
paint.setFilterQuality(kNone_SkFilterQuality);
|
||||
canvas->translate(-90, 263);
|
||||
canvas->scale(300 / kSrcImageClip.width(), 100 / kSrcImageClip.height());
|
||||
canvas->clipRect(kSrcImageClip);
|
||||
canvas->drawPath(path, paint);
|
||||
}
|
||||
|
||||
// TODO: assert the draw target only has one batch in the post-MDB world.
|
||||
}
|
||||
|
||||
sk_sp<SkImage> fSrcImage;
|
||||
sk_sp<SkShader> fSrcImageShader;
|
||||
|
||||
typedef GM INHERITED;
|
||||
};
|
||||
|
||||
DEF_GM( return new CroppedRectsGM(); )
|
||||
|
||||
}
|
@ -109,26 +109,6 @@ public:
|
||||
const SkRect* devBounds, GrAppliedClip*) const = 0;
|
||||
|
||||
virtual ~GrClip() {}
|
||||
|
||||
protected:
|
||||
/**
|
||||
* Returns true if a clip can safely disable its scissor test for a particular draw.
|
||||
*/
|
||||
static bool CanIgnoreScissor(const SkIRect& scissorRect, const SkRect& drawBounds) {
|
||||
// This is the maximum distance that a draw may extend beyond a clip's scissor and still
|
||||
// count as inside. We use a sloppy compare because the draw may have chosen its bounds in a
|
||||
// different coord system. The rationale for 1e-3 is that in the coverage case (and barring
|
||||
// unexpected rounding), as long as coverage stays below 0.5 * 1/256 we ought to be OK.
|
||||
constexpr SkScalar fuzz = 1e-3f;
|
||||
SkASSERT(!scissorRect.isEmpty());
|
||||
SkASSERT(!drawBounds.isEmpty());
|
||||
return scissorRect.fLeft <= drawBounds.fLeft + fuzz &&
|
||||
scissorRect.fTop <= drawBounds.fTop + fuzz &&
|
||||
scissorRect.fRight >= drawBounds.fRight - fuzz &&
|
||||
scissorRect.fBottom >= drawBounds.fBottom - fuzz;
|
||||
}
|
||||
|
||||
friend class GrClipMaskManager;
|
||||
};
|
||||
|
||||
/**
|
||||
|
@ -54,14 +54,12 @@ bool GrFixedClip::apply(GrContext*, const GrPipelineBuilder& pipelineBuilder,
|
||||
if (devBounds && !devBounds->intersects(SkRect::Make(tightScissor))) {
|
||||
return false;
|
||||
}
|
||||
if (!devBounds || !CanIgnoreScissor(fScissorState.rect(), *devBounds)) {
|
||||
if (fHasStencilClip) {
|
||||
out->makeScissoredStencil(tightScissor, &fDeviceBounds);
|
||||
} else {
|
||||
out->makeScissored(tightScissor);
|
||||
}
|
||||
return true;
|
||||
if (fHasStencilClip) {
|
||||
out->makeScissoredStencil(tightScissor, &fDeviceBounds);
|
||||
} else {
|
||||
out->makeScissored(tightScissor);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
out->makeStencil(fHasStencilClip, fDeviceBounds);
|
||||
|
@ -268,7 +268,7 @@ bool GrClipMaskManager::SetupClipping(GrContext* context,
|
||||
} else {
|
||||
SkIRect scissorSpaceIBounds(clipSpaceIBounds);
|
||||
scissorSpaceIBounds.offset(-clip.origin());
|
||||
if (!GrClip::CanIgnoreScissor(scissorSpaceIBounds, devBounds)) {
|
||||
if (!SkRect::Make(scissorSpaceIBounds).contains(devBounds)) {
|
||||
out->makeScissored(scissorSpaceIBounds);
|
||||
}
|
||||
return true;
|
||||
@ -302,11 +302,11 @@ bool GrClipMaskManager::SetupClipping(GrContext* context,
|
||||
&clipFP)) {
|
||||
SkIRect scissorSpaceIBounds(clipSpaceIBounds);
|
||||
scissorSpaceIBounds.offset(-clip.origin());
|
||||
if (GrClip::CanIgnoreScissor(scissorSpaceIBounds, devBounds)) {
|
||||
out->makeFPBased(std::move(clipFP), SkRect::Make(scissorSpaceIBounds));
|
||||
} else {
|
||||
if (!SkRect::Make(scissorSpaceIBounds).contains(devBounds)) {
|
||||
out->makeScissoredFPBased(std::move(clipFP), scissorSpaceIBounds);
|
||||
return true;
|
||||
}
|
||||
out->makeFPBased(std::move(clipFP), SkRect::Make(scissorSpaceIBounds));
|
||||
return true;
|
||||
}
|
||||
}
|
||||
@ -369,11 +369,7 @@ bool GrClipMaskManager::SetupClipping(GrContext* context,
|
||||
// use both stencil and scissor test to the bounds for the final draw.
|
||||
SkIRect scissorSpaceIBounds(clipSpaceIBounds);
|
||||
scissorSpaceIBounds.offset(clipSpaceToStencilSpaceOffset);
|
||||
if (GrClip::CanIgnoreScissor(scissorSpaceIBounds, devBounds)) {
|
||||
out->makeStencil(true, devBounds);
|
||||
} else {
|
||||
out->makeScissoredStencil(scissorSpaceIBounds);
|
||||
}
|
||||
out->makeScissoredStencil(scissorSpaceIBounds);
|
||||
return true;
|
||||
}
|
||||
|
||||
|
@ -270,70 +270,17 @@ static bool should_apply_coverage_aa(const GrPaint& paint, GrRenderTarget* rt,
|
||||
}
|
||||
}
|
||||
|
||||
// Attempts to crop a rect and optional local rect to the clip boundaries.
|
||||
// Returns false if the draw can be skipped entirely.
|
||||
static bool crop_filled_rect(const GrRenderTarget* rt, const GrClip& clip,
|
||||
const SkMatrix& viewMatrix, SkRect* rect,
|
||||
SkRect* localRect = nullptr) {
|
||||
if (!viewMatrix.rectStaysRect()) {
|
||||
return true;
|
||||
}
|
||||
|
||||
SkMatrix inverseViewMatrix;
|
||||
if (!viewMatrix.invert(&inverseViewMatrix)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
SkIRect clipDevBounds;
|
||||
SkRect clipBounds;
|
||||
SkASSERT(inverseViewMatrix.rectStaysRect());
|
||||
|
||||
clip.getConservativeBounds(rt->width(), rt->height(), &clipDevBounds);
|
||||
inverseViewMatrix.mapRect(&clipBounds, SkRect::Make(clipDevBounds));
|
||||
|
||||
if (localRect) {
|
||||
if (!rect->intersects(clipBounds)) {
|
||||
return false;
|
||||
}
|
||||
const SkScalar dx = localRect->width() / rect->width();
|
||||
const SkScalar dy = localRect->height() / rect->height();
|
||||
if (clipBounds.fLeft > rect->fLeft) {
|
||||
localRect->fLeft += (clipBounds.fLeft - rect->fLeft) * dx;
|
||||
rect->fLeft = clipBounds.fLeft;
|
||||
}
|
||||
if (clipBounds.fTop > rect->fTop) {
|
||||
localRect->fTop += (clipBounds.fTop - rect->fTop) * dy;
|
||||
rect->fTop = clipBounds.fTop;
|
||||
}
|
||||
if (clipBounds.fRight < rect->fRight) {
|
||||
localRect->fRight -= (rect->fRight - clipBounds.fRight) * dx;
|
||||
rect->fRight = clipBounds.fRight;
|
||||
}
|
||||
if (clipBounds.fBottom < rect->fBottom) {
|
||||
localRect->fBottom -= (rect->fBottom - clipBounds.fBottom) * dy;
|
||||
rect->fBottom = clipBounds.fBottom;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
return rect->intersect(clipBounds);
|
||||
}
|
||||
|
||||
bool GrDrawContext::drawFilledRect(const GrClip& clip,
|
||||
const GrPaint& paint,
|
||||
const SkMatrix& viewMatrix,
|
||||
const SkRect& rect,
|
||||
const GrUserStencilSettings* ss) {
|
||||
SkRect croppedRect = rect;
|
||||
if (!crop_filled_rect(fRenderTarget.get(), clip, viewMatrix, &croppedRect)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
SkAutoTUnref<GrDrawBatch> batch;
|
||||
bool useHWAA;
|
||||
|
||||
if (InstancedRendering* ir = this->getDrawTarget()->instancedRendering()) {
|
||||
batch.reset(ir->recordRect(croppedRect, viewMatrix, paint.getColor(),
|
||||
batch.reset(ir->recordRect(rect, viewMatrix, paint.getColor(),
|
||||
paint.isAntiAlias(), fInstancedPipelineInfo,
|
||||
&useHWAA));
|
||||
if (batch) {
|
||||
@ -350,10 +297,10 @@ bool GrDrawContext::drawFilledRect(const GrClip& clip,
|
||||
// The fill path can handle rotation but not skew.
|
||||
if (view_matrix_ok_for_aa_fill_rect(viewMatrix)) {
|
||||
SkRect devBoundRect;
|
||||
viewMatrix.mapRect(&devBoundRect, croppedRect);
|
||||
viewMatrix.mapRect(&devBoundRect, rect);
|
||||
|
||||
batch.reset(GrRectBatchFactory::CreateAAFill(paint.getColor(), viewMatrix,
|
||||
croppedRect, devBoundRect));
|
||||
rect, devBoundRect));
|
||||
if (batch) {
|
||||
GrPipelineBuilder pipelineBuilder(paint, useHWAA);
|
||||
if (ss) {
|
||||
@ -364,7 +311,7 @@ bool GrDrawContext::drawFilledRect(const GrClip& clip,
|
||||
}
|
||||
}
|
||||
} else {
|
||||
this->drawNonAAFilledRect(clip, paint, viewMatrix, croppedRect, nullptr, nullptr, ss);
|
||||
this->drawNonAAFilledRect(clip, paint, viewMatrix, rect, nullptr, nullptr, ss);
|
||||
return true;
|
||||
}
|
||||
|
||||
@ -575,18 +522,12 @@ void GrDrawContext::fillRectToRect(const GrClip& clip,
|
||||
SkDEBUGCODE(this->validate();)
|
||||
GR_AUDIT_TRAIL_AUTO_FRAME(fAuditTrail, "GrDrawContext::fillRectToRect");
|
||||
|
||||
SkRect croppedRect = rectToDraw;
|
||||
SkRect croppedLocalRect = localRect;
|
||||
if (!crop_filled_rect(fRenderTarget.get(), clip, viewMatrix, &croppedRect, &croppedLocalRect)) {
|
||||
return;
|
||||
}
|
||||
|
||||
AutoCheckFlush acf(fDrawingManager);
|
||||
SkAutoTUnref<GrDrawBatch> batch;
|
||||
bool useHWAA;
|
||||
|
||||
if (InstancedRendering* ir = this->getDrawTarget()->instancedRendering()) {
|
||||
batch.reset(ir->recordRect(croppedRect, viewMatrix, paint.getColor(), croppedLocalRect,
|
||||
batch.reset(ir->recordRect(rectToDraw, viewMatrix, paint.getColor(), localRect,
|
||||
paint.isAntiAlias(), fInstancedPipelineInfo, &useHWAA));
|
||||
if (batch) {
|
||||
GrPipelineBuilder pipelineBuilder(paint, useHWAA);
|
||||
@ -597,15 +538,15 @@ void GrDrawContext::fillRectToRect(const GrClip& clip,
|
||||
|
||||
if (should_apply_coverage_aa(paint, fRenderTarget.get(), &useHWAA) &&
|
||||
view_matrix_ok_for_aa_fill_rect(viewMatrix)) {
|
||||
batch.reset(GrAAFillRectBatch::CreateWithLocalRect(paint.getColor(), viewMatrix,
|
||||
croppedRect, croppedLocalRect));
|
||||
batch.reset(GrAAFillRectBatch::CreateWithLocalRect(paint.getColor(), viewMatrix, rectToDraw,
|
||||
localRect));
|
||||
if (batch) {
|
||||
GrPipelineBuilder pipelineBuilder(paint, useHWAA);
|
||||
this->drawBatch(pipelineBuilder, clip, batch);
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
this->drawNonAAFilledRect(clip, paint, viewMatrix, croppedRect, &croppedLocalRect,
|
||||
this->drawNonAAFilledRect(clip, paint, viewMatrix, rectToDraw, &localRect,
|
||||
nullptr, nullptr);
|
||||
}
|
||||
|
||||
@ -621,17 +562,12 @@ void GrDrawContext::fillRectWithLocalMatrix(const GrClip& clip,
|
||||
SkDEBUGCODE(this->validate();)
|
||||
GR_AUDIT_TRAIL_AUTO_FRAME(fAuditTrail, "GrDrawContext::fillRectWithLocalMatrix");
|
||||
|
||||
SkRect croppedRect = rectToDraw;
|
||||
if (!crop_filled_rect(fRenderTarget.get(), clip, viewMatrix, &croppedRect)) {
|
||||
return;
|
||||
}
|
||||
|
||||
AutoCheckFlush acf(fDrawingManager);
|
||||
SkAutoTUnref<GrDrawBatch> batch;
|
||||
bool useHWAA;
|
||||
|
||||
if (InstancedRendering* ir = this->getDrawTarget()->instancedRendering()) {
|
||||
batch.reset(ir->recordRect(croppedRect, viewMatrix, paint.getColor(), localMatrix,
|
||||
batch.reset(ir->recordRect(rectToDraw, viewMatrix, paint.getColor(), localMatrix,
|
||||
paint.isAntiAlias(), fInstancedPipelineInfo, &useHWAA));
|
||||
if (batch) {
|
||||
GrPipelineBuilder pipelineBuilder(paint, useHWAA);
|
||||
@ -643,11 +579,11 @@ void GrDrawContext::fillRectWithLocalMatrix(const GrClip& clip,
|
||||
if (should_apply_coverage_aa(paint, fRenderTarget.get(), &useHWAA) &&
|
||||
view_matrix_ok_for_aa_fill_rect(viewMatrix)) {
|
||||
batch.reset(GrAAFillRectBatch::Create(paint.getColor(), viewMatrix, localMatrix,
|
||||
croppedRect));
|
||||
rectToDraw));
|
||||
GrPipelineBuilder pipelineBuilder(paint, useHWAA);
|
||||
this->getDrawTarget()->drawBatch(pipelineBuilder, this, clip, batch);
|
||||
} else {
|
||||
this->drawNonAAFilledRect(clip, paint, viewMatrix, croppedRect, nullptr,
|
||||
this->drawNonAAFilledRect(clip, paint, viewMatrix, rectToDraw, nullptr,
|
||||
&localMatrix, nullptr);
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user