Remove SK_SUPPORT_LEGACY_COLOR32_MATH

Remove SK_SUPPORT_LEGACY_COLOR32_MATH

BUG=480013
TBR=mtklein@google.com,reed@google.com

Committed: https://crrev.com/5d77a2efd07ddf8070bafd95fad908f7a109ead1
Cr-Commit-Position: refs/heads/master@{#326986}

[f(malita), 2015-04-19 01:50:44]

Let's see what we got here :)

[f(malita), 2015-04-19 03:54:51]

On 2015/04/19 01:50:44, f(malita) wrote:
> Let's see what we got here :)

1803 layout failures - that's too bad, I expected much worse!

[mtklein, 2015-04-21 22:27:05]

So, how do we do this?

[mtklein, 2015-04-21 22:27:15]

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

[commit-bot: I haz the power, 2015-04-21 22:28:34]

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

[f(malita), 2015-04-21 23:10:33]

On 2015/04/21 22:27:05, mtklein wrote:
> So, how do we do this?

In a state of uncomfortable excitement, during a quiet/closed tree period :)

I signed up to handle the rebaselines, but I'll be out of town this weekend - so
hopefully the weekend after.

The pixel diff rebaselines are straightforward, although I see a few other
things that will have to be addressed first:

* cc_unittests failures:
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...

If we're lucky, we can just rebaseline their checked-in reference pngs. If not
(divergent GPU vs. sw results), we may need to tweak their fuzzy comparator.

* Blink exact-pixel-val failures:
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...

Failed assertion: got pixel [0,255,0,254] at (79,35), expected [0,255,0,255]

This may be a bug - someone needs to look closely and verify - then possibly
tweak the test.

* one Blink ref-test failure:
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...

Again, gotta figure why this is different now and possibly tweak it.


So we need to figure these out before dealing with the main rebaseline. Mike
also suggested that we should try to address these by making them more
flexible/resilient, such that we don't have to spend cycles on them for the next
big rebaseline.

[chromium-reviews, 2015-04-21 23:19:48]

Doesn't this one just mean we fixed a bug?  The expectation used to be the
assertion failed, and now it passes?

https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...

On Tue, Apr 21, 2015 at 7:10 PM <fmalita@chromium.org> wrote:

> On 2015/04/21 22:27:05, mtklein wrote:
> > So, how do we do this?
>
> In a state of uncomfortable excitement, during a quiet/closed tree period
> :)
>
> I signed up to handle the rebaselines, but I'll be out of town this weekend
> - so
> hopefully the weekend after.
>
> The pixel diff rebaselines are straightforward, although I see a few other
> things that will have to be addressed first:
>
> * cc_unittests failures:
>
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...
>
> If we're lucky, we can just rebaseline their checked-in reference pngs. If
> not
> (divergent GPU vs. sw results), we may need to tweak their fuzzy
> comparator.
>
> * Blink exact-pixel-val failures:
>
>
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...
>
> Failed assertion: got pixel [0,255,0,254] at (79,35), expected
> [0,255,0,255]
>
> This may be a bug - someone needs to look closely and verify - then
> possibly
> tweak the test.
>
> * one Blink ref-test failure:
>
>
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...
>
> Again, gotta figure why this is different now and possibly tweak it.
>
>
> So we need to figure these out before dealing with the main rebaseline.
> Mike
> also suggested that we should try to address these by making them more
> flexible/resilient, such that we don't have to spend cycles on them for the
> next
> big rebaseline.
>
> https://codereview.chromium.org/1095513004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[f(malita), 2015-04-21 23:27:16]

On 2015/04/21 23:19:48, chromium-reviews wrote:
> Doesn't this one just mean we fixed a bug?  The expectation used to be the
> assertion failed, and now it passes?
> 
>
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...

Doh, it sure does - w00t! It also means that someone checked in a bad baseline
for that test and I'm not paying attention to the left vs. right pretty diff
panels :)

[f(malita), 2015-04-22 00:35:58]

Updated the cc unit test to use a fuzzy comparator - it should pass now.

Looking at some pixel results I do notice something strange though: the new
blending yields some horizontal-only diffs (seems inconsistent for horizontal
vs. vertical lines).

For example:
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...

horizontal color: 0xff734c00

vertical color: 0xff744d01

This doesn't smell right - any idea what's going on?

[mtklein, 2015-04-22 12:59:26]

On 2015/04/22 00:35:58, f(malita) wrote:
> Updated the cc unit test to use a fuzzy comparator - it should pass now.
> 
> Looking at some pixel results I do notice something strange though: the new
> blending yields some horizontal-only diffs (seems inconsistent for horizontal
> vs. vertical lines).
> 
> For example:
>
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...
> 
> horizontal color: 0xff734c00
> 
> vertical color: 0xff744d01
> 
> This doesn't smell right - any idea what's going on?

I think this is because we haven't fixed all blending, only one particular
method that's used to blend a constant color over a dst with no AA.  That's why
that SVG upside-down S isn't fully fixed... more methods to fix.

Unfortunately this may be a slow process.  The blend math that we're using that
delivers this new level of correctness at no extra performance cost is a little
hard to package up into reusable components, or at least I haven't figured out
how to do so yet.  I'm hoping that as I start tackling the other bad blends,
I'll find ways to accelerate the fixes with more shared code.  Ultimately I'd
like to get to the point where there is no code in Skia that does bad blending.

[f(malita), 2015-04-22 22:53:09]

On 2015/04/22 12:59:26, mtklein wrote:
> On 2015/04/22 00:35:58, f(malita) wrote:
> > Updated the cc unit test to use a fuzzy comparator - it should pass now.
> > 
> > Looking at some pixel results I do notice something strange though: the new
> > blending yields some horizontal-only diffs (seems inconsistent for
horizontal
> > vs. vertical lines).
> > 
> > For example:
> >
>
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...
> > 
> > horizontal color: 0xff734c00
> > 
> > vertical color: 0xff744d01
> > 
> > This doesn't smell right - any idea what's going on?
> 
> I think this is because we haven't fixed all blending, only one particular
> method that's used to blend a constant color over a dst with no AA.  That's
why
> that SVG upside-down S isn't fully fixed... more methods to fix.
> 
> Unfortunately this may be a slow process.  The blend math that we're using
that
> delivers this new level of correctness at no extra performance cost is a
little
> hard to package up into reusable components, or at least I haven't figured out
> how to do so yet.  I'm hoping that as I start tackling the other bad blends,
> I'll find ways to accelerate the fixes with more shared code.  Ultimately I'd
> like to get to the point where there is no code in Skia that does bad
blending.

OK. I think this could make ref-tests less reliable, but probably not worth
delaying the rebasline over it.

[f(malita), 2015-04-26 15:52:15]

The CQ bit was checked by fmalita@chromium.org

[commit-bot: I haz the power, 2015-04-26 15:52:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1095513004/160001

[f(malita), 2015-04-26 15:59:25]

The CQ bit was unchecked by fmalita@chromium.org

[f(malita), 2015-04-26 17:42:50]

The CQ bit was checked by fmalita@chromium.org

[commit-bot: I haz the power, 2015-04-26 17:42:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1095513004/160001

[commit-bot: I haz the power, 2015-04-26 17:43:25]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-04-26 17:44:27]

Patchset 9 (id:??) landed as
https://crrev.com/5d77a2efd07ddf8070bafd95fad908f7a109ead1
Cr-Commit-Position: refs/heads/master@{#326986}