Revert "alternate approach to unpremul scalePixels()"
This reverts commit c4616804bb407506c6ac1046c7e25e2016911449.
Reason for revert: screwed up the guard I think
Original change's description:
> alternate approach to unpremul scalePixels()
>
> We want to keep the clamps in SkImageShader, and keep unpremul
> scalePixels() happy too.
>
> So we extend SkImageShader's internal API to allow specifying an output
> alpha type, which controls whether we premul or unpremul and how we
> clamp. scalePixels() uses this to draw instead of a drawBitmap() call.
>
> Sort of backwards of usual, we opt our local builds into
> SK_LEGACY_HIGH_QUALITY_SCALING_CLAMP (and Google3 is already defining
> this). Then to rebase Chromium we will _define_ this in Chromium's user
> config, fold it through as if always defined in Skia, then finally
> remove the definition from Chromium's user config.
>
> Change-Id: I38035f0886f79700e7301c3c6042ce362c771d96
> Reviewed-on: https://skia-review.googlesource.com/90480
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Mike Klein <mtklein@chromium.org>
TBR=mtklein@chromium.org,mtklein@google.com,brianosman@google.com,reed@google.com
Change-Id: Ib53a7f29c25310b667f9a61f67f8638403ec9da3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/106220
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Mike Klein <mtklein@chromium.org>
diff --git a/BUILD.gn b/BUILD.gn
index 649c149..a43f95b 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -144,9 +144,6 @@
if (skia_enable_atlas_text) {
defines += [ "SK_SUPPORT_ATLAS_TEXT=1" ]
}
-
- # Temporary.
- defines += [ "SK_LEGACY_HIGH_QUALITY_SCALING_CLAMP" ]
}
# Skia internal APIs, used by Skia itself and a few test tools.
diff --git a/src/core/SkPixmap.cpp b/src/core/SkPixmap.cpp
index 050c6e2..42e2c66 100644
--- a/src/core/SkPixmap.cpp
+++ b/src/core/SkPixmap.cpp
@@ -11,7 +11,6 @@
#include "SkConvertPixels.h"
#include "SkData.h"
#include "SkImageInfoPriv.h"
-#include "SkImageShader.h"
#include "SkHalf.h"
#include "SkMask.h"
#include "SkNx.h"
@@ -230,6 +229,10 @@
return true;
}
+static void set_alphatype(SkPixmap* dst, const SkPixmap& src, SkAlphaType at) {
+ dst->reset(src.info().makeAlphaType(at), src.addr(), src.rowBytes());
+}
+
bool SkPixmap::scalePixels(const SkPixmap& dst, SkFilterQuality quality) const {
// Can't do anthing with empty src or dst
if (this->width() <= 0 || this->height() <= 0 || dst.width() <= 0 || dst.height() <= 0) {
@@ -241,51 +244,39 @@
return this->readPixels(dst);
}
+ // Temp storage in case we need to edit the requested alphatypes
+ SkPixmap storage_src, storage_dst;
+ const SkPixmap* srcPtr = this;
+ const SkPixmap* dstPtr = &dst;
+
+ // Trick: if src and dst are both unpremul, we can give the correct result if we change both
+ // to premul (or opaque), since the draw will not try to blend or otherwise interpret
+ // the pixels' alpha.
+ if (srcPtr->alphaType() == kUnpremul_SkAlphaType &&
+ dstPtr->alphaType() == kUnpremul_SkAlphaType)
+ {
+ set_alphatype(&storage_src, *this, kPremul_SkAlphaType);
+ set_alphatype(&storage_dst, dst, kPremul_SkAlphaType);
+ srcPtr = &storage_src;
+ dstPtr = &storage_dst;
+ }
+
SkBitmap bitmap;
- if (!bitmap.installPixels(*this)) {
+ if (!bitmap.installPixels(*srcPtr)) {
return false;
}
- bitmap.setImmutable(); // Don't copy when we create an image.
- bitmap.setIsVolatile(true); // Disable any caching.
+ bitmap.setIsVolatile(true); // so we don't try to cache it
- // We're going to set this up a little oddly so that we can scale unpremul SkPixmaps
- // to unpremul dsts without ever premultiplying (and potentially throwing away information).
-
- // 1) Here's the scale matrix between our input pixels and the scaled destination.
- SkMatrix matrix = SkMatrix::MakeRectToRect(SkRect::Make(this->bounds()),
- SkRect::Make(dst.bounds()),
- SkMatrix::kFill_ScaleToFit);
-
-
- // 2) Instead of just calling drawBitmap(), we'll create a shader that carefully
- // keeps its output in the alpha type we request, dst.alphaType().
- sk_sp<SkShader> shader = SkImageShader::Make(SkImage::MakeFromBitmap(bitmap),
- SkShader::kClamp_TileMode,
- SkShader::kClamp_TileMode,
- &matrix,
- dst.alphaType());
-
- // 3) No matter what dst's actual alpha type is, we'll construct this surface
- // as if it were not unpremul. This keeps the rest of the drawing pipeline oblivious
- // to our trickery here, and prevents it from doing anything like a manual unpremul.
- SkImageInfo info = dst.info();
- if (info.alphaType() == kUnpremul_SkAlphaType) {
- info = info.makeAlphaType(kPremul_SkAlphaType);
- }
- sk_sp<SkSurface> surface =
- SkSurface::MakeRasterDirect(info, dst.writable_addr(), dst.rowBytes());
- if (!shader || !surface) {
+ auto surface(SkSurface::MakeRasterDirect(dstPtr->info(), dstPtr->writable_addr(), dstPtr->rowBytes()));
+ if (!surface) {
return false;
}
- // 4) Using SkBlendMode::kSrc means we won't be tempted to try any premul-only blending math.
SkPaint paint;
- paint.setBlendMode(SkBlendMode::kSrc);
paint.setFilterQuality(quality);
- paint.setShader(std::move(shader));
-
- // 5) Draw it!
- surface->getCanvas()->drawPaint(paint);
+ paint.setBlendMode(SkBlendMode::kSrc);
+ surface->getCanvas()->drawBitmapRect(bitmap, SkRect::MakeIWH(dst.width(), dst.height()),
+ &paint);
return true;
}
diff --git a/src/shaders/SkImageShader.cpp b/src/shaders/SkImageShader.cpp
index 3e58782..1f77158 100644
--- a/src/shaders/SkImageShader.cpp
+++ b/src/shaders/SkImageShader.cpp
@@ -31,30 +31,23 @@
#endif
}
-SkImageShader::SkImageShader(sk_sp<SkImage> img,
- TileMode tmx, TileMode tmy,
- const SkMatrix* localMatrix,
- SkAlphaType outputAlphaType)
- : INHERITED(localMatrix)
+SkImageShader::SkImageShader(sk_sp<SkImage> img, TileMode tmx, TileMode tmy, const SkMatrix* matrix)
+ : INHERITED(matrix)
, fImage(std::move(img))
, fTileModeX(optimize(tmx, fImage->width()))
, fTileModeY(optimize(tmy, fImage->height()))
- , fOutputAlphaType(outputAlphaType)
{}
-// fOutputAlphaType is always kPremul_SkAlphaType when an SkImageShader is constructed
-// through public APIs that might lead to serialization, so we don't read or write it.
-
sk_sp<SkFlattenable> SkImageShader::CreateProc(SkReadBuffer& buffer) {
const TileMode tx = (TileMode)buffer.readUInt();
const TileMode ty = (TileMode)buffer.readUInt();
- SkMatrix localMatrix;
- buffer.readMatrix(&localMatrix);
+ SkMatrix matrix;
+ buffer.readMatrix(&matrix);
sk_sp<SkImage> img = buffer.readImage();
if (!img) {
return nullptr;
}
- return SkImageShader::Make(std::move(img), tx, ty, &localMatrix);
+ return SkImageShader::Make(std::move(img), tx, ty, &matrix);
}
void SkImageShader::flatten(SkWriteBuffer& buffer) const {
@@ -62,7 +55,6 @@
buffer.writeUInt(fTileModeY);
buffer.writeMatrix(this->getLocalMatrix());
buffer.writeImage(fImage.get());
- SkASSERT(fOutputAlphaType == kPremul_SkAlphaType);
}
bool SkImageShader::isOpaque() const {
@@ -172,14 +164,13 @@
return w > kMaxSize || h > kMaxSize;
}
-sk_sp<SkShader> SkImageShader::Make(sk_sp<SkImage> image,
- TileMode tx, TileMode ty,
- const SkMatrix* localMatrix,
- SkAlphaType outputAlphaType) {
+sk_sp<SkShader> SkImageShader::Make(sk_sp<SkImage> image, TileMode tx, TileMode ty,
+ const SkMatrix* localMatrix) {
if (!image || bitmap_is_too_big(image->width(), image->height())) {
return sk_make_sp<SkEmptyShader>();
+ } else {
+ return sk_make_sp<SkImageShader>(image, tx, ty, localMatrix);
}
- return sk_sp<SkShader>{ new SkImageShader(image, tx,ty, localMatrix, outputAlphaType) };
}
#ifndef SK_IGNORE_TO_STRING
@@ -394,24 +385,19 @@
auto append_misc = [&] {
if (info.colorType() == kAlpha_8_SkColorType) {
p->append(SkRasterPipeline::set_rgb, &misc->paint_color);
- p->append(SkRasterPipeline::premul);
- } else if (info.alphaType() == kUnpremul_SkAlphaType &&
- fOutputAlphaType == kPremul_SkAlphaType) {
- p->append(SkRasterPipeline::premul);
- } else if (info.alphaType() == kPremul_SkAlphaType &&
- fOutputAlphaType == kUnpremul_SkAlphaType) {
- p->append(SkRasterPipeline::unpremul);
}
-
- #if defined(SK_LEGACY_HIGH_QUALITY_SCALING_CLAMP)
+ if (info.colorType() == kAlpha_8_SkColorType ||
+ info.alphaType() == kUnpremul_SkAlphaType) {
+ p->append(SkRasterPipeline::premul);
+ }
+#if defined(SK_LEGACY_HIGH_QUALITY_SCALING_CLAMP)
if (quality > kLow_SkFilterQuality) {
// Bicubic filtering naturally produces out of range values on both sides.
p->append(SkRasterPipeline::clamp_0);
- p->append(fOutputAlphaType == kPremul_SkAlphaType ? SkRasterPipeline::clamp_a
- : SkRasterPipeline::clamp_1);
+ p->append(SkRasterPipeline::clamp_a);
}
- #endif
- append_gamut_transform(p, alloc, info.colorSpace(), rec.fDstCS, fOutputAlphaType);
+#endif
+ append_gamut_transform(p, alloc, info.colorSpace(), rec.fDstCS, kPremul_SkAlphaType);
return true;
};
diff --git a/src/shaders/SkImageShader.h b/src/shaders/SkImageShader.h
index 9ce6155..a3cf3db 100644
--- a/src/shaders/SkImageShader.h
+++ b/src/shaders/SkImageShader.h
@@ -15,11 +15,8 @@
class SkImageShader : public SkShaderBase {
public:
- static sk_sp<SkShader> Make(sk_sp<SkImage>,
- SkShader::TileMode tx,
- SkShader::TileMode ty,
- const SkMatrix* localMatrix,
- SkAlphaType outputAlphaType = kPremul_SkAlphaType);
+ static sk_sp<SkShader> Make(sk_sp<SkImage>, TileMode tx, TileMode ty,
+ const SkMatrix* localMatrix);
bool isOpaque() const override;
@@ -30,23 +27,19 @@
std::unique_ptr<GrFragmentProcessor> asFragmentProcessor(const GrFPArgs&) const override;
#endif
+ SkImageShader(sk_sp<SkImage>, TileMode tx, TileMode ty, const SkMatrix* localMatrix);
+
static bool IsRasterPipelineOnly(const SkMatrix& ctm, SkColorType, SkAlphaType,
SkShader::TileMode tx, SkShader::TileMode ty,
const SkMatrix& localM);
-private:
- SkImageShader(sk_sp<SkImage>,
- SkShader::TileMode tx,
- SkShader::TileMode ty,
- const SkMatrix* localMatrix,
- SkAlphaType outputAlphaType);
-
+protected:
void flatten(SkWriteBuffer&) const override;
Context* onMakeContext(const ContextRec&, SkArenaAlloc* storage) const override;
#ifdef SK_SUPPORT_LEGACY_SHADER_ISABITMAP
- bool onIsABitmap(SkBitmap*, SkMatrix*, SkShader::TileMode*) const override;
+ bool onIsABitmap(SkBitmap*, SkMatrix*, TileMode*) const override;
#endif
- SkImage* onIsAImage(SkMatrix*, SkShader::TileMode*) const override;
+ SkImage* onIsAImage(SkMatrix*, TileMode*) const override;
bool onIsRasterPipelineOnly(const SkMatrix& ctm) const override;
@@ -57,12 +50,13 @@
&this->getLocalMatrix());
}
- sk_sp<SkImage> fImage;
- const SkShader::TileMode fTileModeX;
- const SkShader::TileMode fTileModeY;
- const SkAlphaType fOutputAlphaType;
+ sk_sp<SkImage> fImage;
+ const TileMode fTileModeX;
+ const TileMode fTileModeY;
+private:
friend class SkShaderBase;
+
typedef SkShaderBase INHERITED;
};