Add color type to the image filter DAG's OutputProperties

Don't try to guess the pixel config to use for intermediates. Instead,
just make the intermediates in the same color type (and space) as the
final destination. This removes some no-longer-correct logic that was
using sRGB configs, resulting in linear blending and precision loss.

Change-Id: I627c47193a9f2889c3dc121170ff3e7d5d315fa0
Reviewed-on: https://skia-review.googlesource.com/139547
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This commit is contained in:
Brian Osman 2018-07-06 13:57:01 -04:00 committed by Skia Commit-Bot
parent 5a619a740f
commit a50205fca5
16 changed files with 37 additions and 50 deletions

View File

@ -14,6 +14,7 @@
#include "SkColorSpace.h"
#include "SkFilterQuality.h"
#include "SkFlattenable.h"
#include "SkImageInfo.h"
#include "SkMatrix.h"
#include "SkRect.h"
@ -41,11 +42,14 @@ public:
// consumer of the DAG's output.
class OutputProperties {
public:
explicit OutputProperties(SkColorSpace* colorSpace) : fColorSpace(colorSpace) {}
explicit OutputProperties(SkColorType colorType, SkColorSpace* colorSpace)
: fColorType(colorType), fColorSpace(colorSpace) {}
SkColorType colorType() const { return fColorType; }
SkColorSpace* colorSpace() const { return fColorSpace; }
private:
SkColorType fColorType;
// This will be a pointer to the device's color space, and our lifetime is bounded by
// the device, so we can store a bare pointer.
SkColorSpace* fColorSpace;

View File

@ -643,7 +643,7 @@ void SkBitmapDevice::drawSpecial(SkSpecialImage* src, int x, int y, const SkPain
SkMatrix::MakeTrans(SkIntToScalar(-x), SkIntToScalar(-y)), this->ctm());
const SkIRect clipBounds = fRCStack.rc().getBounds().makeOffset(-x, -y);
sk_sp<SkImageFilterCache> cache(this->getImageFilterCache());
SkImageFilter::OutputProperties outputProperties(fBitmap.colorSpace());
SkImageFilter::OutputProperties outputProperties(fBitmap.colorType(), fBitmap.colorSpace());
SkImageFilter::Context ctx(matrix, clipBounds, cache.get(), outputProperties);
filteredImage = filter->filterImage(src, ctx, &offset);

View File

@ -247,7 +247,7 @@ sk_sp<SkSpecialImage> SkImageFilter::DrawWithFP(GrContext* context,
paint.setPorterDuffXPFactory(SkBlendMode::kSrc);
sk_sp<SkColorSpace> colorSpace = sk_ref_sp(outputProperties.colorSpace());
GrPixelConfig config = GrRenderableConfigForColorSpace(colorSpace.get());
GrPixelConfig config = SkColorType2GrPixelConfig(outputProperties.colorType());
sk_sp<GrRenderTargetContext> renderTargetContext(
context->contextPriv().makeDeferredRenderTargetContext(
SkBackingFit::kApprox, bounds.width(), bounds.height(),

View File

@ -432,10 +432,9 @@ public:
return nullptr;
}
SkColorSpace* colorSpace = outProps.colorSpace();
return SkSpecialSurface::MakeRenderTarget(
fContext, size.width(), size.height(),
GrRenderableConfigForColorSpace(colorSpace), sk_ref_sp(colorSpace));
SkColorType2GrPixelConfig(outProps.colorType()), sk_ref_sp(outProps.colorSpace()));
}
sk_sp<SkSpecialImage> onMakeSubset(const SkIRect& subset) const override {

View File

@ -350,7 +350,7 @@ sk_sp<SkSpecialImage> ArithmeticImageFilterImpl::filterImageGPU(
sk_sp<GrRenderTargetContext> renderTargetContext(
context->contextPriv().makeDeferredRenderTargetContext(
SkBackingFit::kApprox, bounds.width(), bounds.height(),
GrRenderableConfigForColorSpace(outputProperties.colorSpace()),
SkColorType2GrPixelConfig(outputProperties.colorType()),
sk_ref_sp(outputProperties.colorSpace())));
if (!renderTargetContext) {
return nullptr;

View File

@ -243,7 +243,8 @@ sk_sp<SkSpecialImage> SkDisplacementMapEffect::onFilterImage(SkSpecialImage* sou
// 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));
Context displContext(ctx.ctm(), ctx.clipBounds(), ctx.cache(),
OutputProperties(kN32_SkColorType, nullptr));
sk_sp<SkSpecialImage> displ(this->filterInput(0, source, displContext, &displOffset));
if (!displ) {
return nullptr;
@ -308,12 +309,12 @@ sk_sp<SkSpecialImage> SkDisplacementMapEffect::onFilterImage(SkSpecialImage* sou
paint.setPorterDuffXPFactory(SkBlendMode::kSrc);
SkMatrix matrix;
matrix.setTranslate(-SkIntToScalar(colorBounds.x()), -SkIntToScalar(colorBounds.y()));
GrPixelConfig config = SkColorType2GrPixelConfig(ctx.outputProperties().colorType());
sk_sp<GrRenderTargetContext> renderTargetContext(
context->contextPriv().makeDeferredRenderTargetContext(SkBackingFit::kApprox,
bounds.width(), bounds.height(),
GrRenderableConfigForColorSpace(colorSpace),
sk_ref_sp(colorSpace)));
config, sk_ref_sp(colorSpace)));
if (!renderTargetContext) {
return nullptr;
}

View File

@ -469,7 +469,7 @@ sk_sp<SkSpecialImage> SkLightingImageFilterInternal::filterImageGPU(
sk_sp<GrRenderTargetContext> renderTargetContext(
context->contextPriv().makeDeferredRenderTargetContext(
SkBackingFit::kApprox, offsetBounds.width(), offsetBounds.height(),
GrRenderableConfigForColorSpace(outputProperties.colorSpace()),
SkColorType2GrPixelConfig(outputProperties.colorType()),
sk_ref_sp(outputProperties.colorSpace())));
if (!renderTargetContext) {
return nullptr;

View File

@ -466,7 +466,7 @@ static sk_sp<SkSpecialImage> apply_morphology(
sk_sp<GrTextureProxy> srcTexture(input->asTextureProxyRef(context));
SkASSERT(srcTexture);
sk_sp<SkColorSpace> colorSpace = sk_ref_sp(outputProperties.colorSpace());
GrPixelConfig config = GrRenderableConfigForColorSpace(colorSpace.get());
GrPixelConfig config = SkColorType2GrPixelConfig(outputProperties.colorType());
// setup new clip
const GrFixedClip clip(SkIRect::MakeWH(srcTexture->width(), srcTexture->height()));

View File

@ -319,7 +319,7 @@ sk_sp<SkSpecialImage> SkXfermodeImageFilter_Base::filterImageGPU(
sk_sp<GrRenderTargetContext> renderTargetContext(
context->contextPriv().makeDeferredRenderTargetContext(
SkBackingFit::kApprox, bounds.width(), bounds.height(),
GrRenderableConfigForColorSpace(outputProperties.colorSpace()),
SkColorType2GrPixelConfig(outputProperties.colorType()),
sk_ref_sp(outputProperties.colorSpace())));
if (!renderTargetContext) {
return nullptr;

View File

@ -183,8 +183,12 @@ sk_sp<SkSpecialImage> SkGpuDevice::filterTexture(SkSpecialImage* srcImg,
matrix.postTranslate(SkIntToScalar(-left), SkIntToScalar(-top));
const SkIRect clipBounds = this->devClipBounds().makeOffset(-left, -top);
sk_sp<SkImageFilterCache> cache(this->getImageFilterCache());
SkColorType colorType;
if (!GrPixelConfigToColorType(fRenderTargetContext->colorSpaceInfo().config(), &colorType)) {
colorType = kN32_SkColorType;
}
SkImageFilter::OutputProperties outputProperties(
fRenderTargetContext->colorSpaceInfo().colorSpace());
colorType, fRenderTargetContext->colorSpaceInfo().colorSpace());
SkImageFilter::Context ctx(matrix, clipBounds, cache.get(), outputProperties);
return filter->filterImage(srcImg, ctx, offset);

View File

@ -274,20 +274,6 @@ bool GrPixelConfigToColorType(GrPixelConfig config, SkColorType* ctOut) {
return false;
}
GrPixelConfig GrRenderableConfigForColorSpace(const SkColorSpace* colorSpace) {
if (!colorSpace) {
return kRGBA_8888_GrPixelConfig;
} else if (colorSpace->gammaIsLinear()) {
// TODO
return kRGBA_half_GrPixelConfig;
} else if (colorSpace->gammaCloseToSRGB()) {
return kSRGBA_8888_GrPixelConfig;
} else {
SkDEBUGFAIL("No renderable config exists for color space with strange gamma");
return kUnknown_GrPixelConfig;
}
}
////////////////////////////////////////////////////////////////////////////////////////////////
static inline bool blend_requires_shader(const SkBlendMode mode) {

View File

@ -245,12 +245,4 @@ void GrMakeKeyFromImageID(GrUniqueKey* key, uint32_t imageID, const SkIRect& ima
removed should the bitmap's contents change or be destroyed. */
void GrInstallBitmapUniqueKeyInvalidator(const GrUniqueKey& key, SkPixelRef* pixelRef);
//////////////////////////////////////////////////////////////////////////////
/** When image filter code needs to construct a render target context to do intermediate rendering,
we need a renderable pixel config. The source (SkSpecialImage) may not be in a renderable
format, but we want to preserve the color space of that source. This picks an appropriate format
to use. */
GrPixelConfig GrRenderableConfigForColorSpace(const SkColorSpace*);
#endif

View File

@ -256,6 +256,7 @@ sk_sp<SkImage> SkImage::makeWithFilter(const SkImageFilter* filter, const SkIRec
if (!filter || !outSubset || !offset || !this->bounds().contains(subset)) {
return nullptr;
}
SkColorType colorType = as_IB(this)->onImageInfo().colorType();
SkColorSpace* colorSpace = as_IB(this)->onImageInfo().colorSpace();
sk_sp<SkSpecialImage> srcSpecialImage = SkSpecialImage::MakeFromImage(
subset, sk_ref_sp(const_cast<SkImage*>(this)), colorSpace);
@ -265,7 +266,7 @@ sk_sp<SkImage> SkImage::makeWithFilter(const SkImageFilter* filter, const SkIRec
sk_sp<SkImageFilterCache> cache(
SkImageFilterCache::Create(SkImageFilterCache::kDefaultTransientSize));
SkImageFilter::OutputProperties outputProperties(colorSpace);
SkImageFilter::OutputProperties outputProperties(colorType, colorSpace);
SkImageFilter::Context context(SkMatrix::I(), clipBounds, cache.get(), outputProperties);
sk_sp<SkSpecialImage> result = filter->filterImage(srcSpecialImage.get(), context, offset);

View File

@ -2342,9 +2342,9 @@ void SkPDFDevice::drawSpecial(SkSpecialImage* srcImg, int x, int y, const SkPain
const SkIRect clipBounds =
this->cs().bounds(this->bounds()).roundOut().makeOffset(-x, -y);
sk_sp<SkImageFilterCache> cache(this->getImageFilterCache());
// TODO: Should PDF be operating in a specified color space? For now, run the filter
// TODO: Should PDF be operating in a specified color type/space? For now, run the filter
// in the same color space as the source (this is different from all other backends).
SkImageFilter::OutputProperties outputProperties(srcImg->getColorSpace());
SkImageFilter::OutputProperties outputProperties(kN32_SkColorType, srcImg->getColorSpace());
SkImageFilter::Context ctx(matrix, clipBounds, cache.get(), outputProperties);
sk_sp<SkSpecialImage> resultImg(filter->filterImage(srcImg, ctx, &offset));

View File

@ -544,7 +544,7 @@ static void test_crop_rects(skiatest::Reporter* reporter,
for (int i = 0; i < filters.count(); ++i) {
SkImageFilter* filter = filters.getFilter(i);
SkIPoint offset;
SkImageFilter::OutputProperties noColorSpace(nullptr);
SkImageFilter::OutputProperties noColorSpace(kN32_SkColorType, nullptr);
SkImageFilter::Context ctx(SkMatrix::I(), SkIRect::MakeWH(100, 100), nullptr, noColorSpace);
sk_sp<SkSpecialImage> resultImg(filter->filterImage(srcImg.get(), ctx, &offset));
REPORTER_ASSERT(reporter, resultImg, filters.getName(i));
@ -567,7 +567,7 @@ static void test_negative_blur_sigma(skiatest::Reporter* reporter,
gradient));
SkIPoint offset;
SkImageFilter::OutputProperties noColorSpace(nullptr);
SkImageFilter::OutputProperties noColorSpace(kN32_SkColorType, nullptr);
SkImageFilter::Context ctx(SkMatrix::I(), SkIRect::MakeWH(32, 32), nullptr, noColorSpace);
sk_sp<SkSpecialImage> positiveResult1(positiveFilter->filterImage(imgSrc.get(), ctx, &offset));
@ -644,7 +644,7 @@ static void test_zero_blur_sigma(skiatest::Reporter* reporter, GrContext* contex
sk_sp<SkSpecialImage> image(surf->makeImageSnapshot());
SkIPoint offset;
SkImageFilter::OutputProperties noColorSpace(nullptr);
SkImageFilter::OutputProperties noColorSpace(kN32_SkColorType, nullptr);
SkImageFilter::Context ctx(SkMatrix::I(), SkIRect::MakeWH(32, 32), nullptr, noColorSpace);
sk_sp<SkSpecialImage> result(filter->filterImage(image.get(), ctx, &offset));
@ -681,7 +681,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ImageFilterZeroBlurSigma_Gpu, reporter, ctxIn
static void test_fail_affects_transparent_black(skiatest::Reporter* reporter, GrContext* context) {
sk_sp<FailImageFilter> failFilter(new FailImageFilter());
sk_sp<SkSpecialImage> source(create_empty_special_image(context, 5));
SkImageFilter::OutputProperties noColorSpace(nullptr);
SkImageFilter::OutputProperties noColorSpace(kN32_SkColorType, nullptr);
SkImageFilter::Context ctx(SkMatrix::I(), SkIRect::MakeXYWH(0, 0, 1, 1), nullptr, noColorSpace);
sk_sp<SkColorFilter> green(SkColorFilter::MakeModeFilter(SK_ColorGREEN, SkBlendMode::kSrc));
SkASSERT(green->affectsTransparentBlack());
@ -975,7 +975,7 @@ static void test_imagefilter_merge_result_size(skiatest::Reporter* reporter, GrC
sk_sp<SkSpecialImage> srcImg(create_empty_special_image(context, 1));
SkImageFilter::OutputProperties noColorSpace(nullptr);
SkImageFilter::OutputProperties noColorSpace(kN32_SkColorType, nullptr);
SkImageFilter::Context ctx(SkMatrix::I(), SkIRect::MakeXYWH(0, 0, 100, 100), nullptr,
noColorSpace);
SkIPoint offset;
@ -1144,7 +1144,7 @@ static void test_big_kernel(skiatest::Reporter* reporter, GrContext* context) {
SkASSERT(srcImg);
SkIPoint offset;
SkImageFilter::OutputProperties noColorSpace(nullptr);
SkImageFilter::OutputProperties noColorSpace(kN32_SkColorType, nullptr);
SkImageFilter::Context ctx(SkMatrix::I(), SkIRect::MakeWH(100, 100), nullptr, noColorSpace);
sk_sp<SkSpecialImage> resultImg(filter->filterImage(srcImg.get(), ctx, &offset));
REPORTER_ASSERT(reporter, resultImg);
@ -1216,7 +1216,7 @@ static void test_clipped_picture_imagefilter(skiatest::Reporter* reporter, GrCon
sk_sp<SkImageFilter> imageFilter(SkPictureImageFilter::Make(picture));
SkIPoint offset;
SkImageFilter::OutputProperties noColorSpace(nullptr);
SkImageFilter::OutputProperties noColorSpace(kN32_SkColorType, nullptr);
SkImageFilter::Context ctx(SkMatrix::I(), SkIRect::MakeXYWH(1, 1, 1, 1), nullptr, noColorSpace);
sk_sp<SkSpecialImage> resultImage(imageFilter->filterImage(srcImg.get(), ctx, &offset));
@ -1469,7 +1469,7 @@ static void test_composed_imagefilter_offset(skiatest::Reporter* reporter, GrCon
sk_sp<SkImageFilter> composedFilter(SkComposeImageFilter::Make(std::move(blurFilter),
std::move(offsetFilter)));
SkIPoint offset;
SkImageFilter::OutputProperties noColorSpace(nullptr);
SkImageFilter::OutputProperties noColorSpace(kN32_SkColorType, nullptr);
SkImageFilter::Context ctx(SkMatrix::I(), SkIRect::MakeWH(100, 100), nullptr, noColorSpace);
sk_sp<SkSpecialImage> resultImg(composedFilter->filterImage(srcImg.get(), ctx, &offset));
@ -1504,7 +1504,7 @@ static void test_composed_imagefilter_bounds(skiatest::Reporter* reporter, GrCon
std::move(pictureFilter)));
sk_sp<SkSpecialImage> sourceImage(create_empty_special_image(context, 100));
SkImageFilter::OutputProperties noColorSpace(nullptr);
SkImageFilter::OutputProperties noColorSpace(kN32_SkColorType, nullptr);
SkImageFilter::Context ctx(SkMatrix::I(), SkIRect::MakeWH(100, 100), nullptr, noColorSpace);
SkIPoint offset;
sk_sp<SkSpecialImage> result(composedFilter->filterImage(sourceImage.get(), ctx, &offset));
@ -1532,7 +1532,7 @@ static void test_partial_crop_rect(skiatest::Reporter* reporter, GrContext* cont
SkImageFilter::CropRect::kHasWidth_CropEdge | SkImageFilter::CropRect::kHasHeight_CropEdge);
sk_sp<SkImageFilter> filter(make_grayscale(nullptr, &cropRect));
SkIPoint offset;
SkImageFilter::OutputProperties noColorSpace(nullptr);
SkImageFilter::OutputProperties noColorSpace(kN32_SkColorType, nullptr);
SkImageFilter::Context ctx(SkMatrix::I(), SkIRect::MakeWH(100, 100), nullptr, noColorSpace);
sk_sp<SkSpecialImage> resultImg(filter->filterImage(srcImg.get(), ctx, &offset));

View File

@ -87,7 +87,7 @@ static void test_image(const sk_sp<SkSpecialImage>& img, skiatest::Reporter* rep
//--------------
// Test that draw restricts itself to the subset
SkImageFilter::OutputProperties outProps(img->getColorSpace());
SkImageFilter::OutputProperties outProps(kN32_SkColorType, img->getColorSpace());
sk_sp<SkSpecialSurface> surf(img->makeSurface(outProps, SkISize::Make(kFullSize, kFullSize),
kPremul_SkAlphaType));
@ -124,7 +124,7 @@ static void test_image(const sk_sp<SkSpecialImage>& img, skiatest::Reporter* rep
REPORTER_ASSERT(reporter, isGPUBacked != !!tightImg->peekPixels(&tmpPixmap));
}
{
SkImageFilter::OutputProperties outProps(img->getColorSpace());
SkImageFilter::OutputProperties outProps(kN32_SkColorType, img->getColorSpace());
sk_sp<SkSurface> tightSurf(img->makeTightSurface(outProps, subset.size()));
REPORTER_ASSERT(reporter, tightSurf->width() == subset.width());