From e9c8442bfc3a44b00cd64529882d7a8bd4bc6030 Mon Sep 17 00:00:00 2001 From: brianosman Date: Mon, 29 Feb 2016 13:55:40 -0800 Subject: [PATCH] =?utf8?q?Revert=20of=20Progress=20on=20gamma-correctness?= =?utf8?q?=20in=20the=20GPU=20backend.=20Fixed=20conversion=20of=20color?= =?utf8?q?=20and=20profile=20type=20to=20pix=E2=80=A6=20(patchset=20#3=20i?= =?utf8?q?d:40001=20of=20https://codereview.chromium.org/1746253002/=20)?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Reason for revert: Fixing the build. Original issue's description: > 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 > > Committed: https://skia.googlesource.com/skia/+/eef980270d3385fee340eb1633962fe3ba8b7132 TBR=mtklein@google.com,egdaniel@google.com,bsalomon@google.com # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=skia: Review URL: https://codereview.chromium.org/1748823002 --- 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, 16 insertions(+), 88 deletions(-) diff --git a/dm/DM.cpp b/dm/DM.cpp index f5e4682..4279ea3 100644 --- a/dm/DM.cpp +++ b/dm/DM.cpp @@ -725,8 +725,7 @@ static Sink* create_sink(const SkCommandLineConfig* config) { return nullptr; } return new GPUSink(contextType, contextOptions, gpuConfig->getSamples(), - gpuConfig->getUseDIText(), gpuConfig->getColorType(), - gpuConfig->getProfileType(), FLAGS_gpu_threading); + gpuConfig->getUseDIText(), FLAGS_gpu_threading); } } #endif diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp index c1db178..2d2a455 100644 --- a/dm/DMSrcSink.cpp +++ b/dm/DMSrcSink.cpp @@ -856,15 +856,11 @@ 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*) {} @@ -886,8 +882,7 @@ 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(), fColorType, - kPremul_SkAlphaType, fProfileType); + SkImageInfo::Make(size.width(), size.height(), kN32_SkColorType, kPremul_SkAlphaType); #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 babe14d..bbf47cf 100644 --- a/dm/DMSrcSink.h +++ b/dm/DMSrcSink.h @@ -213,8 +213,7 @@ public: class GPUSink : public Sink { public: GPUSink(GrContextFactory::GLContextType, GrContextFactory::GLContextOptions, - int samples, bool diText, SkColorType colorType, SkColorProfileType profileType, - bool threaded); + int samples, bool diText, bool threaded); Error draw(const Src&, SkBitmap*, SkWStream*, SkString*) const override; bool serial() const override { return !fThreaded; } @@ -225,8 +224,6 @@ 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 4106d49..a3848c7 100644 --- a/src/gpu/SkGr.cpp +++ b/src/gpu/SkGr.cpp @@ -370,13 +370,12 @@ GrPixelConfig SkImageInfo2GrPixelConfig(SkColorType ct, SkAlphaType, SkColorProf case kARGB_4444_SkColorType: return kRGBA_4444_GrPixelConfig; case kRGBA_8888_SkColorType: - return (kSRGB_SkColorProfileType == pt) - ? kSRGBA_8888_GrPixelConfig - : kRGBA_8888_GrPixelConfig; + //if (kSRGB_SkColorProfileType == pt) { + // return kSRGBA_8888_GrPixelConfig; + //} + return kRGBA_8888_GrPixelConfig; case kBGRA_8888_SkColorType: - return (kSRGB_SkColorProfileType == pt) - ? kSRGBA_8888_GrPixelConfig // Does not preserve byte order! - : kBGRA_8888_GrPixelConfig; + return 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 e456c2e..f1f353e 100644 --- a/tests/TestConfigParsing.cpp +++ b/tests/TestConfigParsing.cpp @@ -43,9 +43,6 @@ 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 } @@ -68,8 +65,7 @@ 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", - "gpuf16", "gpusrgb", "anglesrgb" + "skp", "svg", "xps", "angle", "angle-gl", "commandbuffer", "mesa", "hwui" }); SkCommandLineConfigArray configs; @@ -105,22 +101,11 @@ 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 611950c..c82e8de 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 gpuf16 gpusrgb nonrendering null nullgpu nvprmsaa16 nvprmsaa4 " + "msaa16 msaa4 nonrendering null nullgpu nvprmsaa16 nvprmsaa4 " "pdf pdf_poppler skp svg xps" #if SK_ANGLE #ifdef SK_BUILD_FOR_WIN - " angle anglesrgb" + " angle" #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,color=string,dit=bool,nvpr=bool,samples=int)\tGPU backend\n" + "gpu(api=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,12 +69,6 @@ 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" @@ -88,8 +82,6 @@ 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" @@ -98,7 +90,6 @@ 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 @@ -124,8 +115,6 @@ 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" }, @@ -134,7 +123,6 @@ 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 @@ -161,15 +149,12 @@ SkCommandLineConfig::~SkCommandLineConfig() { #if SK_SUPPORT_GPU SkCommandLineConfigGpu::SkCommandLineConfigGpu( const SkString& tag, const SkTArray& viaParts, - ContextType contextType, bool useNVPR, bool useDIText, int samples, - SkColorType colorType, SkColorProfileType profileType) + ContextType contextType, bool useNVPR, bool useDIText, int samples) : SkCommandLineConfig(tag, SkString("gpu"), viaParts) , fContextType(contextType) , fUseNVPR(useNVPR) , fUseDIText(useDIText) - , fSamples(samples) - , fColorType(colorType) - , fProfileType(profileType) { + , fSamples(samples) { } static bool parse_option_int(const SkString& value, int* outInt) { if (value.isEmpty()) { @@ -246,26 +231,6 @@ 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, @@ -279,9 +244,6 @@ 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); @@ -306,16 +268,12 @@ 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, - colorType, profileType); + return new SkCommandLineConfigGpu(tag, vias, contextType, useNVPR, useDIText, samples); } #endif diff --git a/tools/flags/SkCommonFlagsConfig.h b/tools/flags/SkCommonFlagsConfig.h index 39f1f97..423cf11 100644 --- a/tools/flags/SkCommonFlagsConfig.h +++ b/tools/flags/SkCommonFlagsConfig.h @@ -52,23 +52,18 @@ class SkCommandLineConfigGpu : public SkCommandLineConfig { public: typedef GrContextFactory::GLContextType ContextType; SkCommandLineConfigGpu(const SkString& tag, const SkTArray& viaParts, - ContextType contextType, bool useNVPR, bool useDIText, int samples, - SkColorType colorType, SkColorProfileType profileType); + ContextType contextType, bool useNVPR, bool useDIText, int samples); 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