Abstracted diffuse color map out of SkLightingShaderImpl into a generic SkShader

src/core/SkLightingShader.cpp

Index: src/core/SkLightingShader.cpp
diff --git a/src/core/SkLightingShader.cpp b/src/core/SkLightingShader.cpp
index 79fe119ea1505c7b215ea9c7bd5c0cfed384e752..fdd4dd801bb4fcc5b1d7c0b8094c0ecd25c83a2b 100644
--- a/src/core/SkLightingShader.cpp
+++ b/src/core/SkLightingShader.cpp
@@ -40,24 +40,18 @@
 */
 class SkLightingShaderImpl : public SkShader {
 public:
-
     /** Create a new lighting shader that uses the provided normal map and
         lights to light the diffuse bitmap.
-        @param diffuse           the diffuse bitmap
-        @param lights            the lights applied to the normal map
-        @param diffLocalM        the local matrix for the diffuse coordinates
+        @param diffuseShader     the shader that provides the diffuse colors
         @param normalSource      the source of normals for lighting computation
+        @param lights            the lights applied to the geometry
     */
-    SkLightingShaderImpl(const SkBitmap& diffuse,
-                         const sk_sp<SkLights> lights,
-                         const SkMatrix* diffLocalM,
-                         sk_sp<SkLightingShader::NormalSource> normalSource)
-        : INHERITED(diffLocalM)
-        , fDiffuseMap(diffuse)
-        , fLights(std::move(lights)) {
-
-        fNormalSource = std::move(normalSource);
-    }
+    SkLightingShaderImpl(sk_sp<SkShader> diffuseShader,
+                         sk_sp<SkLightingShader::NormalSource> normalSource,
+                         const sk_sp<SkLights> lights)
+        : fDiffuseShader(std::move(diffuseShader))
+        , fNormalSource(std::move(normalSource))
+        , fLights(std::move(lights)) {}
 
     bool isOpaque() const override;
 
@@ -74,8 +68,8 @@ public:
         // The context takes ownership of the states. It will call their destructors
         // but will NOT free the memory.
         LightingShaderContext(const SkLightingShaderImpl&, const ContextRec&,
-                              SkBitmapProcState* diffuseState,
-                              SkLightingShader::NormalSource::Provider*);
+                              SkShader::Context* diffuseContext,
+                              SkLightingShader::NormalSource::Provider*, void* heapAllocated);
         ~LightingShaderContext() override;
 
         void shadeSpan(int x, int y, SkPMColor[], int count) override;
@@ -83,10 +77,12 @@ public:
         uint32_t getFlags() const override { return fFlags; }
 
     private:
-        SkBitmapProcState*                        fDiffuseState;
+        SkShader::Context*                        fDiffuseContext;
         SkLightingShader::NormalSource::Provider* fNormalProvider;
         uint32_t                                  fFlags;
 
+        void* fHeapAllocated;
+
         typedef SkShader::Context INHERITED;
     };
 
@@ -99,10 +95,9 @@ protected:
     Context* onCreateContext(const ContextRec&, void*) const override;
 
 private:
-    SkBitmap        fDiffuseMap;
-    sk_sp<SkLights> fLights;
-
+    sk_sp<SkShader> fDiffuseShader;
     sk_sp<SkLightingShader::NormalSource> fNormalSource;
+    sk_sp<SkLights> fLights;
 
     friend class SkLightingShader;
 
