Separate user and raw stencil settings

src/gpu/GrPipelineBuilder.h

Index: src/gpu/GrPipelineBuilder.h
diff --git a/src/gpu/GrPipelineBuilder.h b/src/gpu/GrPipelineBuilder.h
index 2f78fc519862ca72f4cfbdabfabef3189868b81e..dab40e43acef196d5390f3c0cae06323da130cd1 100644
--- a/src/gpu/GrPipelineBuilder.h
+++ b/src/gpu/GrPipelineBuilder.h
@@ -199,57 +199,20 @@ public:
     /// @name Stencil
     ////
 
-    const GrStencilSettings& getStencil() const { return fStencilSettings; }
+    bool hasUserStencilSettings() const {
+        return &GrUserStencilSettings::kUnused != fUserStencilSettings;
+    }
+    const GrUserStencilSettings* getUserStencil() const { return fUserStencilSettings; }
 
     /**
-     * Sets the stencil settings to use for the next draw.
-     * Changing the clip has the side-effect of possibly zeroing
-     * out the client settable stencil bits. So multipass algorithms
-     * using stencil should not change the clip between passes.
+     * Sets the user stencil settings for the next draw.
+     * This class only stores pointers to stencil settings objects.
+     * The caller guarantees the pointer will remain valid until it

[bsalomon, 2016/05/10 14:06:21]

It seems like you've designed GrUserStencilSettings such that this must be true,
or am I missing a possibility?

[Chris Dalton, 2016/05/10 16:39:30]

Unfortunately it can still be declared in non-static scope. e.g:

 constexpr GrUserStencilSettings stackAllocated (...);
 pb.setUserStencil(&stackAllocated);

[bsalomon, 2016/05/10 20:10:58]

Right, of course.

+     * changes or goes out of scope.
      * @param settings  the stencil settings to use.
      */
-    void setStencil(const GrStencilSettings& settings) { fStencilSettings = settings; }
-
-    GrStencilSettings* stencil() { return &fStencilSettings; }
-
-    /**
-     * AutoRestoreStencil
-     *
-     * This simple struct saves and restores the stencil settings
-     * This class can transiently modify its "const" GrPipelineBuilder object but will restore it
-     * when done - so it is notionally "const" correct.
-     */
-    class AutoRestoreStencil : public ::SkNoncopyable {
-    public:
-        AutoRestoreStencil() : fPipelineBuilder(nullptr) {}
-
-        AutoRestoreStencil(const GrPipelineBuilder& ds) : fPipelineBuilder(nullptr) { this->set(&ds); }
-
-        ~AutoRestoreStencil() { this->set(nullptr); }
-
-        void set(const GrPipelineBuilder* ds) {
-            if (fPipelineBuilder) {
-                fPipelineBuilder->setStencil(fStencilSettings);
-            }
-            fPipelineBuilder = const_cast<GrPipelineBuilder*>(ds);
-            if (ds) {
-                fStencilSettings = ds->getStencil();
-            }
-        }
-
-        bool isSet() const { return SkToBool(fPipelineBuilder); }
-
-        void setStencil(const GrStencilSettings& settings) {
-            SkASSERT(this->isSet());
-            fPipelineBuilder->setStencil(settings);
-        }
-
-    private:
-        // notionally const (as marginalia)
-        GrPipelineBuilder*  fPipelineBuilder;
-        GrStencilSettings   fStencilSettings;
-    };
-
+    void setUserStencil(const GrUserStencilSettings* settings) { fUserStencilSettings = settings; }
+    void disableUserStencil() { fUserStencilSettings = &GrUserStencilSettings::kUnused; }
 
     /// @}
 
@@ -371,7 +334,7 @@ private:
 
     SkAutoTUnref<GrRenderTarget>            fRenderTarget;
     uint32_t                                fFlags;
-    GrStencilSettings                       fStencilSettings;
+    const GrUserStencilSettings*            fUserStencilSettings;
     DrawFace                                fDrawFace;
     mutable SkAutoTUnref<const GrXPFactory> fXPFactory;
     FragmentProcessorArray                  fColorFragmentProcessors;