From 09dfc4759e99229e7c74891a88596e8b9b3d9026 Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Wed, 13 Sep 2017 15:25:47 -0400 Subject: Pull non-substantive changes out of explicit GPU resource allocation CL Change-Id: Ib6a289553ecd15c722599b7dc0d347a7800801cb Reviewed-on: https://skia-review.googlesource.com/46284 Reviewed-by: Brian Osman Commit-Queue: Robert Phillips --- gm/blurrect.cpp | 4 ++-- gm/yuvtorgbeffect.cpp | 5 +++-- src/gpu/GrContext.cpp | 13 +++++++------ src/gpu/GrOpList.cpp | 6 ++++++ src/gpu/GrOpList.h | 2 ++ src/gpu/GrRenderTargetContext.cpp | 2 ++ src/gpu/GrResourceAllocator.cpp | 32 ++++++++++++++++++++++++++++++++ src/gpu/GrResourceAllocator.h | 3 ++- src/gpu/text/GrAtlasTextBlob.cpp | 4 ---- tests/CanvasTest.cpp | 4 +--- tests/GpuSampleLocationsTest.cpp | 4 ++-- 11 files changed, 59 insertions(+), 20 deletions(-) diff --git a/gm/blurrect.cpp b/gm/blurrect.cpp index 1e245712fe..54f3c216c0 100644 --- a/gm/blurrect.cpp +++ b/gm/blurrect.cpp @@ -58,7 +58,7 @@ static void draw_donut_skewed(SkCanvas* canvas, const SkRect& r, const SkPaint& /* * Spits out a dummy gradient to test blur with shader on paint */ -static sk_sp MakeRadial() { +static sk_sp make_radial() { SkPoint pts[2] = { { 0, 0 }, { SkIntToScalar(100), SkIntToScalar(100) } @@ -123,7 +123,7 @@ protected: paint.setAlpha(fAlpha); SkPaint paintWithRadial = paint; - paintWithRadial.setShader(MakeRadial()); + paintWithRadial.setShader(make_radial()); constexpr Proc procs[] = { fill_rect, draw_donut, draw_donut_skewed diff --git a/gm/yuvtorgbeffect.cpp b/gm/yuvtorgbeffect.cpp index 001c53d701..f957566d9f 100644 --- a/gm/yuvtorgbeffect.cpp +++ b/gm/yuvtorgbeffect.cpp @@ -253,8 +253,9 @@ protected: SkMatrix viewMatrix; viewMatrix.setTranslate(x, y); grPaint.addColorFragmentProcessor(std::move(fp)); - renderTargetContext->priv().testingOnly_addDrawOp(GrRectOpFactory::MakeNonAAFill( - std::move(grPaint), viewMatrix, renderRect, GrAAType::kNone)); + std::unique_ptr op(GrRectOpFactory::MakeNonAAFill( + std::move(grPaint), viewMatrix, renderRect, GrAAType::kNone)); + renderTargetContext->priv().testingOnly_addDrawOp(std::move(op)); } } } diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index d05f1976f1..ba26cc844e 100644 --- a/src/gpu/GrContext.cpp +++ b/src/gpu/GrContext.cpp @@ -646,12 +646,6 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceContext* src, return false; } - if (!proxyToRead->instantiate(fContext->resourceProvider())) { - return false; - } - - GrSurface* surfaceToRead = proxyToRead->priv().peekSurface(); - if (GrGpu::kRequireDraw_DrawPreference == drawPreference && !didTempDraw) { return false; } @@ -660,6 +654,13 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceContext* src, this->flushSurfaceWrites(proxyToRead.get()); configToRead = tempDrawInfo.fReadConfig; } + + if (!proxyToRead->instantiate(fContext->resourceProvider())) { + return false; + } + + GrSurface* surfaceToRead = proxyToRead->priv().peekSurface(); + if (!fContext->fGpu->readPixels(surfaceToRead, proxyToRead->origin(), left, top, width, height, configToRead, buffer, rowBytes)) { return false; diff --git a/src/gpu/GrOpList.cpp b/src/gpu/GrOpList.cpp index 521beea743..17b0f10ebf 100644 --- a/src/gpu/GrOpList.cpp +++ b/src/gpu/GrOpList.cpp @@ -31,6 +31,7 @@ GrOpList::GrOpList(GrResourceProvider* resourceProvider, fTarget.setProxy(sk_ref_sp(surfaceProxy), kWrite_GrIOType); fTarget.get()->setLastOpList(this); +#ifndef MDB_ALLOC_RESOURCES // MDB TODO: remove this! We are currently moving to having all the ops that target // the RT as a dest (e.g., clear, etc.) rely on the opList's 'fTarget' pointer // for the IO Ref. This works well but until they are all swapped over (and none @@ -39,6 +40,7 @@ GrOpList::GrOpList(GrResourceProvider* resourceProvider, // re-use assumptions. fTarget.get()->instantiate(resourceProvider); fTarget.markPendingIO(); +#endif } GrOpList::~GrOpList() { @@ -101,6 +103,10 @@ void GrOpList::addDependency(GrSurfaceProxy* dependedOn, const GrCaps& caps) { } #ifdef SK_DEBUG +bool GrOpList::isInstantiated() const { + return fTarget.get()->priv().isInstantiated(); +} + void GrOpList::dump() const { SkDebugf("--------------------------------------------------------------\n"); SkDebugf("node: %d -> RT: %d\n", fUniqueID, fTarget.get() ? fTarget.get()->uniqueID().asUInt() diff --git a/src/gpu/GrOpList.h b/src/gpu/GrOpList.h index ae6b9a1906..ef21a24f46 100644 --- a/src/gpu/GrOpList.h +++ b/src/gpu/GrOpList.h @@ -98,6 +98,8 @@ public: void setStencilLoadOp(GrLoadOp loadOp) { fStencilLoadOp = loadOp; } protected: + SkDEBUGCODE(bool isInstantiated() const;) + GrSurfaceProxyRef fTarget; GrAuditTrail* fAuditTrail; diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp index 561eaef340..4f1ad1279e 100644 --- a/src/gpu/GrRenderTargetContext.cpp +++ b/src/gpu/GrRenderTargetContext.cpp @@ -124,10 +124,12 @@ GrRenderTargetContext::GrRenderTargetContext(GrContext* context, fColorXformFromSRGB = GrColorSpaceXform::Make(srgbColorSpace.get(), fColorSpace.get()); } +#ifndef MDB_ALLOC_RESOURCES // MDB TODO: to ensure all resources still get allocated in the correct order in the hybrid // world we need to get the correct opList here so that it, in turn, can grab and hold // its rendertarget. this->getRTOpList(); +#endif SkDEBUGCODE(this->validate();) } diff --git a/src/gpu/GrResourceAllocator.cpp b/src/gpu/GrResourceAllocator.cpp index 76abe1855c..e5d6dbf360 100644 --- a/src/gpu/GrResourceAllocator.cpp +++ b/src/gpu/GrResourceAllocator.cpp @@ -134,3 +134,35 @@ void GrResourceAllocator::assign() { fActiveIntvls.insertByIncreasingEnd(cur); } } + +#ifdef SK_DEBUG +void GrResourceAllocator::dump() { + unsigned int min = fNumOps+1; + unsigned int max = 0; + for(const Interval* cur = fIntvlList.peekHead(); cur; cur = cur->fNext) { + SkDebugf("{ %d,%d }: [%d, %d]\n", + cur->fProxy->uniqueID().asUInt(), cur->fProxy->underlyingUniqueID().asUInt(), + cur->fStart, cur->fEnd); + if (min > cur->fStart) { + min = cur->fStart; + } + if (max < cur->fEnd) { + max = cur->fEnd; + } + } + + for(const Interval* cur = fIntvlList.peekHead(); cur; cur = cur->fNext) { + SkDebugf("{ %3d,%3d }: ", + cur->fProxy->uniqueID().asUInt(), cur->fProxy->underlyingUniqueID().asUInt()); + for (unsigned int i = min; i <= max; ++i) { + if (i >= cur->fStart && i <= cur->fEnd) { + SkDebugf("x"); + } else { + SkDebugf(" "); + } + } + SkDebugf("\n"); + } +} +#endif + diff --git a/src/gpu/GrResourceAllocator.h b/src/gpu/GrResourceAllocator.h index fcaa3d5c9a..bd0c690a58 100644 --- a/src/gpu/GrResourceAllocator.h +++ b/src/gpu/GrResourceAllocator.h @@ -52,6 +52,7 @@ public: } void assign(); + SkDEBUGCODE(void dump();) private: class Interval; @@ -92,7 +93,7 @@ private: static uint32_t Hash(const uint32_t& key) { return key; } GrSurfaceProxy* fProxy; - uint32_t fProxyID; // This is here b.c. DynamicHash requires a ref to the key + uint32_t fProxyID; // This is here b.c. DynamicHash requires a ref to the key unsigned int fStart; unsigned int fEnd; Interval* fNext; diff --git a/src/gpu/text/GrAtlasTextBlob.cpp b/src/gpu/text/GrAtlasTextBlob.cpp index 823444c972..b88ed9329e 100644 --- a/src/gpu/text/GrAtlasTextBlob.cpp +++ b/src/gpu/text/GrAtlasTextBlob.cpp @@ -298,10 +298,6 @@ inline void GrAtlasTextBlob::flushRun(GrRenderTargetContext* rtc, const GrClip& int lastRun = fRuns[run].fSubRunInfo.count() - 1; for (int subRun = 0; subRun <= lastRun; subRun++) { const Run::SubRunInfo& info = fRuns[run].fSubRunInfo[subRun]; - GrPaint grPaint; - if (!paint.toGrPaint(info.maskFormat(), rtc, viewMatrix, &grPaint)) { - continue; - } int glyphCount = info.glyphCount(); if (0 == glyphCount) { continue; diff --git a/tests/CanvasTest.cpp b/tests/CanvasTest.cpp index e64f5e95f7..7aaf554015 100644 --- a/tests/CanvasTest.cpp +++ b/tests/CanvasTest.cpp @@ -105,8 +105,7 @@ DEF_TEST(canvas_clipbounds, reporter) { } } -// Will call proc with multiple styles of canse (recording, raster, pdf) -// +// Will call proc with multiple styles of canvas (recording, raster, pdf) template static void multi_canvas_driver(int w, int h, F proc) { proc(SkPictureRecorder().beginRecording(SkRect::MakeIWH(w, h))); @@ -116,7 +115,6 @@ template static void multi_canvas_driver(int w, int h, F proc) { proc(SkSurface::MakeRasterN32Premul(w, h, nullptr)->getCanvas()); } - const SkIRect gBaseRestrictedR = { 0, 0, 10, 10 }; static void test_restriction(skiatest::Reporter* reporter, SkCanvas* canvas) { diff --git a/tests/GpuSampleLocationsTest.cpp b/tests/GpuSampleLocationsTest.cpp index 399f4310d6..fa99a9ae9e 100644 --- a/tests/GpuSampleLocationsTest.cpp +++ b/tests/GpuSampleLocationsTest.cpp @@ -117,11 +117,11 @@ void test_sampleLocations(skiatest::Reporter* reporter, TestSampleLocationsInter for (int i = 0; i < numTestPatterns; ++i) { int numSamples = (int)kTestPatterns[i].size(); GrAlwaysAssert(numSamples > 1 && SkIsPow2(numSamples)); - bottomUps[i] = ctx->makeDeferredRenderTargetContextWithFallback( + bottomUps[i] = ctx->makeDeferredRenderTargetContext( SkBackingFit::kExact, 100, 100, kRGBA_8888_GrPixelConfig, nullptr, rand.nextRangeU(1 + numSamples / 2, numSamples), kBottomLeft_GrSurfaceOrigin); - topDowns[i] = ctx->makeDeferredRenderTargetContextWithFallback( + topDowns[i] = ctx->makeDeferredRenderTargetContext( SkBackingFit::kExact, 100, 100, kRGBA_8888_GrPixelConfig, nullptr, rand.nextRangeU(1 + numSamples / 2, numSamples), kTopLeft_GrSurfaceOrigin); -- cgit v1.2.3