Use construct/init pattern with GrPipeline to replace CreateAt.

Change-Id: Ic6c7432a9a298a143ce4f2431e94c89a0ea79793
Reviewed-on: https://skia-review.googlesource.com/8938
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>

9 files changed, 62 insertions, 96 deletions

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<int16_t>(args.fDrawFace);
+    fRenderTarget.reset(args.fRenderTarget);
+    fScissorState = args.fAppliedClip->scissorState();
+    fWindowRectsState = args.fAppliedClip->windowRectsState();
+    fUserStencilSettings = args.fUserStencil;
+    fDrawFace = static_cast<int16_t>(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<GrXferProcessor> 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<GrDrawFace>(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<const GrPipeline*>(fPipelineStorage.get());
+        SkASSERT(fPipeline.isInitialized());
+        return &fPipeline;
     }
 
     /**
@@ -155,8 +156,7 @@ protected:
     SkTArray<QueuedUpload>                          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<GrPipeline*>(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();
             }
         }
     }