@@ -126,12 +121,8 @@ private:
 
 class LightingFP : public GrFragmentProcessor {
 public:
-    LightingFP(GrTexture* diffuse, const SkMatrix& diffMatrix, const GrTextureParams& diffParams,
-               sk_sp<SkLights> lights, sk_sp<GrFragmentProcessor> normalFP)
-        : fDiffDeviceTransform(kLocal_GrCoordSet, diffMatrix, diffuse, diffParams.filterMode())
-        , fDiffuseTextureAccess(diffuse, diffParams) {
-        this->addCoordTransform(&fDiffDeviceTransform);
-        this->addTextureAccess(&fDiffuseTextureAccess);
+    LightingFP(sk_sp<GrFragmentProcessor> diffuseFP, sk_sp<GrFragmentProcessor> normalFP,
+               sk_sp<SkLights> lights) {
 
         // fuse all ambient lights into a single one
         fAmbientColor.set(0.0f, 0.0f, 0.0f);
@@ -145,6 +136,7 @@ public:
             }
         }
 
+        this->registerChildProcessor(std::move(diffuseFP));
         this->registerChildProcessor(std::move(normalFP));
         this->initClassID<LightingFP>();
     }
@@ -178,14 +170,12 @@ public:
                                                           kVec3f_GrSLType, kDefault_GrSLPrecision,
                                                           "AmbientColor", &ambientColorUniName);
 
-            fragBuilder->codeAppend("vec4 diffuseColor = ");
-            fragBuilder->appendTextureLookupAndModulate(args.fInputColor, args.fTexSamplers[0],
-                                                args.fCoords[0].c_str(),
-                                                args.fCoords[0].getType());
-            fragBuilder->codeAppend(";");
+            SkString dstDiffuseName("dstDiffuse");
+            this->emitChild(0, args.fInputColor, &dstDiffuseName, args);
+            fragBuilder->codeAppendf("vec4 diffuseColor = %s;", dstDiffuseName.c_str());
 
             SkString dstNormalName("dstNormal");
-            this->emitChild(0, nullptr, &dstNormalName, args);
+            this->emitChild(1, nullptr, &dstNormalName, args);
 
             fragBuilder->codeAppendf("vec3 normal = %s.xyz;", dstNormalName.c_str());
             fragBuilder->codeAppendf("float NdotL = clamp(dot(normal, %s), 0.0, 1.0);",
@@ -257,15 +247,11 @@ private:
 
     bool onIsEqual(const GrFragmentProcessor& proc) const override {
         const LightingFP& lightingFP = proc.cast<LightingFP>();
-        return fDiffDeviceTransform == lightingFP.fDiffDeviceTransform &&
-               fDiffuseTextureAccess == lightingFP.fDiffuseTextureAccess &&
-               fLightDir == lightingFP.fLightDir &&
+        return fLightDir == lightingFP.fLightDir &&
                fLightColor == lightingFP.fLightColor &&
                fAmbientColor == lightingFP.fAmbientColor;
     }
 
-    GrCoordTransform fDiffDeviceTransform;
-    GrTextureAccess  fDiffuseTextureAccess;
     SkVector3        fLightDir;
     SkColor3f        fLightColor;
     SkColor3f        fAmbientColor;
@@ -273,66 +259,20 @@ private:
 
 ////////////////////////////////////////////////////////////////////////////
 
-static bool make_mat(const SkBitmap& bm,
-                     const SkMatrix& localMatrix1,
-                     const SkMatrix* localMatrix2,
-                     SkMatrix* result) {
-
-    result->setIDiv(bm.width(), bm.height());
-
-    SkMatrix lmInverse;
-    if (!localMatrix1.invert(&lmInverse)) {
-        return false;
-    }
-    if (localMatrix2) {
-        SkMatrix inv;
-        if (!localMatrix2->invert(&inv)) {
-            return false;
-        }
-        lmInverse.postConcat(inv);
-    }
-    result->preConcat(lmInverse);
-
-    return true;
-}
-
 sk_sp<GrFragmentProcessor> SkLightingShaderImpl::asFragmentProcessor(
                                                      GrContext* context,
                                                      const SkMatrix& viewM,
                                                      const SkMatrix* localMatrix,
                                                      SkFilterQuality filterQuality,
                                                      SkSourceGammaTreatment gammaTreatment) const {
-    // we assume diffuse and normal maps have same width and height
-    // TODO: support different sizes, will be addressed when diffuse maps are factored out of
-    //       SkLightingShader in a future CL
-    SkMatrix diffM;
-
-    if (!make_mat(fDiffuseMap, this->getLocalMatrix(), localMatrix, &diffM)) {
-        return nullptr;
-    }
-
-    bool doBicubic;
-    GrTextureParams::FilterMode diffFilterMode = GrSkFilterQualityToGrFilterMode(
-                                                    SkTMin(filterQuality, kMedium_SkFilterQuality),
-                                                    viewM,
-                                                    this->getLocalMatrix(),
-                                                    &doBicubic);
-    SkASSERT(!doBicubic);
-
-    // TODO: support other tile modes
-    GrTextureParams diffParams(kClamp_TileMode, diffFilterMode);
-    SkAutoTUnref<GrTexture> diffuseTexture(GrRefCachedBitmapTexture(context, fDiffuseMap,
-                                                                    diffParams, gammaTreatment));
-    if (!diffuseTexture) {
-        SkErrorInternals::SetError(kInternalError_SkError, "Couldn't convert bitmap to texture.");
-        return nullptr;
-    }
-
+    sk_sp<GrFragmentProcessor> diffuseFP(
+            fDiffuseShader->asFragmentProcessor(context, viewM, localMatrix, filterQuality,
+                                               gammaTreatment));
     sk_sp<GrFragmentProcessor> normalFP(
             fNormalSource->asFragmentProcessor(context, viewM, localMatrix, filterQuality,
                                                gammaTreatment));
     sk_sp<GrFragmentProcessor> inner (
-            new LightingFP(diffuseTexture, diffM, diffParams, fLights, std::move(normalFP)));
+            new LightingFP(std::move(diffuseFP), std::move(normalFP), fLights));
 
     return GrFragmentProcessor::MulOutputByInputAlpha(std::move(inner));
 }
