Reland x4 "Drawing YUVA images does not flatten for bicubic."

Actually don't snap the other coordinate.

This reverts commit 5575e3c4ea49e6cb5c9e9382dc00f697ed5e63f9.

Change-Id: I7ef5c16ecbccf5131da595d2396243fbdefb4ddf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/279140
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
diff --git a/gm/imagefromyuvtextures.cpp b/gm/imagefromyuvtextures.cpp
index e54b266..65fed5c 100644
--- a/gm/imagefromyuvtextures.cpp
+++ b/gm/imagefromyuvtextures.cpp
@@ -12,7 +12,6 @@
 #include "include/core/SkCanvas.h"
 #include "include/core/SkColor.h"
 #include "include/core/SkColorFilter.h"
-#include "include/core/SkColorPriv.h"
 #include "include/core/SkImage.h"
 #include "include/core/SkImageInfo.h"
 #include "include/core/SkPaint.h"
@@ -20,27 +19,19 @@
 #include "include/core/SkPoint.h"
 #include "include/core/SkRefCnt.h"
 #include "include/core/SkScalar.h"
-#include "include/core/SkShader.h"
 #include "include/core/SkSize.h"
 #include "include/core/SkString.h"
-#include "include/core/SkTileMode.h"
 #include "include/core/SkTypes.h"
-#include "include/effects/SkGradientShader.h"
+#include "include/core/SkYUVAIndex.h"
 #include "include/gpu/GrBackendSurface.h"
 #include "include/gpu/GrContext.h"
 #include "include/gpu/GrTypes.h"
-#include "include/private/GrTypesPriv.h"
 #include "include/private/SkTo.h"
 #include "src/core/SkYUVMath.h"
+#include "tools/Resources.h"
 
 class GrRenderTargetContext;
 
