Revert "Remove texture sampling from GrConfigConversionEffect"

This reverts commit 3e7cddaf32e280fe9f32eec5bfdd8168ca4941b6.

Reason for revert: Precision issue on some mobile GPUs.

Original change's description:
> Remove texture sampling from GrConfigConversionEffect
>
> Re-land fixed version of https://skia-review.googlesource.com/c/10026/
>
> BUG=skia:
>
> Change-Id: I0754ffb72da2966eb57e5cd7ec818b1cdce84a74
> Reviewed-on: https://skia-review.googlesource.com/10056
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
>

TBR=bsalomon@google.com,robertphillips@google.com,brianosman@google.com,reviews@skia.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Change-Id: I66f52efe191b170612775e26d84a2af3d3f856af
Reviewed-on: https://skia-review.googlesource.com/10118
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>

diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index f6302a9a1e3628e8f935871a3fca0bd46a97ce46..56d811761b3d3cc52caa1e2aa44a87edd7ffeba7 100644 (file)
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -870,18 +870,18 @@ sk_sp<GrFragmentProcessor> GrContext::createPMToUPMEffect(GrTexture* texture,
     ASSERT_SINGLE_OWNER
     // We should have already called this->testPMConversionsIfNecessary().
     SkASSERT(fDidTestPMConversions);
-    GrPixelConfig config = texture->config();
-    sk_sp<GrFragmentProcessor> fp = GrSimpleTextureEffect::Make(texture, nullptr, matrix);
-    if (kRGBA_half_GrPixelConfig == config) {
-        return GrFragmentProcessor::UnpremulOutput(std::move(fp));
-    } else if (kRGBA_8888_GrPixelConfig == config || kBGRA_8888_GrPixelConfig == config) {
+    if (kRGBA_half_GrPixelConfig == texture->config()) {
+        return GrFragmentProcessor::UnpremulOutput(
+                GrSimpleTextureEffect::Make(texture, nullptr, matrix));
+    } else {
         GrConfigConversionEffect::PMConversion pmToUPM =
             static_cast<GrConfigConversionEffect::PMConversion>(fPMToUPMConversion);
         if (GrConfigConversionEffect::kPMConversionCnt != pmToUPM) {
-            return GrConfigConversionEffect::Make(std::move(fp), pmToUPM);
+            return GrConfigConversionEffect::Make(texture, pmToUPM, matrix);
+        } else {
+            return nullptr;
         }
     }
-    return nullptr;
 }
 
 sk_sp<GrFragmentProcessor> GrContext::createPMToUPMEffect(sk_sp<GrTextureProxy> proxy,
@@ -889,19 +889,20 @@ sk_sp<GrFragmentProcessor> GrContext::createPMToUPMEffect(sk_sp<GrTextureProxy>
     ASSERT_SINGLE_OWNER
     // We should have already called this->testPMConversionsIfNecessary().
     SkASSERT(fDidTestPMConversions);
-    GrPixelConfig config = proxy->config();
-    sk_sp<GrFragmentProcessor> fp = GrSimpleTextureEffect::Make(this->resourceProvider(),
-                                                                std::move(proxy), nullptr, matrix);
-    if (kRGBA_half_GrPixelConfig == config) {
-        return GrFragmentProcessor::UnpremulOutput(std::move(fp));
-    } else if (kRGBA_8888_GrPixelConfig == config || kBGRA_8888_GrPixelConfig == config) {
+    if (kRGBA_half_GrPixelConfig == proxy->config()) {
+        return GrFragmentProcessor::UnpremulOutput(
+                GrSimpleTextureEffect::Make(this->resourceProvider(), std::move(proxy),
+                                            nullptr, matrix));
+    } else {
         GrConfigConversionEffect::PMConversion pmToUPM =
             static_cast<GrConfigConversionEffect::PMConversion>(fPMToUPMConversion);
         if (GrConfigConversionEffect::kPMConversionCnt != pmToUPM) {
-            return GrConfigConversionEffect::Make(std::move(fp), pmToUPM);
+            return GrConfigConversionEffect::Make(this->resourceProvider(), std::move(proxy),
+                                                  pmToUPM, matrix);
+        } else {
+            return nullptr;
         }
     }
-    return nullptr;
 }
 
 sk_sp<GrFragmentProcessor> GrContext::createUPMToPMEffect(sk_sp<GrTextureProxy> proxy,
@@ -909,19 +910,20 @@ sk_sp<GrFragmentProcessor> GrContext::createUPMToPMEffect(sk_sp<GrTextureProxy>
     ASSERT_SINGLE_OWNER
     // We should have already called this->testPMConversionsIfNecessary().
     SkASSERT(fDidTestPMConversions);
-    GrPixelConfig config = proxy->config();
-    sk_sp<GrFragmentProcessor> fp = GrSimpleTextureEffect::Make(this->resourceProvider(),
-                                                                std::move(proxy), nullptr, matrix);
-    if (kRGBA_half_GrPixelConfig == config) {
-        return GrFragmentProcessor::PremulOutput(std::move(fp));
-    } else if (kRGBA_8888_GrPixelConfig == config || kBGRA_8888_GrPixelConfig == config) {
+    if (kRGBA_half_GrPixelConfig == proxy->config()) {
+        return GrFragmentProcessor::PremulOutput(
+                GrSimpleTextureEffect::Make(this->resourceProvider(), std::move(proxy),
+                                            nullptr, matrix));
+    } else {
         GrConfigConversionEffect::PMConversion upmToPM =
             static_cast<GrConfigConversionEffect::PMConversion>(fUPMToPMConversion);
         if (GrConfigConversionEffect::kPMConversionCnt != upmToPM) {
-            return GrConfigConversionEffect::Make(std::move(fp), upmToPM);
+            return GrConfigConversionEffect::Make(this->resourceProvider(), std::move(proxy),
+                                                  upmToPM, matrix);
+        } else {
+            return nullptr;
         }
     }
-    return nullptr;
 }
 
 bool GrContext::validPMUPMConversionExists(GrPixelConfig config) const {

diff --git a/src/gpu/effects/GrConfigConversionEffect.cpp b/src/gpu/effects/GrConfigConversionEffect.cpp
index b2c9e3e4fd28996517689e709947b38a3af613b8..2c6589d6aab74a97e243b0a53f156e71e104c7e8 100644 (file)
--- a/src/gpu/effects/GrConfigConversionEffect.cpp
+++ b/src/gpu/effects/GrConfigConversionEffect.cpp
@@ -10,6 +10,7 @@
 #include "GrClip.h"
 #include "GrContext.h"
 #include "GrRenderTargetContext.h"
+#include "GrSimpleTextureEffect.h"
 #include "SkMatrix.h"
 #include "glsl/GrGLSLFragmentProcessor.h"
 #include "glsl/GrGLSLFragmentShaderBuilder.h"
@@ -29,11 +30,10 @@ public:
 
         fragBuilder->codeAppendf("%s;", tmpDecl.c_str());
 
-        if (nullptr == args.fInputColor) {
-            // could optimize this case, but we aren't for now.
-            args.fInputColor = "vec4(1)";
-        }
-        fragBuilder->codeAppendf("%s = %s;", tmpVar.c_str(), args.fInputColor);
+        fragBuilder->codeAppendf("%s = ", tmpVar.c_str());
+        fragBuilder->appendTextureLookup(args.fTexSamplers[0], args.fTransformedCoords[0].c_str(),
+                                         args.fTransformedCoords[0].getType());
+        fragBuilder->codeAppend(";");
 
         switch (pmConversion) {
             case GrConfigConversionEffect::kMulByAlpha_RoundUp_PMConversion:
@@ -68,6 +68,10 @@ public:
                 break;
         }
         fragBuilder->codeAppendf("%s = %s;", args.fOutputColor, tmpVar.c_str());
+
+        SkString modulate;
+        GrGLSLMulVarBy4f(&modulate, args.fOutputColor, args.fInputColor);
+        fragBuilder->codeAppend(modulate.c_str());
     }
 
     static inline void GenKey(const GrProcessor& processor, const GrShaderCaps&,
@@ -83,11 +87,29 @@ private:
 };
 
 ///////////////////////////////////////////////////////////////////////////////
+GrConfigConversionEffect::GrConfigConversionEffect(GrTexture* texture,
+                                                   PMConversion pmConversion,
+                                                   const SkMatrix& matrix)
+        : INHERITED(texture, nullptr, matrix, kNone_OptimizationFlags)
+        , fPMConversion(pmConversion) {
+    this->initClassID<GrConfigConversionEffect>();
+    // We expect to get here with non-BGRA/RGBA only if we're doing not doing a premul/unpremul
+    // conversion.
+    SkASSERT(kRGBA_8888_GrPixelConfig == texture->config() ||
+             kBGRA_8888_GrPixelConfig == texture->config());
+}
 
-GrConfigConversionEffect::GrConfigConversionEffect(PMConversion pmConversion)
-        : INHERITED(kNone_OptimizationFlags)
+GrConfigConversionEffect::GrConfigConversionEffect(GrResourceProvider* resourceProvider,
+                                                   sk_sp<GrTextureProxy> proxy,
+                                                   PMConversion pmConversion,
+                                                   const SkMatrix& matrix)
+        : INHERITED(resourceProvider, kNone_OptimizationFlags, proxy, nullptr, matrix)
         , fPMConversion(pmConversion) {
     this->initClassID<GrConfigConversionEffect>();
+    // We expect to get here with non-BGRA/RGBA only if we're doing not doing a premul/unpremul
+    // conversion.
+    SkASSERT(kRGBA_8888_GrPixelConfig == proxy->config() ||
+             kBGRA_8888_GrPixelConfig == proxy->config());
 }
 
 bool GrConfigConversionEffect::onIsEqual(const GrFragmentProcessor& s) const {
@@ -108,7 +130,10 @@ GR_DEFINE_FRAGMENT_PROCESSOR_TEST(GrConfigConversionEffect);
 #if GR_TEST_UTILS
 sk_sp<GrFragmentProcessor> GrConfigConversionEffect::TestCreate(GrProcessorTestData* d) {
     PMConversion pmConv = static_cast<PMConversion>(d->fRandom->nextULessThan(kPMConversionCnt));
-    return sk_sp<GrFragmentProcessor>(new GrConfigConversionEffect(pmConv));
+    return sk_sp<GrFragmentProcessor>(new GrConfigConversionEffect(
+                            d->resourceProvider(),
+                            d->textureProxy(GrProcessorUnitTest::kSkiaPMTextureIdx),
+                            pmConv, GrTest::TestMatrix(d->fRandom)));
 }
 #endif
 
@@ -201,11 +226,14 @@ void GrConfigConversionEffect::TestForPreservingPMConversions(GrContext* context
         GrPaint paint1;
         GrPaint paint2;
         GrPaint paint3;
-        sk_sp<GrFragmentProcessor> pmToUPM(new GrConfigConversionEffect(*pmToUPMRule));
-        sk_sp<GrFragmentProcessor> upmToPM(new GrConfigConversionEffect(*upmToPMRule));
-
-        paint1.addColorTextureProcessor(resourceProvider, dataProxy, nullptr, SkMatrix::I());
-        paint1.addColorFragmentProcessor(pmToUPM);
+        sk_sp<GrFragmentProcessor> pmToUPM1(new GrConfigConversionEffect(
+                resourceProvider, dataProxy, *pmToUPMRule, SkMatrix::I()));
+        sk_sp<GrFragmentProcessor> upmToPM(new GrConfigConversionEffect(
+                resourceProvider, readRTC->asTextureProxyRef(), *upmToPMRule, SkMatrix::I()));
+        sk_sp<GrFragmentProcessor> pmToUPM2(new GrConfigConversionEffect(
+                resourceProvider, tempRTC->asTextureProxyRef(), *pmToUPMRule, SkMatrix::I()));
+
+        paint1.addColorFragmentProcessor(std::move(pmToUPM1));
         paint1.setPorterDuffXPFactory(SkBlendMode::kSrc);
 
         readRTC->fillRectToRect(GrNoClip(), std::move(paint1), GrAA::kNo, SkMatrix::I(), kDstRect,
@@ -215,17 +243,13 @@ void GrConfigConversionEffect::TestForPreservingPMConversions(GrContext* context
             continue;
         }
 
-        paint2.addColorTextureProcessor(resourceProvider, readRTC->asTextureProxyRef(), nullptr,
-                                        SkMatrix::I());
         paint2.addColorFragmentProcessor(std::move(upmToPM));
         paint2.setPorterDuffXPFactory(SkBlendMode::kSrc);
 
         tempRTC->fillRectToRect(GrNoClip(), std::move(paint2), GrAA::kNo, SkMatrix::I(), kDstRect,
                                 kSrcRect);
 
-        paint3.addColorTextureProcessor(resourceProvider, tempRTC->asTextureProxyRef(), nullptr,
-                                        SkMatrix::I());
-        paint3.addColorFragmentProcessor(std::move(pmToUPM));
+        paint3.addColorFragmentProcessor(std::move(pmToUPM2));
         paint3.setPorterDuffXPFactory(SkBlendMode::kSrc);
 
         readRTC->fillRectToRect(GrNoClip(), std::move(paint3), GrAA::kNo, SkMatrix::I(), kDstRect,
@@ -251,12 +275,28 @@ void GrConfigConversionEffect::TestForPreservingPMConversions(GrContext* context
     }
 }
 
-sk_sp<GrFragmentProcessor> GrConfigConversionEffect::Make(sk_sp<GrFragmentProcessor> fp,
-                                                          PMConversion pmConversion) {
-    if (!fp) {
+sk_sp<GrFragmentProcessor> GrConfigConversionEffect::Make(GrTexture* texture,
+                                                          PMConversion pmConversion,
+                                                          const SkMatrix& matrix) {
+    if (kRGBA_8888_GrPixelConfig != texture->config() &&
+        kBGRA_8888_GrPixelConfig != texture->config()) {
+        // The PM conversions assume colors are 0..255
+        return nullptr;
+    }
+    return sk_sp<GrFragmentProcessor>(
+        new GrConfigConversionEffect(texture, pmConversion, matrix));
+}
+
+sk_sp<GrFragmentProcessor> GrConfigConversionEffect::Make(GrResourceProvider* resourceProvider,
+                                                          sk_sp<GrTextureProxy> proxy,
+                                                          PMConversion pmConversion,
+                                                          const SkMatrix& matrix) {
+    if (kRGBA_8888_GrPixelConfig != proxy->config() &&
+        kBGRA_8888_GrPixelConfig != proxy->config()) {
+        // The PM conversions assume colors are 0..255
         return nullptr;
     }
-    sk_sp<GrFragmentProcessor> ccFP(new GrConfigConversionEffect(pmConversion));
-    sk_sp<GrFragmentProcessor> fpPipeline[] = { fp, ccFP };
-    return GrFragmentProcessor::RunInSeries(fpPipeline, 2);
+    return sk_sp<GrFragmentProcessor>(new GrConfigConversionEffect(resourceProvider,
+                                                                   std::move(proxy),
+                                                                   pmConversion, matrix));
 }

diff --git a/src/gpu/effects/GrConfigConversionEffect.h b/src/gpu/effects/GrConfigConversionEffect.h
index e5fa10cd9bfa8a1692f5ce3382c09178f7812f19..d71b228a7bc88a34b6f0b60976d8afd508e7a6c9 100644 (file)
--- a/src/gpu/effects/GrConfigConversionEffect.h
+++ b/src/gpu/effects/GrConfigConversionEffect.h
@@ -8,13 +8,15 @@
 #ifndef GrConfigConversionEffect_DEFINED
 #define GrConfigConversionEffect_DEFINED
 
-#include "GrFragmentProcessor.h"
+#include "GrSingleTextureEffect.h"
+
+class GrInvariantOutput;
 
 /**
  * This class is used to perform config conversions. Clients may want to read/write data that is
  * unpremultiplied.
  */
-class GrConfigConversionEffect : public GrFragmentProcessor {
+class GrConfigConversionEffect : public GrSingleTextureEffect {
 public:
     /**
      * The PM->UPM or UPM->PM conversions to apply.
@@ -28,11 +30,10 @@ public:
         kPMConversionCnt
     };
 
-    /**
-     *  Returns a fragment processor that calls the passed in fragment processor, and then performs
-     *  the requested premul or unpremul conversion.
-     */
-    static sk_sp<GrFragmentProcessor> Make(sk_sp<GrFragmentProcessor>, PMConversion);
+    static sk_sp<GrFragmentProcessor> Make(GrTexture*, PMConversion, const SkMatrix&);
+
+    static sk_sp<GrFragmentProcessor> Make(GrResourceProvider*, sk_sp<GrTextureProxy>,
+                                           PMConversion, const SkMatrix&);
 
     const char* name() const override { return "Config Conversion"; }
 
@@ -47,7 +48,10 @@ public:
                                                PMConversion* PMToUPMRule,
                                                PMConversion* UPMToPMRule);
 private:
-    GrConfigConversionEffect(PMConversion);
+    GrConfigConversionEffect(GrTexture*, PMConversion, const SkMatrix& matrix);
+
+    GrConfigConversionEffect(GrResourceProvider*, sk_sp<GrTextureProxy>,
+                             PMConversion, const SkMatrix& matrix);
 
     GrGLSLFragmentProcessor* onCreateGLSLInstance() const override;
 
@@ -59,7 +63,7 @@ private:
 
     GR_DECLARE_FRAGMENT_PROCESSOR_TEST;
 
-    typedef GrFragmentProcessor INHERITED;
+    typedef GrSingleTextureEffect INHERITED;
 };
 
 #endif