Revert of Progress on gamma-correctness in the GPU backend. Fixed conversion of color...
authorbrianosman <brianosman@google.com>
Tue, 1 Mar 2016 21:44:28 +0000 (13:44 -0800)
committerCommit bot <commit-bot@chromium.org>
Tue, 1 Mar 2016 21:44:28 +0000 (13:44 -0800)
Reason for revert:
GM breakage. Changes to SkGr.cpp appear to be altering behavior on a variety of tests. Debugging...

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=1750383002
>
> Committed: https://skia.googlesource.com/skia/+/a6f58194733c1c50e4fe5f98585e42344f29b6f0

TBR=mtklein@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/1755553003

dm/DM.cpp
dm/DMSrcSink.cpp
dm/DMSrcSink.h
src/gpu/SkGr.cpp
tests/TestConfigParsing.cpp
tools/flags/SkCommonFlagsConfig.cpp
tools/flags/SkCommonFlagsConfig.h

index 0bd5b79..0190262 100644 (file)
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -739,8 +739,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
index c1db178..2d2a455 100644 (file)
@@ -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();
index babe14d..bbf47cf 100644 (file)
@@ -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;
 };
 
index 4106d49..a3848c7 100644 (file)
@@ -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:
index e456c2e..f1f353e 100644 (file)
@@ -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
index 983a501..c82e8de 100644 (file)
@@ -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<SkString>& 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 = kN32_SkColorType;
-        *outProfileType = kLinear_SkColorProfileType;
-        return true;
-    }
-    if (value.equals("f16")) {
-        *outColorType = kRGBA_F16_SkColorType;
-        *outProfileType = kLinear_SkColorProfileType;
-        return true;
-    }
-    if (value.equals("srgb")) {
-        *outColorType = kN32_SkColorType;
-        *outProfileType = kSRGB_SkColorProfileType;
-        return true;
-    }
-    return false;
-}
 
 SkCommandLineConfigGpu* parse_command_line_config_gpu(const SkString& tag,
                                                       const SkTArray<SkString>& 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 = kN32_SkColorType;
-    SkColorProfileType profileType = kLinear_SkColorProfileType;
 
     SkTArray<SkString> 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
 
index 39f1f97..423cf11 100644 (file)
@@ -52,23 +52,18 @@ class SkCommandLineConfigGpu : public SkCommandLineConfig {
   public:
     typedef GrContextFactory::GLContextType ContextType;
     SkCommandLineConfigGpu(const SkString& tag, const SkTArray<SkString>& 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