@@ -342,17 +282,18 @@ sk_sp<GrFragmentProcessor> SkLightingShaderImpl::asFragmentProcessor(
 ////////////////////////////////////////////////////////////////////////////
 
 bool SkLightingShaderImpl::isOpaque() const {
-    return fDiffuseMap.isOpaque();
+    return fDiffuseShader->isOpaque();
 }
 
 SkLightingShaderImpl::LightingShaderContext::LightingShaderContext(
-        const SkLightingShaderImpl& shader, const ContextRec& rec, SkBitmapProcState* diffuseState,
-        SkLightingShader::NormalSource::Provider* normalProvider)
+        const SkLightingShaderImpl& shader, const ContextRec& rec,
+        SkShader::Context* diffuseContext, SkLightingShader::NormalSource::Provider* normalProvider,
+        void* heapAllocated)
     : INHERITED(shader, rec)
-    , fDiffuseState(diffuseState)
-    , fNormalProvider(normalProvider) {
-    const SkPixmap& pixmap = fDiffuseState->fPixmap;
-    bool isOpaque = pixmap.isOpaque();
+    , fDiffuseContext(diffuseContext)
+    , fNormalProvider(normalProvider)
+    , fHeapAllocated(heapAllocated) {
+    bool isOpaque = shader.isOpaque();
 
     // update fFlags
     uint32_t flags = 0;
@@ -364,10 +305,12 @@ SkLightingShaderImpl::LightingShaderContext::LightingShaderContext(
 }
 
 SkLightingShaderImpl::LightingShaderContext::~LightingShaderContext() {
-    // The bitmap proc states have been created outside of the context on memory that will be freed
-    // elsewhere. Call the destructors but leave the freeing of the memory to the caller.
-    fDiffuseState->~SkBitmapProcState();
+    // The dependencies have been created outside of the context on memory that was allocated by
+    // the onCreateContext() method. Call the destructors and free the memory.
+    fDiffuseContext->~Context();
     fNormalProvider->~Provider();
+
+    free(fHeapAllocated);
 }
 
 static inline SkPMColor convert(SkColor3f color, U8CPU a) {
@@ -394,39 +337,23 @@ static inline SkPMColor convert(SkColor3f color, U8CPU a) {
 
 // larger is better (fewer times we have to loop), but we shouldn't
 // take up too much stack-space (each one here costs 16 bytes)
-#define TMP_COUNT 16
-#define BUFFER_MAX ((int)(TMP_COUNT * sizeof(uint32_t)))
+#define BUFFER_MAX 16
 void SkLightingShaderImpl::LightingShaderContext::shadeSpan(int x, int y,
                                                             SkPMColor result[], int count) {
     const SkLightingShaderImpl& lightShader = static_cast<const SkLightingShaderImpl&>(fShader);
 
-    uint32_t  tmpColor[TMP_COUNT];
-    SkPMColor tmpColor2[2*TMP_COUNT];
-
-    SkBitmapProcState::MatrixProc   diffMProc = fDiffuseState->getMatrixProc();
-    SkBitmapProcState::SampleProc32 diffSProc = fDiffuseState->getSampleProc32();
-
-    int max = fDiffuseState->maxCountForBufferSize(BUFFER_MAX);
-
-    SkASSERT(fDiffuseState->fPixmap.addr());
-
-    SkASSERT(max <= BUFFER_MAX);
+    SkPMColor diffuse[BUFFER_MAX];
     SkPoint3 normals[BUFFER_MAX];
 
     do {
-        int n = count;
-        if (n > max) {
-            n = max;
-        }
-
-        diffMProc(*fDiffuseState, tmpColor, n, x, y);
-        diffSProc(*fDiffuseState, tmpColor, n, tmpColor2);
+        int n = SkTMin(count, BUFFER_MAX);
 
+        fDiffuseContext->shadeSpan(x, y, diffuse, n);
         fNormalProvider->fillScanLine(x, y, normals, n);
 
         for (int i = 0; i < n; ++i) {
 
-            SkColor diffColor = SkUnPreMultiply::PMColorToColor(tmpColor2[i]);
+            SkColor diffColor = SkUnPreMultiply::PMColorToColor(diffuse[i]);
 
             SkColor3f accum = SkColor3f::Make(0.0f, 0.0f, 0.0f);
             // This is all done in linear unpremul color space (each component 0..255.0f though)
@@ -465,19 +392,10 @@ void SkLightingShaderImpl::toString(SkString* str) const {
 #endif
 
 sk_sp<SkFlattenable> SkLightingShaderImpl::CreateProc(SkReadBuffer& buf) {
-    SkMatrix diffLocalM;
-    bool hasDiffLocalM = buf.readBool();
-    if (hasDiffLocalM) {
-        buf.readMatrix(&diffLocalM);
-    } else {
-        diffLocalM.reset();
-    }
 
-    SkBitmap diffuse;
-    if (!buf.readBitmap(&diffuse)) {
-        return nullptr;
-    }
-    diffuse.setImmutable();
+    // Discarding SkShader flattenable params
+    bool hasLocalMatrix = buf.readBool();
+    SkASSERT(!hasLocalMatrix);
 
     int numLights = buf.readInt();
 
@@ -506,16 +424,15 @@ sk_sp<SkFlattenable> SkLightingShaderImpl::CreateProc(SkReadBuffer& buf) {
 
     sk_sp<SkLightingShader::NormalSource> normalSource(
             buf.readFlattenable<SkLightingShader::NormalSource>());
+    sk_sp<SkShader> diffuseShader(buf.readFlattenable<SkShader>());
 
-    return sk_make_sp<SkLightingShaderImpl>(diffuse, std::move(lights), &diffLocalM,
-                                            std::move(normalSource));
+    return sk_make_sp<SkLightingShaderImpl>(std::move(diffuseShader), std::move(normalSource),
+                                            std::move(lights));
 }
 
 void SkLightingShaderImpl::flatten(SkWriteBuffer& buf) const {
     this->INHERITED::flatten(buf);
 
-    buf.writeBitmap(fDiffuseMap);
-
     buf.writeInt(fLights->numLights());
     for (int l = 0; l < fLights->numLights(); ++l) {
         const SkLights::Light& light = fLights->light(l);
@@ -530,42 +447,34 @@ void SkLightingShaderImpl::flatten(SkWriteBuffer& buf) const {
     }
 
     buf.writeFlattenable(fNormalSource.get());
+    buf.writeFlattenable(fDiffuseShader.get());
 }
 
 size_t SkLightingShaderImpl::onContextSize(const ContextRec& rec) const {
-    return sizeof(LightingShaderContext) +
-           sizeof(SkBitmapProcState)     +
-           fNormalSource->providerSize(rec);
+    return sizeof(LightingShaderContext);
 }
 
 SkShader::Context* SkLightingShaderImpl::onCreateContext(const ContextRec& rec,
                                                          void* storage) const {
 
-    SkMatrix diffTotalInv;
-    // computeTotalInverse was called in SkShader::createContext so we know it will succeed
-    SkAssertResult(this->computeTotalInverse(rec, &diffTotalInv));
-
-    void* diffuseStateStorage = (char*)storage + sizeof(LightingShaderContext);
-    SkBitmapProcState* diffuseState = new (diffuseStateStorage) SkBitmapProcState(fDiffuseMap,
-                                              SkShader::kClamp_TileMode, SkShader::kClamp_TileMode,
-                                                                    SkMipMap::DeduceTreatment(rec));
-    SkASSERT(diffuseState);
-    if (!diffuseState->setup(diffTotalInv, *rec.fPaint)) {
-        diffuseState->~SkBitmapProcState();
-        return nullptr;
-    }
-    void* normalProviderStorage = (char*)storage +
-                                  sizeof(LightingShaderContext) +
-                                  sizeof(SkBitmapProcState);
+    size_t heapRequired = fDiffuseShader->contextSize(rec) + fNormalSource->providerSize(rec);
+    void* heapAllocated = malloc(heapRequired); // Freed on ~LightingShaderContext()

[dvonbeck, 2016/06/17 16:05:51]

malloc and free. Should I be using some SkMalloc or something?

+
+    void* diffuseContextStorage = (char*)heapAllocated;
+
+    SkShader::Context* diffuseContext = fDiffuseShader->createContext(rec, diffuseContextStorage);
 
+    void* normalProviderStorage = (char*)heapAllocated + fDiffuseShader->contextSize(rec);
     SkLightingShader::NormalSource::Provider* normalProvider =
             fNormalSource->asProvider(rec, normalProviderStorage);
     if (!normalProvider) {
-        diffuseState->~SkBitmapProcState();
+        diffuseContext->~Context();
+        free(heapAllocated);
         return nullptr;
     }
 
-    return new (storage) LightingShaderContext(*this, rec, diffuseState, normalProvider);
+    return new (storage) LightingShaderContext(*this, rec, diffuseContext, normalProvider,
+                                               heapAllocated);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -581,8 +490,12 @@ sk_sp<SkShader> SkLightingShader::Make(const SkBitmap& diffuse, sk_sp<SkLights>
         return nullptr;
     }
 
-    return sk_make_sp<SkLightingShaderImpl>(diffuse, std::move(lights), diffLocalM,
-                                            std::move(normalSource));
+    // TODO: support other tile modes
+    sk_sp<SkShader> diffuseShader = SkBitmapProcShader::MakeBitmapShader(diffuse,
+            SkShader::kClamp_TileMode, SkShader::kClamp_TileMode, diffLocalM);
+
+    return sk_make_sp<SkLightingShaderImpl>(std::move(diffuseShader), std::move(normalSource),
+                                            std::move(lights));
 }
 
 ///////////////////////////////////////////////////////////////////////////////