aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar brianosman <brianosman@google.com>2016-09-23 08:11:55 -0700
committerGravatar Commit bot <commit-bot@chromium.org>2016-09-23 08:11:55 -0700
commit53c38087949252d27cde668368a3eeb59cc2eb00 (patch)
tree9d005cd6e6037f4b64d1140786e4e687f062f063
parent27a3aa5921b2aaf95314f955285c3cba91e4c19a (diff)
Change SkSpecialImage::makeSurface and makeTightSurface to take output
properties (color space), bounds, and (optional) alphaType. We were being pretty inconsistent before. Raster was honoring all components of the info. GPU was using the supplied color type, but propagating the source's color space. All call sites were saying N32. What we want to do is propagate the original device's color space, and pick a good format from that. Rather than force all the clients to jump through hoops constructing an SkImageInfo that meets our criteria, just have them supply the few bits we care about, and do everything else internally. This also lets us always use RGBA on GPU, but N32 on raster. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2349373004 Review-Url: https://codereview.chromium.org/2349373004
-rw-r--r--src/core/SkImageFilter.cpp6
-rw-r--r--src/core/SkMatrixImageFilter.cpp4
-rw-r--r--src/core/SkSpecialImage.cpp65
-rw-r--r--src/core/SkSpecialImage.h11
-rw-r--r--src/effects/SkColorFilterImageFilter.cpp3
-rw-r--r--src/effects/SkDisplacementMapEffect.cpp13
-rw-r--r--src/effects/SkDropShadowImageFilter.cpp4
-rw-r--r--src/effects/SkImageSource.cpp6
-rwxr-xr-xsrc/effects/SkMergeImageFilter.cpp5
-rw-r--r--src/effects/SkOffsetImageFilter.cpp4
-rw-r--r--src/effects/SkPaintImageFilter.cpp5
-rw-r--r--src/effects/SkPictureImageFilter.cpp9
-rw-r--r--src/effects/SkTileImageFilter.cpp12
-rw-r--r--src/effects/SkXfermodeImageFilter.cpp4
-rw-r--r--tests/SpecialImageTest.cpp11
15 files changed, 92 insertions, 70 deletions
diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp
index be48ca17ba..9373dc1a1d 100644
--- a/src/core/SkImageFilter.cpp
+++ b/src/core/SkImageFilter.cpp
@@ -348,9 +348,9 @@ bool SkImageFilter::applyCropRect(const Context& ctx, const SkIRect& srcBounds,
// around it.
static sk_sp<SkSpecialImage> pad_image(SkSpecialImage* src,
int newWidth, int newHeight, int offX, int offY) {
-
- SkImageInfo info = SkImageInfo::MakeN32Premul(newWidth, newHeight);
- sk_sp<SkSpecialSurface> surf(src->makeSurface(info));
+ // We explicitly want to operate in the source's color space here
+ SkImageFilter::OutputProperties outProps(src->getColorSpace());
+ sk_sp<SkSpecialSurface> surf(src->makeSurface(outProps, SkISize::Make(newWidth, newHeight)));
if (!surf) {
return nullptr;
}
diff --git a/src/core/SkMatrixImageFilter.cpp b/src/core/SkMatrixImageFilter.cpp
index 2e827d82a6..12efc64683 100644
--- a/src/core/SkMatrixImageFilter.cpp
+++ b/src/core/SkMatrixImageFilter.cpp
@@ -70,9 +70,7 @@ sk_sp<SkSpecialImage> SkMatrixImageFilter::onFilterImage(SkSpecialImage* source,
SkIRect dstBounds;
dstRect.roundOut(&dstBounds);
- const SkImageInfo info = SkImageInfo::MakeN32Premul(dstBounds.width(), dstBounds.height());
-
- sk_sp<SkSpecialSurface> surf(input->makeSurface(info));
+ sk_sp<SkSpecialSurface> surf(input->makeSurface(ctx.outputProperties(), dstBounds.size()));
if (!surf) {
return nullptr;
}
diff --git a/src/core/SkSpecialImage.cpp b/src/core/SkSpecialImage.cpp
index 240d469219..5d62c6bc62 100644
--- a/src/core/SkSpecialImage.cpp
+++ b/src/core/SkSpecialImage.cpp
@@ -52,11 +52,13 @@ public:
virtual sk_sp<SkSpecialImage> onMakeSubset(const SkIRect& subset) const = 0;
- virtual sk_sp<SkSpecialSurface> onMakeSurface(const SkImageInfo& info) const = 0;
+ virtual sk_sp<SkSpecialSurface> onMakeSurface(const SkImageFilter::OutputProperties& outProps,
+ const SkISize& size, SkAlphaType at) const = 0;
virtual sk_sp<SkImage> onMakeTightSubset(const SkIRect& subset) const = 0;
- virtual sk_sp<SkSurface> onMakeTightSurface(const SkImageInfo& info) const = 0;
+ virtual sk_sp<SkSurface> onMakeTightSurface(const SkImageFilter::OutputProperties& outProps,
+ const SkISize& size, SkAlphaType at) const = 0;
private:
typedef SkSpecialImage INHERITED;
@@ -151,12 +153,14 @@ sk_sp<GrTexture> SkSpecialImage::asTextureRef(GrContext* context) const {
}
#endif
-sk_sp<SkSpecialSurface> SkSpecialImage::makeSurface(const SkImageInfo& info) const {
- return as_SIB(this)->onMakeSurface(info);
+sk_sp<SkSpecialSurface> SkSpecialImage::makeSurface(const SkImageFilter::OutputProperties& outProps,
+ const SkISize& size, SkAlphaType at) const {
+ return as_SIB(this)->onMakeSurface(outProps, size, at);
}
-sk_sp<SkSurface> SkSpecialImage::makeTightSurface(const SkImageInfo& info) const {
- return as_SIB(this)->onMakeTightSurface(info);
+sk_sp<SkSurface> SkSpecialImage::makeTightSurface(const SkImageFilter::OutputProperties& outProps,
+ const SkISize& size, SkAlphaType at) const {
+ return as_SIB(this)->onMakeTightSurface(outProps, size, at);
}
sk_sp<SkSpecialImage> SkSpecialImage::makeSubset(const SkIRect& subset) const {
@@ -252,7 +256,22 @@ public:
}
#endif
- sk_sp<SkSpecialSurface> onMakeSurface(const SkImageInfo& info) const override {
+// TODO: The raster implementations of image filters all currently assume that the pixels are
+// legacy N32. Until they actually check the format and operate on sRGB or F16 data appropriately,
+// we can't enable this. (They will continue to produce incorrect results, but less-so).
+#define RASTER_IMAGE_FILTERS_SUPPORT_SRGB_AND_F16 0
+
+ sk_sp<SkSpecialSurface> onMakeSurface(const SkImageFilter::OutputProperties& outProps,
+ const SkISize& size, SkAlphaType at) const override {
+#if RASTER_IMAGE_FILTERS_SUPPORT_SRGB_AND_F16
+ SkColorSpace* colorSpace = outProps.colorSpace();
+#else
+ SkColorSpace* colorSpace = nullptr;
+#endif
+ SkColorType colorType = colorSpace && colorSpace->gammaIsLinear()
+ ? kRGBA_F16_SkColorType : kN32_SkColorType;
+ SkImageInfo info = SkImageInfo::Make(size.width(), size.height(), colorType, at,
+ sk_ref_sp(colorSpace));
return SkSpecialSurface::MakeRaster(info, nullptr);
}
@@ -278,7 +297,17 @@ public:
return SkImage::MakeFromBitmap(subsetBM);
}
- sk_sp<SkSurface> onMakeTightSurface(const SkImageInfo& info) const override {
+ sk_sp<SkSurface> onMakeTightSurface(const SkImageFilter::OutputProperties& outProps,
+ const SkISize& size, SkAlphaType at) const override {
+#if RASTER_IMAGE_FILTERS_SUPPORT_SRGB_AND_F16
+ SkColorSpace* colorSpace = outProps.colorSpace();
+#else
+ SkColorSpace* colorSpace = nullptr;
+#endif
+ SkColorType colorType = colorSpace && colorSpace->gammaIsLinear()
+ ? kRGBA_F16_SkColorType : kN32_SkColorType;
+ SkImageInfo info = SkImageInfo::Make(size.width(), size.height(), colorType, at,
+ sk_ref_sp(colorSpace));
return SkSurface::MakeRaster(info);
}
@@ -382,16 +411,16 @@ public:
return fColorSpace.get();
}
- sk_sp<SkSpecialSurface> onMakeSurface(const SkImageInfo& info) const override {
+ sk_sp<SkSpecialSurface> onMakeSurface(const SkImageFilter::OutputProperties& outProps,
+ const SkISize& size, SkAlphaType at) const override {
if (!fTexture->getContext()) {
return nullptr;
}
- GrPixelConfig config = SkImageInfo2GrPixelConfig(info, *fTexture->getContext()->caps());
-
- return SkSpecialSurface::MakeRenderTarget(fTexture->getContext(),
- info.width(), info.height(),
- config, sk_ref_sp(info.colorSpace()));
+ SkColorSpace* colorSpace = outProps.colorSpace();
+ return SkSpecialSurface::MakeRenderTarget(
+ fTexture->getContext(), size.width(), size.height(),
+ GrRenderableConfigForColorSpace(colorSpace), sk_ref_sp(colorSpace));
}
sk_sp<SkSpecialImage> onMakeSubset(const SkIRect& subset) const override {
@@ -428,7 +457,13 @@ public:
fAlphaType, subTx.get(), fColorSpace, SkBudgeted::kYes);
}
- sk_sp<SkSurface> onMakeTightSurface(const SkImageInfo& info) const override {
+ sk_sp<SkSurface> onMakeTightSurface(const SkImageFilter::OutputProperties& outProps,
+ const SkISize& size, SkAlphaType at) const override {
+ SkColorSpace* colorSpace = outProps.colorSpace();
+ SkColorType colorType = colorSpace && colorSpace->gammaIsLinear()
+ ? kRGBA_F16_SkColorType : kRGBA_8888_SkColorType;
+ SkImageInfo info = SkImageInfo::Make(size.width(), size.height(), colorType, at,
+ sk_ref_sp(colorSpace));
return SkSurface::MakeRenderTarget(fTexture->getContext(), SkBudgeted::kYes, info);
}
diff --git a/src/core/SkSpecialImage.h b/src/core/SkSpecialImage.h
index cd8c3141fb..c1f3791edf 100644
--- a/src/core/SkSpecialImage.h
+++ b/src/core/SkSpecialImage.h
@@ -12,7 +12,8 @@
#include "SkRefCnt.h"
#include "SkSurfaceProps.h"
-#include "SkImageInfo.h" // for SkAlphaType
+#include "SkImageFilter.h" // for OutputProperties
+#include "SkImageInfo.h" // for SkAlphaType
class GrContext;
class GrTexture;
@@ -86,13 +87,17 @@ public:
/**
* Create a new special surface with a backend that is compatible with this special image.
*/
- sk_sp<SkSpecialSurface> makeSurface(const SkImageInfo&) const;
+ sk_sp<SkSpecialSurface> makeSurface(const SkImageFilter::OutputProperties& outProps,
+ const SkISize& size,
+ SkAlphaType at = kPremul_SkAlphaType) const;
/**
* Create a new surface with a backend that is compatible with this special image.
* TODO: switch this to makeSurface once we resolved the naming issue
*/
- sk_sp<SkSurface> makeTightSurface(const SkImageInfo&) const;
+ sk_sp<SkSurface> makeTightSurface(const SkImageFilter::OutputProperties& outProps,
+ const SkISize& size,
+ SkAlphaType at = kPremul_SkAlphaType) const;
/**
* Extract a subset of this special image and return it as a special image.
diff --git a/src/effects/SkColorFilterImageFilter.cpp b/src/effects/SkColorFilterImageFilter.cpp
index 63d5942f6a..d6b23d5038 100644
--- a/src/effects/SkColorFilterImageFilter.cpp
+++ b/src/effects/SkColorFilterImageFilter.cpp
@@ -79,8 +79,7 @@ sk_sp<SkSpecialImage> SkColorFilterImageFilter::onFilterImage(SkSpecialImage* so
return nullptr;
}
- SkImageInfo info = SkImageInfo::MakeN32(bounds.width(), bounds.height(), kPremul_SkAlphaType);
- sk_sp<SkSpecialSurface> surf(source->makeSurface(info));
+ sk_sp<SkSpecialSurface> surf(source->makeSurface(ctx.outputProperties(), bounds.size()));
if (!surf) {
return nullptr;
}
diff --git a/src/effects/SkDisplacementMapEffect.cpp b/src/effects/SkDisplacementMapEffect.cpp
index 8d068072a7..4f6386d92e 100644
--- a/src/effects/SkDisplacementMapEffect.cpp
+++ b/src/effects/SkDisplacementMapEffect.cpp
@@ -279,7 +279,18 @@ sk_sp<SkSpecialImage> SkDisplacementMapEffect::onFilterImage(SkSpecialImage* sou
}
SkIPoint displOffset = SkIPoint::Make(0, 0);
- sk_sp<SkSpecialImage> displ(this->filterInput(0, source, ctx, &displOffset));
+ // Creation of the displacement map should happen in a non-colorspace aware context. This
+ // texture is a purely mathematical construct, so we want to just operate on the stored
+ // values. Consider:
+ // User supplies an sRGB displacement map. If we're rendering to a wider gamut, then we could
+ // end up filtering the displacement map into that gamut, which has the effect of reducing
+ // the amount of displacement that it represents (as encoded values move away from the
+ // primaries).
+ // With a more complex DAG attached to this input, it's not clear that working in ANY specific
+ // color space makes sense, so we ignore color spaces (and gamma) entirely. This may not be
+ // ideal, but it's at least consistent and predictable.
+ Context displContext(ctx.ctm(), ctx.clipBounds(), ctx.cache(), OutputProperties(nullptr));
+ sk_sp<SkSpecialImage> displ(this->filterInput(0, source, displContext, &displOffset));
if (!displ) {
return nullptr;
}
diff --git a/src/effects/SkDropShadowImageFilter.cpp b/src/effects/SkDropShadowImageFilter.cpp
index 5befe71194..b4b8cac4f0 100644
--- a/src/effects/SkDropShadowImageFilter.cpp
+++ b/src/effects/SkDropShadowImageFilter.cpp
@@ -77,9 +77,7 @@ sk_sp<SkSpecialImage> SkDropShadowImageFilter::onFilterImage(SkSpecialImage* sou
return nullptr;
}
- const SkImageInfo info = SkImageInfo::MakeN32(bounds.width(), bounds.height(),
- kPremul_SkAlphaType);
- sk_sp<SkSpecialSurface> surf(source->makeSurface(info));
+ sk_sp<SkSpecialSurface> surf(source->makeSurface(ctx.outputProperties(), bounds.size()));
if (!surf) {
return nullptr;
}
diff --git a/src/effects/SkImageSource.cpp b/src/effects/SkImageSource.cpp
index f3fc05491e..f434de4aa0 100644
--- a/src/effects/SkImageSource.cpp
+++ b/src/effects/SkImageSource.cpp
@@ -93,11 +93,7 @@ sk_sp<SkSpecialImage> SkImageSource::onFilterImage(SkSpecialImage* source, const
const SkIRect dstIRect = dstRect.roundOut();
- // SRGBTODO: Propagate SkColorType?
- const SkImageInfo info = SkImageInfo::MakeN32(dstIRect.width(), dstIRect.height(),
- kPremul_SkAlphaType);
-
- sk_sp<SkSpecialSurface> surf(source->makeSurface(info));
+ sk_sp<SkSpecialSurface> surf(source->makeSurface(ctx.outputProperties(), dstIRect.size()));
if (!surf) {
return nullptr;
}
diff --git a/src/effects/SkMergeImageFilter.cpp b/src/effects/SkMergeImageFilter.cpp
index 2dd02ee529..cc7e336a3d 100755
--- a/src/effects/SkMergeImageFilter.cpp
+++ b/src/effects/SkMergeImageFilter.cpp
@@ -113,10 +113,7 @@ sk_sp<SkSpecialImage> SkMergeImageFilter::onFilterImage(SkSpecialImage* source,
const int x0 = bounds.left();
const int y0 = bounds.top();
- SkImageInfo info = SkImageInfo::MakeN32(bounds.width(), bounds.height(),
- kPremul_SkAlphaType);
-
- sk_sp<SkSpecialSurface> surf(source->makeSurface(info));
+ sk_sp<SkSpecialSurface> surf(source->makeSurface(ctx.outputProperties(), bounds.size()));
if (!surf) {
return nullptr;
}
diff --git a/src/effects/SkOffsetImageFilter.cpp b/src/effects/SkOffsetImageFilter.cpp
index af60164d99..1c99154f16 100644
--- a/src/effects/SkOffsetImageFilter.cpp
+++ b/src/effects/SkOffsetImageFilter.cpp
@@ -49,9 +49,7 @@ sk_sp<SkSpecialImage> SkOffsetImageFilter::onFilterImage(SkSpecialImage* source,
return nullptr;
}
- SkImageInfo info = SkImageInfo::MakeN32(bounds.width(), bounds.height(),
- kPremul_SkAlphaType);
- sk_sp<SkSpecialSurface> surf(source->makeSurface(info));
+ sk_sp<SkSpecialSurface> surf(source->makeSurface(ctx.outputProperties(), bounds.size()));
if (!surf) {
return nullptr;
}
diff --git a/src/effects/SkPaintImageFilter.cpp b/src/effects/SkPaintImageFilter.cpp
index 8833629eed..0a0e4e92ed 100644
--- a/src/effects/SkPaintImageFilter.cpp
+++ b/src/effects/SkPaintImageFilter.cpp
@@ -43,10 +43,7 @@ sk_sp<SkSpecialImage> SkPaintImageFilter::onFilterImage(SkSpecialImage* source,
return nullptr;
}
- SkImageInfo info = SkImageInfo::MakeN32(bounds.width(), bounds.height(),
- kPremul_SkAlphaType);
-
- sk_sp<SkSpecialSurface> surf(source->makeSurface(info));
+ sk_sp<SkSpecialSurface> surf(source->makeSurface(ctx.outputProperties(), bounds.size()));
if (!surf) {
return nullptr;
}
diff --git a/src/effects/SkPictureImageFilter.cpp b/src/effects/SkPictureImageFilter.cpp
index bfe26b62c5..6539104a30 100644
--- a/src/effects/SkPictureImageFilter.cpp
+++ b/src/effects/SkPictureImageFilter.cpp
@@ -118,8 +118,7 @@ sk_sp<SkSpecialImage> SkPictureImageFilter::onFilterImage(SkSpecialImage* source
SkASSERT(!bounds.isEmpty());
- SkImageInfo info = SkImageInfo::MakeN32(bounds.width(), bounds.height(), kPremul_SkAlphaType);
- sk_sp<SkSpecialSurface> surf(source->makeSurface(info));
+ sk_sp<SkSpecialSurface> surf(source->makeSurface(ctx.outputProperties(), bounds.size()));
if (!surf) {
return nullptr;
}
@@ -167,10 +166,8 @@ void SkPictureImageFilter::drawPictureAtLocalResolution(SkSpecialImage* source,
sk_sp<SkSpecialImage> localImg;
{
- const SkImageInfo info = SkImageInfo::MakeN32(localIBounds.width(), localIBounds.height(),
- kPremul_SkAlphaType);
-
- sk_sp<SkSpecialSurface> localSurface(source->makeSurface(info));
+ sk_sp<SkSpecialSurface> localSurface(source->makeSurface(ctx.outputProperties(),
+ localIBounds.size()));
if (!localSurface) {
return;
}
diff --git a/src/effects/SkTileImageFilter.cpp b/src/effects/SkTileImageFilter.cpp
index c098bc2955..46c4d9af67 100644
--- a/src/effects/SkTileImageFilter.cpp
+++ b/src/effects/SkTileImageFilter.cpp
@@ -55,9 +55,7 @@ sk_sp<SkSpecialImage> SkTileImageFilter::onFilterImage(SkSpecialImage* source,
}
const SkIRect dstIRect = dstRect.roundOut();
- int dstWidth = dstIRect.width();
- int dstHeight = dstIRect.height();
- if (!fSrcRect.width() || !fSrcRect.height() || !dstWidth || !dstHeight) {
+ if (!fSrcRect.width() || !fSrcRect.height() || !dstIRect.width() || !dstIRect.height()) {
return nullptr;
}
@@ -80,9 +78,7 @@ sk_sp<SkSpecialImage> SkTileImageFilter::onFilterImage(SkSpecialImage* source,
return nullptr;
}
} else {
- const SkImageInfo info = SkImageInfo::MakeN32(srcIRect.width(), srcIRect.height(),
- kPremul_SkAlphaType);
- sk_sp<SkSurface> surf(input->makeTightSurface(info));
+ sk_sp<SkSurface> surf(input->makeTightSurface(ctx.outputProperties(), srcIRect.size()));
if (!surf) {
return nullptr;
}
@@ -102,9 +98,7 @@ sk_sp<SkSpecialImage> SkTileImageFilter::onFilterImage(SkSpecialImage* source,
SkASSERT(subset->width() == srcIRect.width());
SkASSERT(subset->height() == srcIRect.height());
- const SkImageInfo info = SkImageInfo::MakeN32(dstWidth, dstHeight, kPremul_SkAlphaType);
-
- sk_sp<SkSpecialSurface> surf(source->makeSurface(info));
+ sk_sp<SkSpecialSurface> surf(source->makeSurface(ctx.outputProperties(), dstIRect.size()));
if (!surf) {
return nullptr;
}
diff --git a/src/effects/SkXfermodeImageFilter.cpp b/src/effects/SkXfermodeImageFilter.cpp
index c7e4143e11..d655c9484f 100644
--- a/src/effects/SkXfermodeImageFilter.cpp
+++ b/src/effects/SkXfermodeImageFilter.cpp
@@ -95,9 +95,7 @@ sk_sp<SkSpecialImage> SkXfermodeImageFilter::onFilterImage(SkSpecialImage* sourc
}
#endif
- const SkImageInfo info = SkImageInfo::MakeN32(bounds.width(), bounds.height(),
- kPremul_SkAlphaType);
- sk_sp<SkSpecialSurface> surf(source->makeSurface(info));
+ sk_sp<SkSpecialSurface> surf(source->makeSurface(ctx.outputProperties(), bounds.size()));
if (!surf) {
return nullptr;
}
diff --git a/tests/SpecialImageTest.cpp b/tests/SpecialImageTest.cpp
index ba0eb58f4e..e992177430 100644
--- a/tests/SpecialImageTest.cpp
+++ b/tests/SpecialImageTest.cpp
@@ -85,9 +85,9 @@ static void test_image(const sk_sp<SkSpecialImage>& img, skiatest::Reporter* rep
//--------------
// Test that draw restricts itself to the subset
- SkImageInfo info = SkImageInfo::MakeN32(kFullSize, kFullSize, kOpaque_SkAlphaType);
-
- sk_sp<SkSpecialSurface> surf(img->makeSurface(info));
+ SkImageFilter::OutputProperties outProps(img->getColorSpace());
+ sk_sp<SkSpecialSurface> surf(img->makeSurface(outProps, SkISize::Make(kFullSize, kFullSize),
+ kOpaque_SkAlphaType));
SkCanvas* canvas = surf->getCanvas();
@@ -122,9 +122,8 @@ static void test_image(const sk_sp<SkSpecialImage>& img, skiatest::Reporter* rep
REPORTER_ASSERT(reporter, peekTextureSucceeds != !!tightImg->peekPixels(&tmpPixmap));
}
{
- SkImageInfo info = SkImageInfo::MakeN32(subset.width(), subset.height(),
- kPremul_SkAlphaType);
- sk_sp<SkSurface> tightSurf(img->makeTightSurface(info));
+ SkImageFilter::OutputProperties outProps(img->getColorSpace());
+ sk_sp<SkSurface> tightSurf(img->makeTightSurface(outProps, subset.size()));
REPORTER_ASSERT(reporter, tightSurf->width() == subset.width());
REPORTER_ASSERT(reporter, tightSurf->height() == subset.height());