From eef980270d3385fee340eb1633962fe3ba8b7132 Mon Sep 17 00:00:00 2001 From: brianosman Date: Mon, 29 Feb 2016 13:39:09 -0800 Subject: [PATCH] Progress on gamma-correctness in the GPU backend. Fixed conversion of color and profile type to pixel config, which makes many things "just work". Added (color=8888|f16|srgb) option to gpu configurations, along with gpuf16, gpusrgb, and anglesrgb predefined configs. Runs the gpu backend in gamma-correct mode (with either FP16 linear or sRGB 8888 frambuffers). BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1746253002 Review URL: https://codereview.chromium.org/1746253002 --- dm/DM.cpp | 3 ++- dm/DMSrcSink.cpp | 7 ++++- dm/DMSrcSink.h | 5 +++- src/gpu/SkGr.cpp | 11 ++++---- tests/TestConfigParsing.cpp | 17 +++++++++++- tools/flags/SkCommonFlagsConfig.cpp | 54 ++++++++++++++++++++++++++++++++----- tools/flags/SkCommonFlagsConfig.h | 7 ++++- 7 files changed, 88 insertions(+), 16 deletions(-) diff --git a/dm/DM.cpp b/dm/DM.cpp index 4279ea3..f5e4682 100644 --- a/dm/DM.cpp +++ b/dm/DM.cpp @@ -725,7 +725,8 @@ static Sink* create_sink(const SkCommandLineConfig* config) { return nullptr; } return new GPUSink(contextType, contextOptions, gpuConfig->getSamples(), - gpuConfig->getUseDIText(), FLAGS_gpu_threading); + gpuConfig->getUseDIText(), gpuConfig->getColorType(), + gpuConfig->getProfileType(), FLAGS_gpu_threading); } } #endif diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp index 2d2a455..c1db178 100644 --- a/dm/DMSrcSink.cpp +++ b/dm/DMSrcSink.cpp @@ -856,11 +856,15 @@ GPUSink::GPUSink(GrContextFactory::GLContextType ct, GrContextFactory::GLContextOptions options, int samples, bool diText, + SkColorType colorType, + SkColorProfileType profileType, bool threaded) : fContextType(ct) , fContextOptions(options) , fSampleCount(samples) , fUseDIText(diText) + , fColorType(colorType) + , fProfileType(profileType) , fThreaded(threaded) {} void PreAbandonGpuContextErrorHandler(SkError, void*) {} @@ -882,7 +886,8 @@ Error GPUSink::draw(const Src& src, SkBitmap* dst, SkWStream*, SkString* log) co GrContextFactory factory(grOptions); const SkISize size = src.size(); const SkImageInfo info = - SkImageInfo::Make(size.width(), size.height(), kN32_SkColorType, kPremul_SkAlphaType); + SkImageInfo::Make(size.width(), size.height(), fColorType, + kPremul_SkAlphaType, fProfileType); #if SK_SUPPORT_GPU const int maxDimension = factory.getContextInfo(fContextType, fContextOptions). fGrContext->caps()->maxTextureSize(); diff --git a/dm/DMSrcSink.h b/dm/DMSrcSink.h index bbf47cf..babe14d 100644 --- a/dm/DMSrcSink.h +++ b/dm/DMSrcSink.h @@ -213,7 +213,8 @@ public: class GPUSink : public Sink { public: GPUSink(GrContextFactory::GLContextType, GrContextFactory::GLContextOptions, - int samples, bool diText, bool threaded); + int samples, bool diText, SkColorType colorType, SkColorProfileType profileType, + bool threaded); Error draw(const Src&, SkBitmap*, SkWStream*, SkString*) const override; bool serial() const override { return !fThreaded; } @@ -224,6 +225,8 @@ private: GrContextFactory::GLContextOptions fContextOptions; int fSampleCount; bool fUseDIText; + SkColorType fColorType; + SkColorProfileType fProfileType; bool fThreaded; }; diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp index a3848c7..4106d49 100644 --- a/src/gpu/SkGr.cpp +++ b/src/gpu/SkGr.cpp @@ -370,12 +370,13 @@ GrPixelConfig SkImageInfo2GrPixelConfig(SkColorType ct, SkAlphaType, SkColorProf case kARGB_4444_SkColorType: return kRGBA_4444_GrPixelConfig; case kRGBA_8888_SkColorType: - //if (kSRGB_SkColorProfileType == pt) { - // return kSRGBA_8888_GrPixelConfig; - //} - return kRGBA_8888_GrPixelConfig; + return (kSRGB_SkColorProfileType == pt) + ? kSRGBA_8888_GrPixelConfig + : kRGBA_8888_GrPixelConfig; case kBGRA_8888_SkColorType: - return kBGRA_8888_GrPixelConfig; + return (kSRGB_SkColorProfileType == pt) + ? kSRGBA_8888_GrPixelConfig // Does not preserve byte order! + : kBGRA_8888_GrPixelConfig; case kIndex_8_SkColorType: return kIndex_8_GrPixelConfig; case kGray_8_SkColorType: diff --git a/tests/TestConfigParsing.cpp b/tests/TestConfigParsing.cpp index f1f353e..e456c2e 100644 --- a/tests/TestConfigParsing.cpp +++ b/tests/TestConfigParsing.cpp @@ -43,6 +43,9 @@ DEF_TEST(ParseConfigs_Gpu, reporter) { REPORTER_ASSERT(reporter, configs[0]->asConfigGpu()->getUseNVPR() == false); REPORTER_ASSERT(reporter, configs[0]->asConfigGpu()->getUseDIText() == false); REPORTER_ASSERT(reporter, configs[0]->asConfigGpu()->getSamples() == 0); + REPORTER_ASSERT(reporter, configs[0]->asConfigGpu()->getColorType() == kN32_SkColorType); + REPORTER_ASSERT(reporter, configs[0]->asConfigGpu()->getProfileType() + == kLinear_SkColorProfileType); #endif } @@ -65,7 +68,8 @@ DEF_TEST(ParseConfigs_DefaultConfigs, reporter) { SkCommandLineFlags::StringArray config1 = make_string_array({ "565", "8888", "debug", "gpu", "gpudebug", "gpudft", "gpunull", "msaa16", "msaa4", "nonrendering", "null", "nullgpu", "nvprmsaa16", "nvprmsaa4", "pdf", "pdf_poppler", - "skp", "svg", "xps", "angle", "angle-gl", "commandbuffer", "mesa", "hwui" + "skp", "svg", "xps", "angle", "angle-gl", "commandbuffer", "mesa", "hwui", + "gpuf16", "gpusrgb", "anglesrgb" }); SkCommandLineConfigArray configs; @@ -101,11 +105,22 @@ DEF_TEST(ParseConfigs_DefaultConfigs, reporter) { REPORTER_ASSERT(reporter, !configs[17]->asConfigGpu()); REPORTER_ASSERT(reporter, !configs[18]->asConfigGpu()); REPORTER_ASSERT(reporter, !configs[23]->asConfigGpu()); + REPORTER_ASSERT(reporter, configs[24]->asConfigGpu()->getColorType() + == kRGBA_F16_SkColorType); + REPORTER_ASSERT(reporter, configs[24]->asConfigGpu()->getProfileType() + == kLinear_SkColorProfileType); + REPORTER_ASSERT(reporter, configs[25]->asConfigGpu()->getColorType() + == kN32_SkColorType); + REPORTER_ASSERT(reporter, configs[25]->asConfigGpu()->getProfileType() + == kSRGB_SkColorProfileType); #if SK_ANGLE #ifdef SK_BUILD_FOR_WIN REPORTER_ASSERT(reporter, configs[19]->asConfigGpu()); + REPORTER_ASSERT(reporter, configs[26]->asConfigGpu()->getProfileType() + == kSRGB_SkColorProfileType); #else REPORTER_ASSERT(reporter, !configs[19]->asConfigGpu()); + REPORTER_ASSERT(reporter, !configs[26]->asConfigGpu()); #endif REPORTER_ASSERT(reporter, configs[20]->asConfigGpu()); #else diff --git a/tools/flags/SkCommonFlagsConfig.cpp b/tools/flags/SkCommonFlagsConfig.cpp index c82e8de..611950c 100644 --- a/tools/flags/SkCommonFlagsConfig.cpp +++ b/tools/flags/SkCommonFlagsConfig.cpp @@ -23,11 +23,11 @@ static const char defaultConfigs[] = static const char configHelp[] = "Options: 565 8888 debug gpu gpudebug gpudft gpunull " - "msaa16 msaa4 nonrendering null nullgpu nvprmsaa16 nvprmsaa4 " + "msaa16 msaa4 gpuf16 gpusrgb nonrendering null nullgpu nvprmsaa16 nvprmsaa4 " "pdf pdf_poppler skp svg xps" #if SK_ANGLE #ifdef SK_BUILD_FOR_WIN - " angle" + " angle anglesrgb" #endif " angle-gl" #endif @@ -47,7 +47,7 @@ static const char configExtendedHelp[] = "Possible backends and options:\n" #if SK_SUPPORT_GPU "\n" - "gpu(api=string,dit=bool,nvpr=bool,samples=int)\tGPU backend\n" + "gpu(api=string,color=string,dit=bool,nvpr=bool,samples=int)\tGPU backend\n" "\tapi\ttype: string\tdefault: native.\n" "\t Select graphics API to use with gpu backend.\n" "\t Options:\n" @@ -69,6 +69,12 @@ static const char configExtendedHelp[] = #if SK_MESA "\t\tmesa\t\t\tUse MESA.\n" #endif + "\tcolor\ttype: string\tdefault: 8888.\n" + "\t Select framebuffer color format.\n" + "\t Options:\n" + "\t\t8888\t\t\tLinear 8888.\n" + "\t\tf16 \t\t\tLinear 16-bit floating point.\n" + "\t\tsrgb\t\t\tsRGB 8888.\n" "\tdit\ttype: bool\tdefault: false.\n" "\t Use device independent text.\n" "\tnvpr\ttype: bool\tdefault: false.\n" @@ -82,6 +88,8 @@ static const char configExtendedHelp[] = "\tmsaa16 \t= gpu(samples=16)\n" "\tnvprmsaa4\t= gpu(nvpr=true,samples=4)\n" "\tnvprmsaa16\t= gpu(nvpr=true,samples=16)\n" + "\tgpuf16 \t= gpu(color=f16)\n" + "\tgpusrgb \t= gpu(color=srgb)\n" "\tgpudft \t= gpu(dit=true)\n" "\tgpudebug \t= gpu(api=debug)\n" "\tgpunull \t= gpu(api=null)\n" @@ -90,6 +98,7 @@ static const char configExtendedHelp[] = #if SK_ANGLE #ifdef SK_BUILD_FOR_WIN "\tangle \t= gpu(api=angle)\n" + "\tanglesrgb \t= gpu(api=angle,color=srgb)\n" #endif "\tangle-gl \t= gpu(api=angle-gl)\n" #endif @@ -115,6 +124,8 @@ static const struct { { "msaa16", "gpu", "samples=16" }, { "nvprmsaa4", "gpu", "nvpr=true,samples=4,dit=true" }, { "nvprmsaa16", "gpu", "nvpr=true,samples=16,dit=true" }, + { "gpuf16", "gpu", "color=f16" }, + { "gpusrgb", "gpu", "color=srgb" }, { "gpudft", "gpu", "dit=true" }, { "gpudebug", "gpu", "api=debug" }, { "gpunull", "gpu", "api=null" }, @@ -123,6 +134,7 @@ static const struct { #if SK_ANGLE #ifdef SK_BUILD_FOR_WIN , { "angle", "gpu", "api=angle" } + , { "anglesrgb", "gpu", "api=angle,color=srgb" } #endif , { "angle-gl", "gpu", "api=angle-gl" } #endif @@ -149,12 +161,15 @@ SkCommandLineConfig::~SkCommandLineConfig() { #if SK_SUPPORT_GPU SkCommandLineConfigGpu::SkCommandLineConfigGpu( const SkString& tag, const SkTArray& viaParts, - ContextType contextType, bool useNVPR, bool useDIText, int samples) + ContextType contextType, bool useNVPR, bool useDIText, int samples, + SkColorType colorType, SkColorProfileType profileType) : SkCommandLineConfig(tag, SkString("gpu"), viaParts) , fContextType(contextType) , fUseNVPR(useNVPR) , fUseDIText(useDIText) - , fSamples(samples) { + , fSamples(samples) + , fColorType(colorType) + , fProfileType(profileType) { } static bool parse_option_int(const SkString& value, int* outInt) { if (value.isEmpty()) { @@ -231,6 +246,26 @@ static bool parse_option_gpu_api(const SkString& value, #endif return false; } +static bool parse_option_gpu_color(const SkString& value, + SkColorType* outColorType, + SkColorProfileType* outProfileType) { + if (value.equals("8888")) { + *outColorType = kRGBA_8888_SkColorType; + *outProfileType = kLinear_SkColorProfileType; + return true; + } + if (value.equals("f16")) { + *outColorType = kRGBA_F16_SkColorType; + *outProfileType = kLinear_SkColorProfileType; + return true; + } + if (value.equals("srgb")) { + *outColorType = kRGBA_8888_SkColorType; + *outProfileType = kSRGB_SkColorProfileType; + return true; + } + return false; +} SkCommandLineConfigGpu* parse_command_line_config_gpu(const SkString& tag, const SkTArray& vias, @@ -244,6 +279,9 @@ SkCommandLineConfigGpu* parse_command_line_config_gpu(const SkString& tag, bool useDIText = false; bool seenSamples = false; int samples = 0; + bool seenColor = false; + SkColorType colorType = kRGBA_8888_SkColorType; + SkColorProfileType profileType = kLinear_SkColorProfileType; SkTArray optionParts; SkStrSplit(options.c_str(), ",", kStrict_SkStrSplitMode, &optionParts); @@ -268,12 +306,16 @@ SkCommandLineConfigGpu* parse_command_line_config_gpu(const SkString& tag, } else if (key.equals("samples") && !seenSamples) { valueOk = parse_option_int(value, &samples); seenSamples = true; + } else if (key.equals("color") && !seenColor) { + valueOk = parse_option_gpu_color(value, &colorType, &profileType); + seenColor = true; } if (!valueOk) { return nullptr; } } - return new SkCommandLineConfigGpu(tag, vias, contextType, useNVPR, useDIText, samples); + return new SkCommandLineConfigGpu(tag, vias, contextType, useNVPR, useDIText, samples, + colorType, profileType); } #endif diff --git a/tools/flags/SkCommonFlagsConfig.h b/tools/flags/SkCommonFlagsConfig.h index 423cf11..39f1f97 100644 --- a/tools/flags/SkCommonFlagsConfig.h +++ b/tools/flags/SkCommonFlagsConfig.h @@ -52,18 +52,23 @@ class SkCommandLineConfigGpu : public SkCommandLineConfig { public: typedef GrContextFactory::GLContextType ContextType; SkCommandLineConfigGpu(const SkString& tag, const SkTArray& viaParts, - ContextType contextType, bool useNVPR, bool useDIText, int samples); + ContextType contextType, bool useNVPR, bool useDIText, int samples, + SkColorType colorType, SkColorProfileType profileType); const SkCommandLineConfigGpu* asConfigGpu() const override { return this; } ContextType getContextType() const { return fContextType; } bool getUseNVPR() const { return fUseNVPR; } bool getUseDIText() const { return fUseDIText; } int getSamples() const { return fSamples; } + SkColorType getColorType() const { return fColorType; } + SkColorProfileType getProfileType() const { return fProfileType; } private: ContextType fContextType; bool fUseNVPR; bool fUseDIText; int fSamples; + SkColorType fColorType; + SkColorProfileType fProfileType; }; #endif -- 2.7.4