Refactoring of GPU NormalMap handling out into its own class

src/core/SkLightingShader.h

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef SkLightingShader_DEFINED
 #define SkLightingShader_DEFINED
 
 #include "SkLights.h"
 #include "SkShader.h"
 
 class SkBitmap;
 class SkMatrix;
 
 class SK_API SkLightingShader {
 public:
+    // TODO does this need to be tagged with SK_API?
+    class NormalSource : public SkFlattenable {

[egdaniel, 2016/06/10 14:23:28]

For SK_API, it is believed that having it on the outer class should extend it to
all the inner objects including nested classes. So for now lets leave it out,
put it becomes very obvious if certain builds fail that this is the issue and we
can add it back.

So talking with Mike Klein, we don't want this class to subclass SkFlattenable.
Instead just add functions (and just match Flattenable names) for the various
reads and writes for flattening. Then just call them from SkLightingShader's
flatten commands. This way you don't need this up in the enum.

[dvonbeck, 2016/06/10 15:22:29]

The problem with the approach you suggested is that normal source is an abstract
class. To be able to recreate it from a byte stream without subclassing
SkFlattenable I would need to encode the subclass in an enum that I set and then
have a switch statement that forks into the appropriate constructor, etc. I feel
like NormalSource should take advantage of the fact that serialization is
already implemented. Let me know what you think.

[dvonbeck, 2016/06/10 18:31:21]

Done.

+

[egdaniel, 2016/06/10 14:23:28]

remove \n between class and public

[dvonbeck, 2016/06/10 15:22:29]

Done.

+    public:
+
+        virtual ~NormalSource();
+
+        virtual sk_sp<GrFragmentProcessor> asFragmentProcessor(
+                GrContext* context,
+                const SkMatrix& viewM,
+                const SkMatrix* localMatrix,
+                SkFilterQuality filterQuality,
+                SkSourceGammaTreatment gammaTreatment) const = 0;
+
+
+        // SK_TO_STRING_VIRT() // TODO do I need this one? just copied from SkShader
+        SK_DEFINE_FLATTENABLE_TYPE(NormalSource)

[egdaniel, 2016/06/10 14:23:28]

These macros just declare/define various flattenable functions. If you want to
use the declare ones to get the names that is fine, or just declare them
manually yourself (especially since we won't be subclassing flattenable).

[dvonbeck, 2016/06/10 15:22:29]

Acknowledged.

+    };
+
+    class NormalMapSource {
+    public:
+        static sk_sp<NormalSource> Make(const SkBitmap& normal, const SkVector& invNormRotation,
+                                        const SkMatrix* normLocalM);
+
+        // TODO Why? Shouldn't the impls be registered?
+        SK_DECLARE_FLATTENABLE_REGISTRAR_GROUP()
+    };
+
     /** Returns a shader that lights the diffuse and normal maps with a set of lights.
 
         It returns a shader with a reference count of 1.
         The caller should decrement the shader's reference count when done with the shader.
         It is an error for count to be < 2.
         @param  diffuse     the diffuse bitmap
         @param  normal      the normal map
         @param  lights       the lights applied to the normal map
         @param  invNormRotation rotation applied to the normal map's normals
         @param  diffLocalMatrix the local matrix for the diffuse texture
@@ skipping 17 matching lines @@
         (127, 127, 0).
     */
     static sk_sp<SkShader> Make(const SkBitmap& diffuse, const SkBitmap& normal,
                                 sk_sp<SkLights> lights, const SkVector& invNormRotation,
                                 const SkMatrix* diffLocalMatrix, const SkMatrix* normLocalMatrix);
 
     SK_DECLARE_FLATTENABLE_REGISTRAR_GROUP()
 };
 
 #endif