Add sRGB mode toggle to Viewer.

Add sRGB mode toggle to Viewer.

Unlike SampleApp, this just switches out the format of the window
surface (and then adjusts the gamma-correct flag on the SkSurfaces).

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1950983007

Committed: https://skia.googlesource.com/skia/+/05de216ffb864cb1f3a4fa37a2c3a772be38a0c9

[Brian Osman, 2016-05-06 17:49:53]

Description was changed from

==========
Add sRGB mode toggle to Viewer.

Unlike SampleApp, this just switches out the format of the window
surface (and then adjusts the gamma-correct flag on the SkSurfaces).

BUG=skia:
==========

to

==========
Add sRGB mode toggle to Viewer.

Unlike SampleApp, this just switches out the format of the window
surface (and then adjusts the gamma-correct flag on the SkSurfaces).

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[Brian Osman, 2016-05-06 18:04:18]

The CQ bit was checked by brianosman@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-06 18:04:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1950983007/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1950983007/1

[Brian Osman, 2016-05-06 18:05:33]

brianosman@google.com changed reviewers:
+ bsalomon@google.com, jvanverth@google.com

[Brian Osman, 2016-05-06 18:05:33]

I wish this didn't have to touch so much code, but I need to plumb the request
all the way down... Oh well.

https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/android...
File tools/viewer/sk_app/android/Window_android.h (right):

https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/android...
tools/viewer/sk_app/android/Window_android.h:34: bool attach(BackEndType
attachType, int msaaSampleCount, bool srgb) override;
I posit that this code didn't compile before.

[commit-bot: I haz the power, 2016-05-06 18:27:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-06 18:27:33]

Dry run: This issue passed the CQ dry run.

[jvanverth1, 2016-05-06 18:27:55]

lgtm other than minor comment below. Btw, this will probably conflict with
https://codereview.chromium.org/1952323004/

https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/android...
File tools/viewer/sk_app/android/Window_android.h (right):

https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/android...
tools/viewer/sk_app/android/Window_android.h:34: bool attach(BackEndType
attachType, int msaaSampleCount, bool srgb) override;
On 2016/05/06 18:05:33, Brian Osman wrote:
> I posit that this code didn't compile before.

Probably not.

https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/android...
tools/viewer/sk_app/android/Window_android.h:48: bool mSRGB;
This should be fSRGB (despite what the other members say).

[liyuqian, 2016-05-06 18:29:37]

On 2016/05/06 18:27:55, jvanverth1 wrote:
> lgtm other than minor comment below. Btw, this will probably conflict with
> https://codereview.chromium.org/1952323004/
> 
>
https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/android...
> File tools/viewer/sk_app/android/Window_android.h (right):
> 
>
https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/android...
> tools/viewer/sk_app/android/Window_android.h:34: bool attach(BackEndType
> attachType, int msaaSampleCount, bool srgb) override;
> On 2016/05/06 18:05:33, Brian Osman wrote:
> > I posit that this code didn't compile before.
> 
> Probably not.
> 
>
https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/android...
> tools/viewer/sk_app/android/Window_android.h:48: bool mSRGB;
> This should be fSRGB (despite what the other members say).

Don't worry, I'll merge and fix the conflict.

[bsalomon, 2016-05-06 18:30:15]

https://codereview.chromium.org/1950983007/diff/1/tools/viewer/Viewer.cpp
File tools/viewer/Viewer.cpp (right):

https://codereview.chromium.org/1950983007/diff/1/tools/viewer/Viewer.cpp#new...
tools/viewer/Viewer.cpp:162: updateTitle();
this->

https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/VulkanW...
File tools/viewer/sk_app/VulkanWindowContext.h (right):

https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/VulkanW...
tools/viewer/sk_app/VulkanWindowContext.h:72: bool createSwapchain(uint32_t
width, uint32_t height, bool srgb);
Should we take a config here or something more general? Just trying to think
ahead for 10bit, ...

[bsalomon, 2016-05-06 18:31:24]

On 2016/05/06 18:30:15, bsalomon wrote:
> https://codereview.chromium.org/1950983007/diff/1/tools/viewer/Viewer.cpp
> File tools/viewer/Viewer.cpp (right):
> 
>
https://codereview.chromium.org/1950983007/diff/1/tools/viewer/Viewer.cpp#new...
> tools/viewer/Viewer.cpp:162: updateTitle();
> this->
> 
>
https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/VulkanW...
> File tools/viewer/sk_app/VulkanWindowContext.h (right):
> 
>
https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/VulkanW...
> tools/viewer/sk_app/VulkanWindowContext.h:72: bool createSwapchain(uint32_t
> width, uint32_t height, bool srgb);
> Should we take a config here or something more general? Just trying to think
> ahead for 10bit, ...

and same for getSRGB()/setSRGB() on Window

[Brian Osman, 2016-05-06 18:42:06]

