Remove unpremul/remul from SkArithmeticMode, both raster and GPU, since the SVG spec actually defin…

Disable the unpremultiply/remultiply in SkArithmeticMode, both raster and GPU, since the SVG spec actually defines arithmetic compositing to operate on premul colours directly.

The unpremul/remul code has been placed behind a gUseUnpremul global, which is default false. The non-unpremul path uses a min() to keep the colours to valid premult values.

Note:  This change will require new baselines for the arithmode and xfermodeimagefilter GMs.

R=reed@google.com

Committed: https://code.google.com/p/skia/source/detail?r=9555

[Stephen White, 2013-06-11 17:45:49]

PTAL

[reed1, 2013-06-11 17:48:57]

still trying to figure out why I went with unpremul in the first place. clearly
its more work, so I'm still thinking there was a good reason... I think I'd like
to keep it around for the moment.

[Stephen White, 2013-06-11 17:59:25]

On 2013/06/11 17:48:57, reed1 wrote:
> still trying to figure out why I went with unpremul in the first place.
clearly
> its more work, so I'm still thinking there was a good reason... 

Maybe this discussion here: 
http://lists.w3.org/Archives/Public/www-svg/2012Feb/0038.html
which never seemed to reach a satisfactory conclusion.

> I think I'd like to keep it around for the moment.

Sure. Would you prefer a runtime flag or compile-time (ifdef)?

[reed1, 2013-06-11 21:21:24]

On 2013/06/11 17:59:25, Stephen White wrote:
> On 2013/06/11 17:48:57, reed1 wrote:
> > still trying to figure out why I went with unpremul in the first place.
> clearly
> > its more work, so I'm still thinking there was a good reason... 
> 
> Maybe this discussion here: 
> http://lists.w3.org/Archives/Public/www-svg/2012Feb/0038.html
> which never seemed to reach a satisfactory conclusion.
> 
> > I think I'd like to keep it around for the moment.
> 
> Sure. Would you prefer a runtime flag or compile-time (ifdef)?

Either one is fine. Perhaps

if (false) {
...
}

Is best, since it ensures that the old code will still compile, but not make us
complicated the API with a flag (yet).

[Stephen White, 2013-06-11 21:39:05]

Here's a version with the unpremul code back, but behind a runtime flag.

[Stephen White, 2013-06-11 21:45:19]

On 2013/06/11 21:21:24, reed1 wrote:
> On 2013/06/11 17:59:25, Stephen White wrote:
> > On 2013/06/11 17:48:57, reed1 wrote:
> > > still trying to figure out why I went with unpremul in the first place.
> > clearly
> > > its more work, so I'm still thinking there was a good reason... 
> > 
> > Maybe this discussion here: 
> > http://lists.w3.org/Archives/Public/www-svg/2012Feb/0038.html
> > which never seemed to reach a satisfactory conclusion.
> > 
> > > I think I'd like to keep it around for the moment.
> > 
> > Sure. Would you prefer a runtime flag or compile-time (ifdef)?
> 
> Either one is fine. Perhaps
> 
> if (false) {
> ...
> }
> 
> Is best, since it ensures that the old code will still compile, but not make
us
> complicated the API with a flag (yet).

Ignore my last patch, then. :) (Didn't see your comment.)

[reed1, 2013-06-11 21:45:40]

https://codereview.chromium.org/16034013/diff/9001/include/effects/SkArithmet...
File include/effects/SkArithmeticMode.h (right):

https://codereview.chromium.org/16034013/diff/9001/include/effects/SkArithmet...
include/effects/SkArithmeticMode.h:29: SkScalar k3, SkScalar k4, bool
unpremultiply);
If we want a runtime param (which is fine with me), lets use an enum, so the
callsite can be more readable.

e.g.

enum Mode {
    kPremul_Mode,
    kUnpremul_Mode,
};

Or some other (better) name.

[Stephen White, 2013-06-11 21:59:51]

Here's a version with the unpremul code behind if (false).

[Stephen White, 2013-06-11 22:03:23]

On 2013/06/11 21:59:51, Stephen White wrote:
> Here's a version with the unpremul code behind if (false).

I kind of like it better this way, since there's less to go wrong.  (Standing
still now, and stopping the sidewalk dance. :) )

[reed1, 2013-06-12 13:19:39]

since there's more than one place where the (false) was needed, perhaps a
comment block above each saying something like "enable this to get unpremul
behavior (currently disabled)" or so. Alternatively, could use a const global to
get the same effect

static const bool gUseUnpremul = false;

if (gUseUnpremul) ...

otherwise, lgtm

[Stephen White, 2013-06-12 13:52:52]

On 2013/06/12 13:19:39, reed1 wrote:
> since there's more than one place where the (false) was needed, perhaps a
> comment block above each saying something like "enable this to get unpremul
> behavior (currently disabled)" or so. Alternatively, could use a const global
to
> get the same effect
> 
> static const bool gUseUnpremul = false;
> 
> if (gUseUnpremul) ...
> 
> otherwise, lgtm

Will fix on landing. Thanks for the review!

[Stephen White, 2013-06-12 22:15:46]

Committed patchset #4 manually as r9555 (presubmit successful).