New helper for creating a color space from another, with linear gamma

New helper for creating a color space from another, with linear gamma

Going to need this in many places once we expliclty require F16 surfaces
to have linear gamma color spaces.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2270103002

Committed: https://skia.googlesource.com/skia/+/8aeec39f1fedfdd3508365ff0548cc3f876f6a00

[Brian Osman, 2016-08-23 17:51:46]

Description was changed from

==========
New helper for creating a color space from another, with linear gamma

Going to need this in many places once we expliclty require F16 surfaces
to have linear gamma color spaces.

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

to

==========
New helper for creating a color space from another, with linear gamma

Going to need this in many places once we expliclty require F16 surfaces
to have linear gamma color spaces.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2270103002
==========

[Brian Osman, 2016-08-23 17:52:31]

brianosman@google.com changed reviewers:
+ msarett@google.com, mtklein@google.com, reed@google.com

[Brian Osman, 2016-08-23 17:52:37]

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

[commit-bot: I haz the power, 2016-08-23 17:52:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msarett, 2016-08-23 18:01:12]

lgtm if the consensus is to always mark F16 buffers with linear color spaces

https://codereview.chromium.org/2270103002/diff/1/src/core/SkColorSpace.cpp
File src/core/SkColorSpace.cpp (right):

https://codereview.chromium.org/2270103002/diff/1/src/core/SkColorSpace.cpp#n...
src/core/SkColorSpace.cpp:159: return
SkColorSpace_Base::NewRGB(kLinear_GammaNamed, fToXYZD50);
Eventually we'll want to add once-ptrs for the common cases of this...

[commit-bot: I haz the power, 2016-08-23 18:18:09]

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

[commit-bot: I haz the power, 2016-08-23 18:18:10]

Dry run: This issue passed the CQ dry run.

[reed1, 2016-08-24 10:22:27]

Will we also want a "named" singleton for linear-srgb? 

lgtm

[mtklein, 2016-08-24 12:10:00]

On 2016/08/24 10:22:27, reed1 wrote:
> Will we also want a "named" singleton for linear-srgb? 

I imagine the canonical way to get at the linear-srgb singleton, if one exists
in the future, would be NewNamed(kSRGB_Named)->makeLinearGamma().

> lgtm

lgtm too

[mtklein, 2016-08-24 12:10:21]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2016-08-24 12:10:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-24 12:11:22]

Description was changed from

==========
New helper for creating a color space from another, with linear gamma

Going to need this in many places once we expliclty require F16 surfaces
to have linear gamma color spaces.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2270103002
==========

to

==========
New helper for creating a color space from another, with linear gamma

Going to need this in many places once we expliclty require F16 surfaces
to have linear gamma color spaces.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2270103002

Committed:
https://skia.googlesource.com/skia/+/8aeec39f1fedfdd3508365ff0548cc3f876f6a00
==========

[commit-bot: I haz the power, 2016-08-24 12:11:23]

Committed patchset #1 (id:1) as
https://skia.googlesource.com/skia/+/8aeec39f1fedfdd3508365ff0548cc3f876f6a00

[reed1, 2016-08-24 13:21:57]

On 2016/08/24 12:10:00, mtklein wrote:
> On 2016/08/24 10:22:27, reed1 wrote:
> > Will we also want a "named" singleton for linear-srgb? 
> 
> I imagine the canonical way to get at the linear-srgb singleton, if one exists
> in the future, would be NewNamed(kSRGB_Named)->makeLinearGamma().

Understood, but that is not as obviously going to return a singleton. I'm sure
we can wait and see how common the need is.

> 
> > lgtm
> 
> lgtm too

[mtklein, 2016-08-24 14:00:29]

> Understood, but that is not as obviously going to return a singleton. I'm sure
> we can wait and see how common the need is.

Is it essential that these obviously return singletons?  I thought the
singletons were just there to save memory behind the scenes.

My preference for making this existing roundabout version the canonical way to
get at linear sRGB is just to make sure that, if we do have a singleton, all
paths lead to it.  When there's only one path that's guaranteed.  I'd worry
about getting tripped up between the potentially different pointers returned by
MakeLinearSRGB() and exisiting_colorspace->makeLinearGamma() when
existing_colorspace has sRGB gamut.

Wouldn't see any problem with a friendly inline wrapper around
NewNamed(kSRGB_Named)->makeLinearGamma() of course.

[reed1, 2016-08-24 19:33:01]

Hmmm. Its a small matter, but I can't see how offering kSRGB_Linear_Named as a
single factory option would make it hard to be consistent with the two-step
version.

[Brian Osman, 2016-08-25 16:54:49]

On 2016/08/24 19:33:01, reed1 wrote:
> Hmmm. Its a small matter, but I can't see how offering kSRGB_Linear_Named as a
> single factory option would make it hard to be consistent with the two-step
> version.

I had an idea of doing something like this, if people agree:

SkColorSpace_Base {
+ sk_sp<SkColorSpace> fLinear;
}

SkColorSpace::makeLinearGamma() {
  if (this->fNamedGamma == Linear) {
    return this;
  }
  if (!this->fLinear) {
    fLinear = NewRGB(kLinear, fToXYZD50);
  }
  return fLinear;
}

It bloats all color spaces by another pointer, but basically gives us single
instances of the linear versions of anything that's already a singleton.