Updated the easy stuff...

https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/VulkanW...
File tools/viewer/sk_app/VulkanWindowContext.h (right):

https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/VulkanW...
tools/viewer/sk_app/VulkanWindowContext.h:72: bool createSwapchain(uint32_t
width, uint32_t height, bool srgb);
On 2016/05/06 18:30:15, bsalomon wrote:
> Should we take a config here or something more general? Just trying to think
> ahead for 10bit, ...

Did you mean GrPixelConfig? That would invert some of the internal logic, and
doesn't give us any way to request 10-bit. The only option I came up with was to
make a params struct to pass along sRGB/DeepColor/etc... That would be cleaner,
and I guess we could still have the get() return a const& to the underlying
stored copy for the calling code to query?

[jvanverth1, 2016-05-06 18:47:08]

On 2016/05/06 18:42:06, Brian Osman wrote:
> Updated the easy stuff...
> 
>
https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/VulkanW...
> File tools/viewer/sk_app/VulkanWindowContext.h (right):
> 
>
https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/VulkanW...
> tools/viewer/sk_app/VulkanWindowContext.h:72: bool createSwapchain(uint32_t
> width, uint32_t height, bool srgb);
> On 2016/05/06 18:30:15, bsalomon wrote:
> > Should we take a config here or something more general? Just trying to think
> > ahead for 10bit, ...
> 
> Did you mean GrPixelConfig? That would invert some of the internal logic, and
> doesn't give us any way to request 10-bit. The only option I came up with was
to
> make a params struct to pass along sRGB/DeepColor/etc... That would be
cleaner,
> and I guess we could still have the get() return a const& to the underlying
> stored copy for the calling code to query?

How about an enum rather than a bool: enum { kLinear, kSRGB, k10Bit } or some
such?

[bsalomon, 2016-05-06 18:48:32]

https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/VulkanW...
File tools/viewer/sk_app/VulkanWindowContext.h (right):

https://codereview.chromium.org/1950983007/diff/1/tools/viewer/sk_app/VulkanW...
tools/viewer/sk_app/VulkanWindowContext.h:72: bool createSwapchain(uint32_t
width, uint32_t height, bool srgb);
On 2016/05/06 18:42:06, Brian Osman wrote:
> On 2016/05/06 18:30:15, bsalomon wrote:
> > Should we take a config here or something more general? Just trying to think
> > ahead for 10bit, ...
> 
> Did you mean GrPixelConfig? That would invert some of the internal logic, and
> doesn't give us any way to request 10-bit. The only option I came up with was
to
> make a params struct to pass along sRGB/DeepColor/etc... That would be
cleaner,
> and I guess we could still have the get() return a const& to the underlying
> stored copy for the calling code to query?

What about (SkColorType, SkProfileType)?

I think we're going to wind up with 10bit as a GrPixelConfig/SkColorType
eventually anyway even if for a while the only way to create one is to wrap a
external swap buffer.

[Brian Osman, 2016-05-06 20:01:14]

Switched to using SkColorType ad SkColorProfileType, plus I bundled those and
the MSAA sample count together. Should be easier to extend if we need
other/different properties in the future.

[bsalomon, 2016-05-06 20:03:37]

On 2016/05/06 20:01:14, Brian Osman wrote:
> Switched to using SkColorType ad SkColorProfileType, plus I bundled those and
> the MSAA sample count together. Should be easier to extend if we need
> other/different properties in the future.

I like it! lgtm

[Brian Osman, 2016-05-06 20:05:33]

The CQ bit was checked by brianosman@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-06 20:05:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1950983007/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1950983007/40001

[Brian Osman, 2016-05-06 20:20:05]

The CQ bit was unchecked by brianosman@google.com

[Brian Osman, 2016-05-06 20:20:07]

The CQ bit was checked by brianosman@google.com

[Brian Osman, 2016-05-06 20:20:07]

The patchset sent to the CQ was uploaded after l-g-t-m from jvanverth@google.com
Link to the patchset: https://codereview.chromium.org/1950983007/#ps40001
(title: "Bundle window surface parameters in struct, use
SkColorType/SkColorProfileType")

[commit-bot: I haz the power, 2016-05-06 20:20:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1950983007/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1950983007/40001

[commit-bot: I haz the power, 2016-05-06 20:28:59]

Description was changed from

==========
Add sRGB mode toggle to Viewer.

Unlike SampleApp, this just switches out the format of the window
surface (and then adjusts the gamma-correct flag on the SkSurfaces).

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Add sRGB mode toggle to Viewer.

Unlike SampleApp, this just switches out the format of the window
surface (and then adjusts the gamma-correct flag on the SkSurfaces).

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/05de216ffb864cb1f3a4fa37a2c3a772be38a0c9
==========

[commit-bot: I haz the power, 2016-05-06 20:29:00]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/05de216ffb864cb1f3a4fa37a2c3a772be38a0c9