From b5cb6835c449c1c292b1ab124691a45d3d113694 Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Fri, 24 Feb 2017 11:01:15 -0500 Subject: Use construct/init pattern with GrPipeline to replace CreateAt. Change-Id: Ic6c7432a9a298a143ce4f2431e94c89a0ea79793 Reviewed-on: https://skia-review.googlesource.com/8938 Commit-Queue: Brian Salomon Reviewed-by: Greg Daniel --- gn/gpu.gni | 1 - src/gpu/GrGpuCommandBuffer.cpp | 1 + src/gpu/GrPipeline.cpp | 65 ++++++++++++++++++---------------------- src/gpu/GrPipeline.h | 21 ++++++++----- src/gpu/GrPipelineBuilder.h | 2 +- src/gpu/GrRenderTargetOpList.cpp | 9 ++---- src/gpu/ops/GrDrawOp.cpp | 27 ----------------- src/gpu/ops/GrDrawOp.h | 14 ++++----- tests/GpuSampleLocationsTest.cpp | 18 +++++------ 9 files changed, 62 insertions(+), 96 deletions(-) delete mode 100644 src/gpu/ops/GrDrawOp.cpp diff --git a/gn/gpu.gni b/gn/gpu.gni index ae05f2cb31..d0fd0da23c 100644 --- a/gn/gpu.gni +++ b/gn/gpu.gni @@ -256,7 +256,6 @@ skia_gpu_sources = [ "$_src/gpu/ops/GrDiscardOp.h", "$_src/gpu/ops/GrDrawAtlasOp.cpp", "$_src/gpu/ops/GrDrawAtlasOp.h", - "$_src/gpu/ops/GrDrawOp.cpp", "$_src/gpu/ops/GrDrawOp.h", "$_src/gpu/ops/GrDrawPathOp.cpp", "$_src/gpu/ops/GrDrawPathOp.h", diff --git a/src/gpu/GrGpuCommandBuffer.cpp b/src/gpu/GrGpuCommandBuffer.cpp index 9ef459f08d..4eb98433ef 100644 --- a/src/gpu/GrGpuCommandBuffer.cpp +++ b/src/gpu/GrGpuCommandBuffer.cpp @@ -39,6 +39,7 @@ bool GrGpuCommandBuffer::draw(const GrPipeline& pipeline, const GrMesh* mesh, int meshCount, const SkRect& bounds) { + SkASSERT(pipeline.isInitialized()); if (primProc.numAttribs() > this->gpu()->caps()->maxVertexAttributes()) { this->gpu()->stats()->incNumFailedDraws(); return false; diff --git a/src/gpu/GrPipeline.cpp b/src/gpu/GrPipeline.cpp index 51b6666148..0dcf20db74 100644 --- a/src/gpu/GrPipeline.cpp +++ b/src/gpu/GrPipeline.cpp @@ -19,49 +19,43 @@ #include "ops/GrOp.h" -GrPipeline* GrPipeline::CreateAt(void* memory, const CreateArgs& args, - GrPipelineOptimizations* optimizations) { +GrPipelineOptimizations GrPipeline::init(const InitArgs& args) { SkASSERT(args.fAnalysis); SkASSERT(args.fRenderTarget); - GrPipeline* pipeline = new (memory) GrPipeline; - pipeline->fRenderTarget.reset(args.fRenderTarget); - pipeline->fScissorState = args.fAppliedClip->scissorState(); - pipeline->fWindowRectsState = args.fAppliedClip->windowRectsState(); - pipeline->fUserStencilSettings = args.fUserStencil; - pipeline->fDrawFace = static_cast(args.fDrawFace); + fRenderTarget.reset(args.fRenderTarget); + fScissorState = args.fAppliedClip->scissorState(); + fWindowRectsState = args.fAppliedClip->windowRectsState(); + fUserStencilSettings = args.fUserStencil; + fDrawFace = static_cast(args.fDrawFace); - pipeline->fFlags = args.fFlags; + fFlags = args.fFlags; if (args.fProcessors->usesDistanceVectorField()) { - pipeline->fFlags |= kUsesDistanceVectorField_Flag; + fFlags |= kUsesDistanceVectorField_Flag; } if (args.fAppliedClip->hasStencilClip()) { - pipeline->fFlags |= kHasStencilClip_Flag; + fFlags |= kHasStencilClip_Flag; } if (!args.fUserStencil->isDisabled(args.fAppliedClip->hasStencilClip())) { - pipeline->fFlags |= kStencilEnabled_Flag; + fFlags |= kStencilEnabled_Flag; } if (args.fProcessors->disableOutputConversionToSRGB()) { - pipeline->fFlags |= kDisableOutputConversionToSRGB_Flag; + fFlags |= kDisableOutputConversionToSRGB_Flag; } if (args.fProcessors->allowSRGBInputs()) { - pipeline->fFlags |= kAllowSRGBInputs_Flag; + fFlags |= kAllowSRGBInputs_Flag; } bool isHWAA = kHWAntialias_Flag & args.fFlags; // Create XferProcessor from DS's XPFactory - bool hasMixedSamples = args.fRenderTarget->isMixedSampled() && - (isHWAA || pipeline->isStencilEnabled()); + bool hasMixedSamples = args.fRenderTarget->isMixedSampled() && (isHWAA || isStencilEnabled()); const GrXPFactory* xpFactory = args.fProcessors->xpFactory(); sk_sp xferProcessor; if (xpFactory) { xferProcessor.reset(xpFactory->createXferProcessor(*args.fAnalysis, hasMixedSamples, &args.fDstTexture, *args.fCaps)); - if (!xferProcessor) { - pipeline->~GrPipeline(); - return nullptr; - } + SkASSERT(xferProcessor); } else { // This may return nullptr in the common case of src-over implemented using hw blending. xferProcessor.reset(GrPorterDuffXPFactory::CreateSrcOverXferProcessor( @@ -83,7 +77,7 @@ GrPipeline* GrPipeline::CreateAt(void* memory, const CreateArgs& args, overrideColor = GrColor_ILLEGAL; } - pipeline->fXferProcessor.reset(xferProcessor.get()); + fXferProcessor.reset(xferProcessor.get()); if ((optFlags & GrXferProcessor::kIgnoreColor_OptFlag) || (optFlags & GrXferProcessor::kOverrideColor_OptFlag)) { @@ -94,50 +88,49 @@ GrPipeline* GrPipeline::CreateAt(void* memory, const CreateArgs& args, // Copy GrFragmentProcessors from GrPipelineBuilder to Pipeline, possibly removing some of the // color fragment processors. - pipeline->fNumColorProcessors = - args.fProcessors->numColorFragmentProcessors() - colorFPsToEliminate; + fNumColorProcessors = args.fProcessors->numColorFragmentProcessors() - colorFPsToEliminate; int numTotalProcessors = - pipeline->fNumColorProcessors + args.fProcessors->numCoverageFragmentProcessors(); + fNumColorProcessors + args.fProcessors->numCoverageFragmentProcessors(); if (args.fAppliedClip->clipCoverageFragmentProcessor()) { ++numTotalProcessors; } - pipeline->fFragmentProcessors.reset(numTotalProcessors); + fFragmentProcessors.reset(numTotalProcessors); int currFPIdx = 0; for (int i = colorFPsToEliminate; i < args.fProcessors->numColorFragmentProcessors(); ++i, ++currFPIdx) { const GrFragmentProcessor* fp = args.fProcessors->colorFragmentProcessor(i); - pipeline->fFragmentProcessors[currFPIdx].reset(fp); + fFragmentProcessors[currFPIdx].reset(fp); usesLocalCoords = usesLocalCoords || fp->usesLocalCoords(); } for (int i = 0; i < args.fProcessors->numCoverageFragmentProcessors(); ++i, ++currFPIdx) { const GrFragmentProcessor* fp = args.fProcessors->coverageFragmentProcessor(i); - pipeline->fFragmentProcessors[currFPIdx].reset(fp); + fFragmentProcessors[currFPIdx].reset(fp); usesLocalCoords = usesLocalCoords || fp->usesLocalCoords(); } if (const GrFragmentProcessor* fp = args.fAppliedClip->clipCoverageFragmentProcessor()) { - pipeline->fFragmentProcessors[currFPIdx].reset(fp); + fFragmentProcessors[currFPIdx].reset(fp); usesLocalCoords = usesLocalCoords || fp->usesLocalCoords(); } // Setup info we need to pass to GrPrimitiveProcessors that are used with this GrPipeline. - optimizations->fFlags = 0; + GrPipelineOptimizations optimizations; + optimizations.fFlags = 0; if (GrColor_ILLEGAL != overrideColor) { - optimizations->fFlags |= GrPipelineOptimizations::kUseOverrideColor_Flag; - optimizations->fOverrideColor = overrideColor; + optimizations.fFlags |= GrPipelineOptimizations::kUseOverrideColor_Flag; + optimizations.fOverrideColor = overrideColor; } if (usesLocalCoords) { - optimizations->fFlags |= GrPipelineOptimizations::kReadsLocalCoords_Flag; + optimizations.fFlags |= GrPipelineOptimizations::kReadsLocalCoords_Flag; } if (SkToBool(optFlags & GrXferProcessor::kCanTweakAlphaForCoverage_OptFlag)) { - optimizations->fFlags |= GrPipelineOptimizations::kCanTweakAlphaForCoverage_Flag; + optimizations.fFlags |= GrPipelineOptimizations::kCanTweakAlphaForCoverage_Flag; } if (GrXPFactory::WillReadDst(xpFactory, *args.fAnalysis)) { - optimizations->fFlags |= GrPipelineOptimizations::kXPReadsDst_Flag; + optimizations.fFlags |= GrPipelineOptimizations::kXPReadsDst_Flag; } - - return pipeline; + return optimizations; } static void add_dependencies_for_processor(const GrFragmentProcessor* proc, GrRenderTarget* rt) { diff --git a/src/gpu/GrPipeline.h b/src/gpu/GrPipeline.h index a9d34aea9c..7f2ac79b4f 100644 --- a/src/gpu/GrPipeline.h +++ b/src/gpu/GrPipeline.h @@ -55,27 +55,36 @@ public: kSnapVerticesToPixelCenters_Flag = 0x2, }; - struct CreateArgs { + struct InitArgs { uint32_t fFlags = 0; GrDrawFace fDrawFace = GrDrawFace::kBoth; const GrProcessorSet* fProcessors = nullptr; const GrProcessorSet::FragmentProcessorAnalysis* fAnalysis; const GrUserStencilSettings* fUserStencil = &GrUserStencilSettings::kUnused; - GrAppliedClip* fAppliedClip = nullptr; + const GrAppliedClip* fAppliedClip = nullptr; GrRenderTarget* fRenderTarget = nullptr; const GrCaps* fCaps = nullptr; GrXferProcessor::DstTexture fDstTexture; }; - /** Creates a pipeline into a pre-allocated buffer */ - static GrPipeline* CreateAt(void* memory, const CreateArgs&, GrPipelineOptimizations*); + /** + * A Default constructed pipeline is unusable until init() is called. + **/ + GrPipeline() = default; /** * Creates a simple pipeline with default settings and no processors. The provided blend mode - * must be "Porter Duff" (<= kLastCoeffMode). + * must be "Porter Duff" (<= kLastCoeffMode). This pipeline is initialized without requiring + * a call to init(). **/ GrPipeline(GrRenderTarget*, SkBlendMode); + /** (Re)initializes a pipeline. After initialization the pipeline can be used. */ + GrPipelineOptimizations init(const InitArgs&); + + /** True if the pipeline has been initialized. */ + bool isInitialized() const { return SkToBool(fRenderTarget.get()); } + /// @} /////////////////////////////////////////////////////////////////////////// @@ -195,8 +204,6 @@ public: GrDrawFace getDrawFace() const { return static_cast(fDrawFace); } private: - GrPipeline() { /** Initialized in factory function*/ } - /** This is a continuation of the public "Flags" enum. */ enum PrivateFlags { kDisableOutputConversionToSRGB_Flag = 0x4, diff --git a/src/gpu/GrPipelineBuilder.h b/src/gpu/GrPipelineBuilder.h index 294f409b96..17dfe90ddd 100644 --- a/src/gpu/GrPipelineBuilder.h +++ b/src/gpu/GrPipelineBuilder.h @@ -128,7 +128,7 @@ public: /// @} - void initPipelineCreateArgs(GrPipeline::CreateArgs* args) const { + void getPipelineInitArgs(GrPipeline::InitArgs* args) const { args->fFlags = fFlags; args->fUserStencil = fUserStencilSettings; args->fDrawFace = fDrawFace; diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp index eb80cbc819..4fc898c3ac 100644 --- a/src/gpu/GrRenderTargetOpList.cpp +++ b/src/gpu/GrRenderTargetOpList.cpp @@ -289,8 +289,8 @@ void GrRenderTargetOpList::addDrawOp(const GrPipelineBuilder& pipelineBuilder, GrProcessorSet::FragmentProcessorAnalysis analysis; op->analyzeProcessors(&analysis, pipelineBuilder.processors(), appliedClip, *this->caps()); - GrPipeline::CreateArgs args; - pipelineBuilder.initPipelineCreateArgs(&args); + GrPipeline::InitArgs args; + pipelineBuilder.getPipelineInitArgs(&args); args.fAppliedClip = &appliedClip; // This forces instantiation of the render target. Pipeline creation is moving to flush time // by which point instantiation must have occurred anyway. @@ -331,10 +331,7 @@ void GrRenderTargetOpList::addDrawOp(const GrPipelineBuilder& pipelineBuilder, return; } } - - if (!op->installPipeline(args)) { - return; - } + op->initPipeline(args); #ifdef ENABLE_MDB SkASSERT(fSurface); diff --git a/src/gpu/ops/GrDrawOp.cpp b/src/gpu/ops/GrDrawOp.cpp deleted file mode 100644 index e3e14bec82..0000000000 --- a/src/gpu/ops/GrDrawOp.cpp +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Copyright 2015 Google Inc. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#include "GrDrawOp.h" - -GrDrawOp::GrDrawOp(uint32_t classID) : INHERITED(classID), fPipelineInstalled(false) { } - -GrDrawOp::~GrDrawOp() { - if (fPipelineInstalled) { - this->pipeline()->~GrPipeline(); - } -} - -bool GrDrawOp::installPipeline(const GrPipeline::CreateArgs& args) { - GrPipelineOptimizations optimizations; - void* location = fPipelineStorage.get(); - if (!GrPipeline::CreateAt(location, args, &optimizations)) { - return false; - } - fPipelineInstalled = true; - this->applyPipelineOptimizations(optimizations); - return true; -} diff --git a/src/gpu/ops/GrDrawOp.h b/src/gpu/ops/GrDrawOp.h index 2c7fbc2879..1512642bb4 100644 --- a/src/gpu/ops/GrDrawOp.h +++ b/src/gpu/ops/GrDrawOp.h @@ -54,10 +54,11 @@ public: class Target; - GrDrawOp(uint32_t classID); - ~GrDrawOp() override; + GrDrawOp(uint32_t classID) : INHERITED(classID) {} - bool installPipeline(const GrPipeline::CreateArgs&); + void initPipeline(const GrPipeline::InitArgs& args) { + this->applyPipelineOptimizations(fPipeline.init(args)); + } /** * Performs analysis of the fragment processors in GrProcessorSet and GrAppliedClip using the @@ -106,8 +107,8 @@ protected: } const GrPipeline* pipeline() const { - SkASSERT(fPipelineInstalled); - return reinterpret_cast(fPipelineStorage.get()); + SkASSERT(fPipeline.isInitialized()); + return &fPipeline; } /** @@ -155,8 +156,7 @@ protected: SkTArray fInlineUploads; private: - SkAlignedSTStorage<1, GrPipeline> fPipelineStorage; - bool fPipelineInstalled; + GrPipeline fPipeline; typedef GrOp INHERITED; }; diff --git a/tests/GpuSampleLocationsTest.cpp b/tests/GpuSampleLocationsTest.cpp index 45a8ae100c..e3764a5977 100644 --- a/tests/GpuSampleLocationsTest.cpp +++ b/tests/GpuSampleLocationsTest.cpp @@ -91,24 +91,21 @@ public: virtual ~TestSampleLocationsInterface() {} }; -static GrPipeline* construct_dummy_pipeline(GrRenderTargetContext* dc, void* storage) { +static void construct_dummy_pipeline(GrRenderTargetContext* dc, GrPipeline* pipeline) { GrPipelineBuilder dummyBuilder(GrPaint(), GrAAType::kNone); GrScissorState dummyScissor; GrWindowRectsState dummyWindows; - GrPipelineOptimizations dummyOverrides; GrAppliedClip dummyAppliedClip(SkRect::MakeLargest()); GrProcessorSet::FragmentProcessorAnalysis analysis; - GrPipeline::CreateArgs args; - dummyBuilder.initPipelineCreateArgs(&args); + GrPipeline::InitArgs args; + dummyBuilder.getPipelineInitArgs(&args); args.fRenderTarget = dc->accessRenderTarget(); args.fAnalysis = &analysis; args.fCaps = dc->caps(); args.fAppliedClip = &dummyAppliedClip; args.fDstTexture = GrXferProcessor::DstTexture(); - - GrPipeline::CreateAt(storage, args, &dummyOverrides); - return reinterpret_cast(storage); + pipeline->init(args); } void assert_equal(skiatest::Reporter* reporter, const SamplePattern& pattern, @@ -149,17 +146,16 @@ void test_sampleLocations(skiatest::Reporter* reporter, TestSampleLocationsInter } // Ensure all sample locations get queried and/or cached properly. - SkAlignedSTStorage<1, GrPipeline> pipelineStorage; for (int repeat = 0; repeat < 2; ++repeat) { for (int i = 0; i < numTestPatterns; ++i) { testInterface->overrideSamplePattern(kTestPatterns[i]); for (GrRenderTargetContext* dc : {bottomUps[i].get(), topDowns[i].get()}) { - GrPipeline* dummyPipe = construct_dummy_pipeline(dc, pipelineStorage.get()); + GrPipeline dummyPipeline; + construct_dummy_pipeline(dc, &dummyPipeline); GrRenderTarget* rt = dc->accessRenderTarget(); assert_equal(reporter, kTestPatterns[i], - rt->renderTargetPriv().getMultisampleSpecs(*dummyPipe), + rt->renderTargetPriv().getMultisampleSpecs(dummyPipeline), kBottomLeft_GrSurfaceOrigin == rt->origin()); - dummyPipe->~GrPipeline(); } } } -- cgit v1.2.3