-static sk_sp<SkColorFilter> yuv_to_rgb_colorfilter() {
-    float m[20];
-    SkColorMatrix_YUV2RGB(kJPEG_SkYUVColorSpace, m);
-    return SkColorFilters::Matrix(m);
-}
-
 namespace skiagm {
 class ImageFromYUVTextures : public GpuGM {
 public:
@@ -53,93 +44,104 @@
         return SkString("image_from_yuv_textures");
     }
 
-    SkISize onISize() override {
-        // Original image, plus each color space drawn twice
-        int numBitmaps = 2 * (kLastEnum_SkYUVColorSpace + 1) + 1;
-        return SkISize::Make(kBmpSize + 2 * kPad, numBitmaps * (kBmpSize + kPad) + kPad);
-    }
+    SkISize onISize() override { return {1420, 610}; }
 
     void onOnceBeforeDraw() override {
-        // We create an RGB bitmap and then extract YUV bmps where the U and V bitmaps are
-        // subsampled by 2 in both dimensions.
-        SkPaint paint;
-        constexpr SkColor kColors[] =
-            { SK_ColorBLUE, SK_ColorYELLOW, SK_ColorGREEN, SK_ColorWHITE };
-        paint.setShader(SkGradientShader::MakeRadial(SkPoint::Make(0,0), kBmpSize / 2.f, kColors,
-                                                     nullptr, SK_ARRAY_COUNT(kColors),
-                                                     SkTileMode::kMirror));
-        SkBitmap rgbBmp;
-        auto ii =
-                SkImageInfo::Make(kBmpSize, kBmpSize, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
-        rgbBmp.allocPixels(ii);
-        SkCanvas canvas(rgbBmp);
-        canvas.drawPaint(paint);
-        SkPMColor* rgbColors = static_cast<SkPMColor*>(rgbBmp.getPixels());
+        fRGBABmp = this->createBmpAndPlanes("images/mandrill_32.png", fYUVABmps);
+    }
 
-        SkImageInfo yinfo = SkImageInfo::Make(kBmpSize, kBmpSize, kGray_8_SkColorType,
-                                              kUnpremul_SkAlphaType);
-        fYUVBmps[0].allocPixels(yinfo);
-        SkImageInfo uinfo = SkImageInfo::Make(kBmpSize / 2, kBmpSize / 2, kGray_8_SkColorType,
-                                              kUnpremul_SkAlphaType);
-        fYUVBmps[1].allocPixels(uinfo);
-        SkImageInfo vinfo = SkImageInfo::Make(kBmpSize / 2, kBmpSize / 2, kGray_8_SkColorType,
-                                              kUnpremul_SkAlphaType);
-        fYUVBmps[2].allocPixels(vinfo);
-        unsigned char* yPixels;
-        signed char* uvPixels[2];
-        yPixels = static_cast<unsigned char*>(fYUVBmps[0].getPixels());
-        uvPixels[0] = static_cast<signed char*>(fYUVBmps[1].getPixels());
-        uvPixels[1] = static_cast<signed char*>(fYUVBmps[2].getPixels());
+    SkBitmap createBmpAndPlanes(const char* name, SkBitmap yuvaBmps[4]) {
+        SkBitmap bmp;
+        if (!GetResourceAsBitmap(name, &bmp)) {
+            return {};
+        }
+        auto ii = SkImageInfo::Make(bmp.dimensions(), kRGBA_8888_SkColorType, kPremul_SkAlphaType);
+
+        SkBitmap rgbaBmp;
+        rgbaBmp.allocPixels(ii);
+        bmp.readPixels(rgbaBmp.pixmap(), 0, 0);
+
+        SkImageInfo yaInfo = SkImageInfo::Make(rgbaBmp.dimensions(), kAlpha_8_SkColorType,
+                                               kUnpremul_SkAlphaType);
+        yuvaBmps[0].allocPixels(yaInfo);
+        SkISize uvSize = {rgbaBmp.width()/2, rgbaBmp.height()/2};
+        SkImageInfo uvInfo = SkImageInfo::Make(uvSize, kAlpha_8_SkColorType, kUnpremul_SkAlphaType);
+        yuvaBmps[1].allocPixels(uvInfo);
+        yuvaBmps[2].allocPixels(uvInfo);
+        yuvaBmps[3].allocPixels(yaInfo);
+
+        unsigned char* yuvPixels[] = {
+                static_cast<unsigned char*>(yuvaBmps[0].getPixels()),
+                static_cast<unsigned char*>(yuvaBmps[1].getPixels()),
+                static_cast<unsigned char*>(yuvaBmps[2].getPixels()),
+                static_cast<unsigned char*>(yuvaBmps[3].getPixels()),
+        };
 
         float m[20];
         SkColorMatrix_RGB2YUV(kJPEG_SkYUVColorSpace, m);
         // Here we encode using the kJPEG_SkYUVColorSpace (i.e., full-swing Rec 601) even though
         // we will draw it with all the supported yuv color spaces when converted back to RGB
-        for (int i = 0; i < kBmpSize * kBmpSize; ++i) {
-            auto r = (rgbColors[i] & 0x000000ff) >>  0;
-            auto g = (rgbColors[i] & 0x0000ff00) >>  8;
-            auto b = (rgbColors[i] & 0x00ff0000) >> 16;
-            auto a = (rgbColors[i] & 0xff000000) >> 24;
-            yPixels[i] = SkToU8(sk_float_round2int(m[0]*r + m[1]*g + m[2]*b + m[3]*a + 255*m[4]));
+        for (int j = 0; j < yaInfo.height(); ++j) {
+            for (int i = 0; i < yaInfo.width(); ++i) {
+                auto rgba = *rgbaBmp.getAddr32(i, j);
+                auto r = (rgba & 0x000000ff) >>  0;
+                auto g = (rgba & 0x0000ff00) >>  8;
+                auto b = (rgba & 0x00ff0000) >> 16;
+                auto a = (rgba & 0xff000000) >> 24;
+                yuvPixels[0][j*yaInfo.width() + i] = SkToU8(
+                        sk_float_round2int(m[0]*r + m[1]*g + m[2]*b + m[3]*a + 255*m[4]));
+                yuvPixels[3][j*yaInfo.width() + i] = SkToU8(sk_float_round2int(
+                        m[15]*r + m[16]*g + m[17]*b + m[18]*a + 255*m[19]));
+            }
         }
-        for (int j = 0; j < kBmpSize / 2; ++j) {
-            for (int i = 0; i < kBmpSize / 2; ++i) {
+        for (int j = 0; j < uvInfo.height(); ++j) {
+            for (int i = 0; i < uvInfo.width(); ++i) {
                 // Average together 4 pixels of RGB.
                 int rgba[] = {0, 0, 0, 0};
                 for (int y = 0; y < 2; ++y) {
                     for (int x = 0; x < 2; ++x) {
-                        int rgbIndex = (2 * j + y) * kBmpSize + 2 * i + x;
-                        rgba[0] += (rgbColors[rgbIndex] & 0x000000ff) >>  0;
-                        rgba[1] += (rgbColors[rgbIndex] & 0x0000ff00) >>  8;
-                        rgba[2] += (rgbColors[rgbIndex] & 0x00ff0000) >> 16;
-                        rgba[3] += (rgbColors[rgbIndex] & 0xff000000) >> 24;
+                        auto src = *rgbaBmp.getAddr32(2 * i + x, 2 * j + y);
+                        rgba[0] += (src & 0x000000ff) >> 0;
+                        rgba[1] += (src & 0x0000ff00) >> 8;
+                        rgba[2] += (src & 0x00ff0000) >> 16;
+                        rgba[3] += (src & 0xff000000) >> 24;
                     }
                 }
                 for (int c = 0; c < 4; ++c) {
                     rgba[c] /= 4;
                 }
-                int uvIndex = j * kBmpSize / 2 + i;
-                uvPixels[0][uvIndex] = SkToU8(sk_float_round2int(
+                int uvIndex = j*uvInfo.width() + i;
+                yuvPixels[1][uvIndex] = SkToU8(sk_float_round2int(
                         m[5]*rgba[0] + m[6]*rgba[1] + m[7]*rgba[2] + m[8]*rgba[3] + 255*m[9]));
-                uvPixels[1][uvIndex] = SkToU8(sk_float_round2int(
+                yuvPixels[2][uvIndex] = SkToU8(sk_float_round2int(
                         m[10]*rgba[0] + m[11]*rgba[1] + m[12]*rgba[2] + m[13]*rgba[3] + 255*m[14]));
             }
         }
-        fRGBImage = SkImage::MakeRasterCopy(SkPixmap(rgbBmp.info(), rgbColors, rgbBmp.rowBytes()));
+        return rgbaBmp;
     }
 
-    void createYUVTextures(GrContext* context, GrBackendTexture yuvTextures[3]) {
-        for (int i = 0; i < 3; ++i) {
-            SkASSERT(fYUVBmps[i].width() == SkToInt(fYUVBmps[i].rowBytes()));
-            yuvTextures[i] = context->createBackendTexture(&fYUVBmps[i].pixmap(), 1,
-                                                           GrRenderable::kNo, GrProtected::kNo);
+    void createYUVTextures(SkBitmap bmps[4], GrContext* context, GrBackendTexture textures[4]) {
+        for (int i = 0; i < 4; ++i) {
+            SkASSERT(bmps[i].width() == SkToInt(bmps[i].rowBytes()));
+            // To skirt issues of whether createBackendTexture() will return a single channel
+            // red or alpha backend format texture we use a RGBA pixmap and assume no swizzling
+            // happens in the upload here.
+            SkBitmap tmp;
+            tmp.allocPixels(bmps[i].info().makeColorType(kRGBA_8888_SkColorType));
+            // This draw writes the same value to all four channels of tmp.
+            SkCanvas canvas(tmp);
+            SkPaint paint;
+            paint.setColor(SK_ColorWHITE);
+            paint.setBlendMode(SkBlendMode::kSrc);
+            canvas.drawBitmap(bmps[i], 0, 0, &paint);
+            textures[i] = context->createBackendTexture(tmp.pixmap(), GrRenderable::kNo,
+                                                        GrProtected::kNo);
         }
     }
 
-    void createResultTexture(GrContext* context, int width, int height,
-                             GrBackendTexture* resultTexture) {
+    void createResultTexture(GrContext* context, SkISize size, GrBackendTexture* resultTexture) {
         *resultTexture = context->createBackendTexture(
-                width, height, kRGBA_8888_SkColorType, SkColors::kTransparent,
+                size.width(), size.height(), kRGBA_8888_SkColorType, SkColors::kTransparent,
                 GrMipMapped::kNo, GrRenderable::kYes, GrProtected::kNo);
     }
 
@@ -158,67 +160,103 @@
     }
 
     void onDraw(GrContext* context, GrRenderTargetContext*, SkCanvas* canvas) override {
-        // draw the original
-        SkScalar yOffset = kPad;
-        canvas->drawImage(fRGBImage.get(), kPad, yOffset);
-        yOffset += kBmpSize + kPad;
+        GrBackendTexture yuvaTextures[4];
+        // Because of the workaround in createYUVTextures() the actual channels don't matter here.
+        static constexpr SkYUVAIndex kIndices[] = {
+                {0, SkColorChannel::kR},
+                {1, SkColorChannel::kR},
+                {2, SkColorChannel::kR},
+                {3, SkColorChannel::kR},
+        };
+        this->createYUVTextures(fYUVABmps, context, yuvaTextures);
+        auto image1 = SkImage::MakeFromYUVATextures(context,
+                                                    kJPEG_SkYUVColorSpace,
+                                                    yuvaTextures,
+                                                    kIndices,
+                                                    fRGBABmp.dimensions(),
+                                                    kTopLeft_GrSurfaceOrigin);
 
-        for (int space = kJPEG_SkYUVColorSpace; space <= kLastEnum_SkYUVColorSpace; ++space) {
-            GrBackendTexture yuvTextures[3];
-            this->createYUVTextures(context, yuvTextures);
-            auto image = SkImage::MakeFromYUVTexturesCopy(context,
-                                                          static_cast<SkYUVColorSpace>(space),
-                                                          yuvTextures,
-                                                          kTopLeft_GrSurfaceOrigin);
-            this->deleteBackendTextures(context, yuvTextures, 3);
+        GrBackendTexture resultTexture;
+        this->createResultTexture(context, fRGBABmp.dimensions(), &resultTexture);
+        auto image2 = SkImage::MakeFromYUVTexturesCopyWithExternalBackend(context,
+                                                                          kJPEG_SkYUVColorSpace,
+                                                                          yuvaTextures,
+                                                                          kTopLeft_GrSurfaceOrigin,
+                                                                          resultTexture);
 
-            SkPaint paint;
-            if (kIdentity_SkYUVColorSpace == space) {
-                // The identity color space needs post-processing to appear correct
-                paint.setColorFilter(yuv_to_rgb_colorfilter());
+        auto draw_image = [canvas](SkImage* image, SkFilterQuality fq) -> SkSize {
+            if (!image) {
+                return {0, 0};
             }
+            SkPaint paint;
+            paint.setFilterQuality(fq);
+            canvas->drawImage(image, 0, 0, &paint);
+            return {SkIntToScalar(image->width()), SkIntToScalar(image->height())};
+        };
 
-            canvas->drawImage(image.get(), kPad, yOffset, &paint);
-            yOffset += kBmpSize + kPad;
+        auto draw_image_rect = [canvas](SkImage* image, SkFilterQuality fq) -> SkSize {
+            if (!image) {
+                return {0, 0};
+            }
+            SkPaint paint;
+            paint.setFilterQuality(fq);
+            auto subset = SkRect::Make(image->dimensions());
+            subset.inset(subset.width() * .05f, subset.height() * .1f);
+            auto dst = SkRect::MakeWH(subset.width(), subset.height());
+            canvas->drawImageRect(image, subset, dst, &paint);
+            return {dst.width(), dst.height()};
+        };
+
+        auto draw_image_shader = [canvas](SkImage* image, SkFilterQuality fq) -> SkSize {
+            if (!image) {
+                return {0, 0};
+            }
+            SkMatrix m;
+            m.setRotate(45, image->width()/2.f, image->height()/2.f);
+            auto shader = image->makeShader(SkTileMode::kMirror, SkTileMode::kDecal, m);
+            SkPaint paint;
+            paint.setFilterQuality(fq);
+            paint.setShader(std::move(shader));
+            auto rect = SkRect::MakeWH(image->width() * 1.3f, image->height());
+            canvas->drawRect(rect, paint);
+            return {rect.width(), rect.height()};
+        };
+
+        canvas->translate(kPad, kPad);
+        using DrawSig = SkSize(SkImage* image, SkFilterQuality fq);
+        using DF = std::function<DrawSig>;
+        for (const auto& draw : {DF(draw_image), DF(draw_image_rect), DF(draw_image_shader)}) {
+            for (auto scale : {1.f, 4.f, 0.75f}) {
+                SkScalar h = 0;
+                canvas->save();
+                for (auto fq : {kNone_SkFilterQuality, kLow_SkFilterQuality,
+                                kMedium_SkFilterQuality, kHigh_SkFilterQuality}) {
+                    canvas->save();
+                        canvas->scale(scale, scale);
+                        auto s1 = draw(image1.get(), fq);
+                    canvas->restore();
+                    canvas->translate(kPad + SkScalarCeilToScalar(scale*s1.width()), 0);
+                    canvas->save();
+                        canvas->scale(scale, scale);
+                        auto s2 = draw(image2.get(), fq);
+                    canvas->restore();
+                    canvas->translate(kPad + SkScalarCeilToScalar(scale*s2.width()), 0);
+                    h = std::max({h, s1.height(), s2.height()});
+                }
+                canvas->restore();
+                canvas->translate(0, kPad + SkScalarCeilToScalar(scale*h));
+            }
         }
 
-        for (int space = kJPEG_SkYUVColorSpace; space <= kLastEnum_SkYUVColorSpace; ++space) {
-            GrBackendTexture yuvTextures[3];
-            GrBackendTexture resultTexture;
-            this->createYUVTextures(context, yuvTextures);
-            this->createResultTexture(
-                    context, yuvTextures[0].width(), yuvTextures[0].height(), &resultTexture);
-            auto image = SkImage::MakeFromYUVTexturesCopyWithExternalBackend(
-                                                          context,
-                                                          static_cast<SkYUVColorSpace>(space),
-                                                          yuvTextures,
-                                                          kTopLeft_GrSurfaceOrigin,
-                                                          resultTexture);
-
-            SkPaint paint;
-            if (kIdentity_SkYUVColorSpace == space) {
-                // The identity color space needs post-processing to appear correct
-                paint.setColorFilter(yuv_to_rgb_colorfilter());
-            }
-            canvas->drawImage(image.get(), kPad, yOffset, &paint);
-            yOffset += kBmpSize + kPad;
-
-            GrBackendTexture texturesToDelete[4]{
-                    yuvTextures[0],
-                    yuvTextures[1],
-                    yuvTextures[2],
-                    resultTexture,
-            };
-            this->deleteBackendTextures(context, texturesToDelete, 4);
-        }
+        this->deleteBackendTextures(context, &resultTexture, 1);
+        this->deleteBackendTextures(context, yuvaTextures, 4);
      }
 
 private:
-    sk_sp<SkImage>  fRGBImage;
-    SkBitmap        fYUVBmps[3];
+    SkBitmap fRGBABmp;
+    SkBitmap fYUVABmps[4];
 
     static constexpr SkScalar kPad = 10.0f;
-    static constexpr int kBmpSize  = 32;
 
     typedef GM INHERITED;
 };
diff --git a/src/gpu/GrImageTextureMaker.cpp b/src/gpu/GrImageTextureMaker.cpp
index 5a4f7e7..71c2d26 100644
--- a/src/gpu/GrImageTextureMaker.cpp
+++ b/src/gpu/GrImageTextureMaker.cpp
@@ -11,6 +11,7 @@
 #include "src/gpu/GrContextPriv.h"
 #include "src/gpu/GrRecordingContextPriv.h"
 #include "src/gpu/SkGr.h"
+#include "src/gpu/effects/GrBicubicEffect.h"
 #include "src/gpu/effects/GrYUVtoRGBEffect.h"
 #include "src/image/SkImage_GpuYUVA.h"
 #include "src/image/SkImage_Lazy.h"
@@ -65,17 +66,18 @@
         GrSamplerState::WrapMode wrapX,
         GrSamplerState::WrapMode wrapY,
         const GrSamplerState::Filter* filterOrNullForBicubic) {
-    // Check simple cases to see if we need to fall back to flattening the image (or whether it's
-    // already been flattened.)
-    if (!filterOrNullForBicubic || fImage->fRGBView.proxy()) {
+    // Check whether it's already been flattened.
+    if (fImage->fRGBView.proxy()) {
         return this->INHERITED::createFragmentProcessor(
                 textureMatrix, constraintRect, filterConstraint, coordsLimitedToConstraintRect,
                 wrapX, wrapY, filterOrNullForBicubic);
     }
 
+    GrSamplerState::Filter filter =
+            filterOrNullForBicubic ? *filterOrNullForBicubic : GrSamplerState::Filter::kNearest;
+
     // Check to see if the client has given us pre-mipped textures or we can generate them
     // If not, fall back to bilerp. Also fall back to bilerp when a domain is requested
-    GrSamplerState::Filter filter = *filterOrNullForBicubic;
     if (GrSamplerState::Filter::kMipMap == filter &&
         (filterConstraint == GrTextureProducer::kYes_FilterConstraint ||
          !fImage->setupMipmapsForPlanes(this->context()))) {
@@ -91,8 +93,13 @@
     }
 
     const auto& caps = *fImage->context()->priv().caps();
+    const SkMatrix& m = filterOrNullForBicubic ? textureMatrix : SkMatrix::I();
     auto fp = GrYUVtoRGBEffect::Make(fImage->fViews, fImage->fYUVAIndices, fImage->fYUVColorSpace,
-                                     filter, caps, textureMatrix, domain);
+                                     filter, caps, m, domain);
+    if (!filterOrNullForBicubic) {
+        fp = GrBicubicEffect::Make(std::move(fp), fImage->alphaType(), textureMatrix,
+                                   GrBicubicEffect::Direction::kXY);
+    }
     if (fImage->fFromColorSpace) {
         fp = GrColorSpaceXformEffect::Make(std::move(fp), fImage->fFromColorSpace.get(),
                                            fImage->alphaType(), fImage->colorSpace());
diff --git a/src/gpu/effects/GrBicubicEffect.cpp b/src/gpu/effects/GrBicubicEffect.cpp
index d7f3933..db4663f 100644
--- a/src/gpu/effects/GrBicubicEffect.cpp
+++ b/src/gpu/effects/GrBicubicEffect.cpp
@@ -19,22 +19,12 @@
     void emitCode(EmitArgs&) override;
 
 private:
-    void onSetData(const GrGLSLProgramDataManager&, const GrFragmentProcessor&) override;
-
-    UniformHandle fDimensions;
-    GrTextureDomain::GLDomain   fDomain;
-
     typedef GrGLSLFragmentProcessor INHERITED;
 };
 
 void GrBicubicEffect::Impl::emitCode(EmitArgs& args) {
     const GrBicubicEffect& bicubicEffect = args.fFp.cast<GrBicubicEffect>();
 
-    GrGLSLUniformHandler* uniformHandler = args.fUniformHandler;
-    fDimensions = uniformHandler->addUniform(kFragment_GrShaderFlag, kHalf4_GrSLType, "Dimensions");
-
-    const char* dims = uniformHandler->getUniformCStr(fDimensions);
-
     GrGLSLFPFragmentBuilder* fragBuilder = args.fFragBuilder;
     SkString coords2D = fragBuilder->ensureCoords2D(args.fTransformedCoords[0].fVaryingPoint);
 
@@ -61,14 +51,15 @@
                             "-9.0 / 18.0,   0.0 / 18.0,   9.0 / 18.0,  0.0 / 18.0,"
                             "15.0 / 18.0, -36.0 / 18.0,  27.0 / 18.0, -6.0 / 18.0,"
                             "-7.0 / 18.0,  21.0 / 18.0, -21.0 / 18.0,  7.0 / 18.0);");
-    fragBuilder->codeAppendf("float2 coord = %s - %s.xy * float2(0.5);", coords2D.c_str(), dims);
-    // We unnormalize the coord in order to determine our fractional offset (f) within the texel
-    // We then snap coord to a texel center and renormalize. The snap prevents cases where the
-    // starting coords are near a texel boundary and accumulations of dims would cause us to skip/
-    // double hit a texel.
-    fragBuilder->codeAppendf("half2 f = half2(fract(coord * %s.zw));", dims);
-    fragBuilder->codeAppendf("coord = coord + (half2(0.5) - f) * %s.xy;", dims);
+    // We determine our fractional offset (f) within the texel. We then snap coord to a texel
+    // center. The snap prevents cases where the starting coords are near a texel boundary and
+    // offsets with imperfect precision would cause us to skip/double hit a texel.
+    // The use of "texel" above is somewhat abstract as we're sampling a child processor. It is
+    // assumed the child processor represents something akin to a nearest neighbor sampled texture.
     if (bicubicEffect.fDirection == GrBicubicEffect::Direction::kXY) {
+        fragBuilder->codeAppendf("float2 coord = %s - float2(0.5);", coords2D.c_str());
+        fragBuilder->codeAppend("half2 f = half2(fract(coord));");
+        fragBuilder->codeAppend("coord += 0.5 - f;");
         fragBuilder->codeAppend(
                 "half4 wx = kMitchellCoefficients * half4(1.0, f.x, f.x * f.x, f.x * f.x * f.x);");
         fragBuilder->codeAppend(
@@ -77,7 +68,7 @@
         for (int y = 0; y < 4; ++y) {
             for (int x = 0; x < 4; ++x) {
                 SkString coord;
-                coord.printf("coord + %s.xy * float2(%d, %d)", dims, x - 1, y - 1);
+                coord.printf("coord + float2(%d, %d)", x - 1, y - 1);
                 auto childStr =
                         this->invokeChild(0, args, SkSL::String(coord.c_str(), coord.size()));
                 fragBuilder->codeAppendf("rowColors[%d] = %s;", x, childStr.c_str());
@@ -90,14 +81,20 @@
         fragBuilder->codeAppend(
                 "half4 bicubicColor = wy.x * s0 + wy.y * s1 + wy.z * s2 + wy.w * s3;");
     } else {
-        // One of the dims.xy values will be zero. So v here selects the nonzero value of f.
-        fragBuilder->codeAppend("half v = f.x + f.y;");
-        fragBuilder->codeAppend("half v2 = v * v;");
-        fragBuilder->codeAppend("half4 w = kMitchellCoefficients * half4(1.0, v, v2, v2 * v);");
+        const char* d = bicubicEffect.fDirection == Direction::kX ? "x" : "y";
+        fragBuilder->codeAppendf("float coord = %s.%s - 0.5;", coords2D.c_str(), d);
+        fragBuilder->codeAppend("half f = half(fract(coord));");
+        fragBuilder->codeAppend("coord += 0.5 - f;");
+        fragBuilder->codeAppend("half f2 = f * f;");
+        fragBuilder->codeAppend("half4 w = kMitchellCoefficients * half4(1.0, f, f2, f2 * f);");
         fragBuilder->codeAppend("half4 c[4];");
         for (int i = 0; i < 4; ++i) {
             SkString coord;
-            coord.printf("coord + %s.xy * half(%d)", dims, i - 1);
+            if (bicubicEffect.fDirection == Direction::kX) {
+                coord.printf("float2(coord + %d, %s.y)", i - 1, coords2D.c_str());
+            } else {
+                coord.printf("float2(%s.x, coord + %d)", coords2D.c_str(), i - 1);
+            }
             auto childStr = this->invokeChild(0, args, SkSL::String(coord.c_str(), coord.size()));
             fragBuilder->codeAppendf("c[%d] = %s;", i, childStr.c_str());
         }
@@ -118,42 +115,14 @@
     fragBuilder->codeAppendf("%s = bicubicColor * %s;", args.fOutputColor, args.fInputColor);
 }
 
-void GrBicubicEffect::Impl::onSetData(const GrGLSLProgramDataManager& pdman,
-                                      const GrFragmentProcessor& processor) {
-    const GrBicubicEffect& bicubicEffect = processor.cast<GrBicubicEffect>();
-    // Currently we only ever construct with GrTextureEffect and always take its
-    // coord transform as our own.
-    SkASSERT(bicubicEffect.fCoordTransform.peekTexture());
-    SkISize textureDims = bicubicEffect.fCoordTransform.peekTexture()->dimensions();
-
-    float dims[4] = {0, 0, 0, 0};
-    if (bicubicEffect.fDirection != GrBicubicEffect::Direction::kY) {
-        if (bicubicEffect.fCoordTransform.normalize()) {
-            dims[0] = 1.f / textureDims.width();
-            dims[2] = textureDims.width();
-        } else {
-            dims[0] = dims[2] = 1.f;
-        }
-    }
-    if (bicubicEffect.fDirection != GrBicubicEffect::Direction::kX) {
-        if (bicubicEffect.fCoordTransform.normalize()) {
-            dims[1] = 1.f / textureDims.height();
-            dims[3] = textureDims.height();
-        } else {
-            dims[1] = dims[3] = 1.f;
-        }
-    }
-    pdman.set4fv(fDimensions, 1, dims);
-}
-
 std::unique_ptr<GrFragmentProcessor> GrBicubicEffect::Make(GrSurfaceProxyView view,
                                                            SkAlphaType alphaType,
                                                            const SkMatrix& matrix,
                                                            Direction direction) {
-    auto fp = GrTextureEffect::Make(std::move(view), alphaType, matrix);
+    auto fp = GrTextureEffect::Make(std::move(view), alphaType, SkMatrix::I());
     auto clamp = kPremul_SkAlphaType == alphaType ? Clamp::kPremul : Clamp::kUnpremul;
     return std::unique_ptr<GrFragmentProcessor>(
-            new GrBicubicEffect(std::move(fp), direction, clamp));
+            new GrBicubicEffect(std::move(fp), matrix, direction, clamp));
 }
 
 std::unique_ptr<GrFragmentProcessor> GrBicubicEffect::Make(GrSurfaceProxyView view,
@@ -165,10 +134,10 @@
                                                            const GrCaps& caps) {
     GrSamplerState sampler(wrapX, wrapY, GrSamplerState::Filter::kNearest);
     std::unique_ptr<GrFragmentProcessor> fp;
-    fp = GrTextureEffect::Make(std::move(view), alphaType, matrix, sampler, caps);
+    fp = GrTextureEffect::Make(std::move(view), alphaType, SkMatrix::I(), sampler, caps);
     auto clamp = kPremul_SkAlphaType == alphaType ? Clamp::kPremul : Clamp::kUnpremul;
     return std::unique_ptr<GrFragmentProcessor>(
-            new GrBicubicEffect(std::move(fp), direction, clamp));
+            new GrBicubicEffect(std::move(fp), matrix, direction, clamp));
 }
 
 std::unique_ptr<GrFragmentProcessor> GrBicubicEffect::MakeSubset(
@@ -182,23 +151,32 @@
         const GrCaps& caps) {
     GrSamplerState sampler(wrapX, wrapY, GrSamplerState::Filter::kNearest);
     std::unique_ptr<GrFragmentProcessor> fp;
-    fp = GrTextureEffect::MakeSubset(std::move(view), alphaType, matrix, sampler, subset, caps);
+    fp = GrTextureEffect::MakeSubset(
+            std::move(view), alphaType, SkMatrix::I(), sampler, subset, caps);
     auto clamp = kPremul_SkAlphaType == alphaType ? Clamp::kPremul : Clamp::kUnpremul;
     return std::unique_ptr<GrFragmentProcessor>(
-            new GrBicubicEffect(std::move(fp), direction, clamp));
+            new GrBicubicEffect(std::move(fp), matrix, direction, clamp));
+}
+
+std::unique_ptr<GrFragmentProcessor> GrBicubicEffect::Make(std::unique_ptr<GrFragmentProcessor> fp,
+                                                           SkAlphaType alphaType,
+                                                           const SkMatrix& matrix,
+                                                           Direction direction) {
+    auto clamp = kPremul_SkAlphaType == alphaType ? Clamp::kPremul : Clamp::kUnpremul;
+    return std::unique_ptr<GrFragmentProcessor>(
+            new GrBicubicEffect(std::move(fp), matrix, direction, clamp));
 }
 
 GrBicubicEffect::GrBicubicEffect(std::unique_ptr<GrFragmentProcessor> fp,
+                                 const SkMatrix& matrix,
                                  Direction direction,
                                  Clamp clamp)
         : INHERITED(kGrBicubicEffect_ClassID, ProcessorOptimizationFlags(fp.get()))
+        , fCoordTransform(matrix)
         , fDirection(direction)
         , fClamp(clamp) {
-    SkASSERT(fp->numCoordTransforms() == 1);
-    fCoordTransform = fp->coordTransform(0);
-    this->addCoordTransform(&fCoordTransform);
-    fp->coordTransform(0) = {};
     fp->setSampledWithExplicitCoords(true);
+    this->addCoordTransform(&fCoordTransform);
     this->registerChildProcessor(std::move(fp));
 }
 
@@ -215,8 +193,7 @@
 
 void GrBicubicEffect::onGetGLSLProcessorKey(const GrShaderCaps& caps,
                                             GrProcessorKeyBuilder* b) const {
-    uint32_t key = (fDirection == GrBicubicEffect::Direction::kXY)
-                 | (static_cast<uint32_t>(fClamp) << 1);
+    uint32_t key = static_cast<uint32_t>(fDirection) | (static_cast<uint32_t>(fClamp) << 2);
     b->add32(key);
 }
 
@@ -227,6 +204,10 @@
     return fDirection == that.fDirection && fClamp == that.fClamp;
 }
 
+SkPMColor4f GrBicubicEffect::constantOutputForConstantInput(const SkPMColor4f& input) const {
+    return GrFragmentProcessor::ConstantOutputForConstantInput(this->childProcessor(0), input);
+}
+
 GR_DEFINE_FRAGMENT_PROCESSOR_TEST(GrBicubicEffect);
 
 #if GR_TEST_UTILS
@@ -243,24 +224,42 @@
             direction = Direction::kXY;
             break;
     }
-    auto [view, ct, at] = d->randomView();
     auto m = GrTest::TestMatrix(d->fRandom);
-    if (d->fRandom->nextBool()) {
-        GrSamplerState::WrapMode wm[2];
-        GrTest::TestWrapModes(d->fRandom, wm);
+    switch (d->fRandom->nextULessThan(3)) {
+        case 0: {
+            auto [view, ct, at] = d->randomView();
+            GrSamplerState::WrapMode wm[2];
+            GrTest::TestWrapModes(d->fRandom, wm);
 
-        if (d->fRandom->nextBool()) {
-            SkRect subset;
-            subset.fLeft   = d->fRandom->nextSScalar1() * view.width();
-            subset.fTop    = d->fRandom->nextSScalar1() * view.height();
-            subset.fRight  = d->fRandom->nextSScalar1() * view.width();
-            subset.fBottom = d->fRandom->nextSScalar1() * view.height();
-            subset.sort();
-            return MakeSubset(std::move(view), at, m, wm[0], wm[1], subset, direction, *d->caps());
+            if (d->fRandom->nextBool()) {
+                SkRect subset;
+                subset.fLeft = d->fRandom->nextSScalar1() * view.width();
+                subset.fTop = d->fRandom->nextSScalar1() * view.height();
+                subset.fRight = d->fRandom->nextSScalar1() * view.width();
+                subset.fBottom = d->fRandom->nextSScalar1() * view.height();
+                subset.sort();
+                return MakeSubset(
+                        std::move(view), at, m, wm[0], wm[1], subset, direction, *d->caps());
+            }
+            return Make(std::move(view), at, m, wm[0], wm[1], direction, *d->caps());
         }
-        return Make(std::move(view), at, m, wm[0], wm[1], direction, *d->caps());
-    } else {
-        return Make(std::move(view), at, m, direction);
+        case 1: {
+            auto [view, ct, at] = d->randomView();
+            return Make(std::move(view), at, m, direction);
+        }
+        default: {
+            SkAlphaType at;
+            do {
+                at = static_cast<SkAlphaType>(d->fRandom->nextULessThan(kLastEnum_SkAlphaType + 1));
+            } while (at != kUnknown_SkAlphaType);
+            std::unique_ptr<GrFragmentProcessor> fp;
+            // We have a restriction that explicit coords only work for FPs with zero or one
+            // coord transform.
+            do {
+                fp = GrProcessorUnitTest::MakeChildFP(d);
+            } while (fp->numCoordTransforms() > 1);
+            return Make(std::move(fp), at, m, direction);
+        }
     }
 }
 #endif
diff --git a/src/gpu/effects/GrBicubicEffect.h b/src/gpu/effects/GrBicubicEffect.h
index 1ad5688..51b43f6 100644
--- a/src/gpu/effects/GrBicubicEffect.h
+++ b/src/gpu/effects/GrBicubicEffect.h
@@ -66,6 +66,16 @@
                                                            const SkRect& subset,
                                                            Direction,
                                                            const GrCaps&);
+
+    /**
+     * Make a Mitchell filter of a another fragment processor. The bicubic filter assumes that the
+     * discrete samples of the provided processor are at half-integer coords.
+     */
+    static std::unique_ptr<GrFragmentProcessor> Make(std::unique_ptr<GrFragmentProcessor>,
+                                                     SkAlphaType,
+                                                     const SkMatrix&,
+                                                     Direction);
+
     /**
      * Determines whether the bicubic effect should be used based on the transformation from the
      * local coords to the device. Returns true if the bicubic effect should be used. filterMode
@@ -84,7 +94,8 @@
         kPremul,    // clamps a to 0..1 and rgb to 0..a
     };
 
-    GrBicubicEffect(std::unique_ptr<GrFragmentProcessor> fp, Direction direction, Clamp clamp);
+    GrBicubicEffect(std::unique_ptr<GrFragmentProcessor>, const SkMatrix&, Direction, Clamp);
+
     explicit GrBicubicEffect(const GrBicubicEffect&);
 
     GrGLSLFragmentProcessor* onCreateGLSLInstance() const override;
@@ -93,6 +104,8 @@
 
     bool onIsEqual(const GrFragmentProcessor&) const override;
 
+    SkPMColor4f constantOutputForConstantInput(const SkPMColor4f&) const override;
+
     GrCoordTransform fCoordTransform;
     Direction fDirection;
     Clamp fClamp;
diff --git a/src/gpu/effects/GrDeviceSpaceEffect.fp b/src/gpu/effects/GrDeviceSpaceEffect.fp
index d817ec2..879c7c4 100644
--- a/src/gpu/effects/GrDeviceSpaceEffect.fp
+++ b/src/gpu/effects/GrDeviceSpaceEffect.fp
@@ -13,10 +13,10 @@
 
 @test(d) {
     std::unique_ptr<GrFragmentProcessor> fp;
-    // We have a restriction that explicit coords only work for FPs with exactly one
+    // We have a restriction that explicit coords only work for FPs with zero or one
     // coord transform.
     do {
         fp = GrProcessorUnitTest::MakeChildFP(d);
-    } while (fp->numCoordTransforms() != 1);
+    } while (fp->numCoordTransforms() > 1);
     return GrDeviceSpaceEffect::Make(std::move(fp));
 }
diff --git a/src/gpu/effects/generated/GrDeviceSpaceEffect.cpp b/src/gpu/effects/generated/GrDeviceSpaceEffect.cpp
index daf7eb9..fbab6e1 100644
--- a/src/gpu/effects/generated/GrDeviceSpaceEffect.cpp
+++ b/src/gpu/effects/generated/GrDeviceSpaceEffect.cpp
@@ -61,11 +61,11 @@
 #if GR_TEST_UTILS
 std::unique_ptr<GrFragmentProcessor> GrDeviceSpaceEffect::TestCreate(GrProcessorTestData* d) {
     std::unique_ptr<GrFragmentProcessor> fp;
-    // We have a restriction that explicit coords only work for FPs with exactly one
+    // We have a restriction that explicit coords only work for FPs with zero or one
     // coord transform.
     do {
         fp = GrProcessorUnitTest::MakeChildFP(d);
-    } while (fp->numCoordTransforms() != 1);
+    } while (fp->numCoordTransforms() > 1);
     return GrDeviceSpaceEffect::Make(std::move(fp));
 }
 #endif