I think the CL that cause this might be https://skia-review.googlesource.com/c/skia/+/87000 because it is the only one that touches on
SkFilterQuality.

I just tried it out on my local build and that confirmed my suspicion. But mtklein@ seems to have created a revert already and so these tests are no longer
failing.

Agreed.

Any idea of the spread of platforms those 4 tests were failing on?  I don't think we saw any failure like that when flipping this feature on here: https://chromium-review.googlesource.com/c/chromium/src/+/836807

I caught one in Mac (https://ci.chromium.org/buildbot/tryserver.chromium.mac/mac_chromium_rel_ng/616587) and can reproduce it in my Linux machine. So I guess it is an all-platform
issue.

Btw I'm trying to remove the suppression of these
4 tests in TestExpectations in https://chromium-review.googlesource.com/c/chromium/src/+/839968. They should
be all green after the revert now.

The reason that they are not failing anymore also might be the fact that I flagged them as flaky to release the bots. Regarding the spread of platforms, they were failing on Mac, Win7 and Linux. See  crbug.com/796760 .

Thanks Olivia for removing the suppression on the tests.

I did a little bit of investigation. My first impression was that the change may have result in a slight variance in color values of resized pixels, and this breaks the tests. However, the effect seems to be strange. For premul case, the resized pixels slightly change, which is expected, I assume. But for unpremul case, the results gets very different:

Source unpremul pixels (2 pixels):
255,0,0,255    255,0,0,26

Resized pixels before Skia CL (4 pixels):
255,0,0,255    255,0,0,200    255,0,0,81    255,0,0,21

Resized pixels after Skia CL:
255,0,0,255,   4,0,0,200,     5,0,0,81,     255,0,0,21

It is worthy to mention that unpremul resize code path in ImageBitmap is not allowed to premul the pixels. Due to the alpha clamping issue that we had when the resize quality was set to high, the code path is doing the resize on alpha channel and color channels separately and merges the result using SkImageFilters. Please see the code here:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.cpp?q=imagebitmap.cpp&sq=package:chromium&dr&l=376

Correction. SkImagFilter used only to separate alpha from color components. Also fwiw, when doing that, we set the alpha of the color components to 255:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.cpp?q=imagebitmap.cpp&sq=package:chromium&dr&l=381

I'm not sure I can picture in my head the change.  Are those a,r,g,b?

Are these unit tests?  If so what are they trying to assert?  Can they be re-written as things that can be looked at and rebaselined?

The Skia change in question should allow native high-quality scaling of unpremul bitmaps, without going through the split-filter-twice image filter strategy.

These are RGBA, sorry for not mentioning. I remove the branch and report the result here.

These are layout tests and check different combination of ImageBitmapOptions and resize (2x2 pixels -> 4x4 pixels). The failing configuration is when the user asks to not premul the unpremul source pixels in any operation, including resize.

Are you sure?  This CL _removed_ that [0,a] clamp.

The blink code still uses the old way. I'm testing to see what happens with the new code path. I'll report in a couple of minutes.

I removed the branch and merged the code path for "unpremul, high" with other
filter qualities. Unpremul "pixelated, low, medium" pass, but unpremul high still
fails. Fwiw, these are the pixels in RGBA:

Source:
255,0,0,255		255,0,0,26
0,255,0,255     0,255,0,26

Expected resized pixels:
255,0,0,255,    255,0,0,200,    255,0,0,81,     255,0,0,21,
198,63,0,255,   194,61,0,200,   195,63,0,81,    194,61,0,21,
63,198,0,255,   61,194,0,200,   63,195,0,81,    61,194,0,21,
0,255,0,255,    0,255,0,200,    0,255,0,81,     0,255,0,21

Generated result:
255,0,0,255,    4,0,0,200,      5,0,0,81,        255,0,0,21,
198,63,0,255,   194,61,0,200,   195,63,0,81,     194,61,0,21,
63,198,0,255,   61,194,0,200,   63,195,0,81,     61,194,0,21,
0,255,0,255,    0,4,0,200,      0,5,0,81,        0,255,0,21

In the merged code path we simply call SkImage->scalePixels:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.cpp?q=imagebitmap.cpp&sq=package:chromium&dr&l=344

So the "Expected resized pixels:" pixels are coming from the version that splits into two images (rgb1, 000a), upscales each image with a premul HQ scalePixels(), and remerges the results, and the "Generated result:" pixels are coming from a path that's just doing a single unpremul HQ scalePixels() call?

Yes. You also can see the expected pixels for unpremul low quality resize here:

https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fast/webgl/resources/tex-image-and-sub-image-image-bitmap-utils-resize.js?type=cs&q=/WebKit/LayoutTests/fast/webgl/resources/tex-image-and-sub-image-image-bitmap-utils-resize.js&sq=package:chromium&l=33

As you can see it is very close to the expected pixels for unpremul high quality resize. Unpremul low quality uses a single scalePixels() call.

If it helps you at all, I wrote a fiddle earlier to check the unpremul high quality filter:

https://fiddle.skia.org/c/285f4fb5b7f19041db4d222e8df41cba

If your change makes this fiddle work and generate similar results for high quality filter, then the problem should be somewhere in Blink.

What are you expecting to see in that fiddle?  It prints this out for me:

Color Space: Legacy, Unpremul
[   0   0 255 255 ][   0   0 255  26 ]
[   0 255   0 255 ][   0 255   0  26 ]
Color Space: Legacy, Premul
[   0   0 255 255 ][   0   0 255  26 ]
[   0 255   0 255 ][   0 255   0  26 ]
Color Space: Legacy, Unpremul, kNone_SkFilterQuality
[   0   0 255 255 ][   0   0 255 255 ][   0   0 255  26 ][   0   0 255  26 ]
[   0   0 255 255 ][   0   0 255 255 ][   0   0 255  26 ][   0   0 255  26 ]
[   0 255   0 255 ][   0 255   0 255 ][   0 255   0  26 ][   0 255   0  26 ]
[   0 255   0 255 ][   0 255   0 255 ][   0 255   0  26 ][   0 255   0  26 ]
Color Space: Legacy, Unpremul, kLow_SkFilterQuality
[   0   0 255 255 ][   0   0 255 197 ][   0   0 255  83 ][   0   0 255  26 ]
[   0  63 191 255 ][   0  63 191 197 ][   0  63 191  83 ][   0  63 191  26 ]
[   0 191  63 255 ][   0 191  63 197 ][   0 191  63  83 ][   0 191  63  26 ]
[   0 255   0 255 ][   0 255   0 197 ][   0 255   0  83 ][   0 255   0  26 ]
Color Space: Legacy, Unpremul, kMedium_SkFilterQuality
[   0   0 255 255 ][   0   0 255 197 ][   0   0 255  83 ][   0   0 255  26 ]
[   0  63 191 255 ][   0  63 191 197 ][   0  63 191  83 ][   0  63 191  26 ]
[   0 191  63 255 ][   0 191  63 197 ][   0 191  63  83 ][   0 191  63  26 ]
[   0 255   0 255 ][   0 255   0 197 ][   0 255   0  83 ][   0 255   0  26 ]
Color Space: Legacy, Unpremul, kHigh_SkFilterQuality
[   0   0 255 255 ][   0   0 255 200 ][   0   0 255  81 ][   0   0 255  21 ]
[   0  62 193 255 ][   0  62 193 200 ][   0  62 193  81 ][   0  62 193  21 ]
[   0 193  62 255 ][   0 193  62 200 ][   0 193  62  81 ][   0 193  62  21 ]
[   0 255   0 255 ][   0 255   0 200 ][   0 255   0  81 ][   0 255   0  21 ]

That's Skia built at https://skia.googlesource.com/skia/+/16c81a1ee9e2b90b97d73a1f08b03bebfadfbc81 yesterday, which includes active "bc9f349 remove bicubic clamp in SkImageShader".  That all looks about expected to me.

Interesting. Let me check the blink code again.

Okay, I can confirm that something is conflicting in the logic in ImageBitmap resize code (to be exact, when the source is premul and the flag in ImageBitmapOptions is set to unpremul). Please go ahead and land your CL. I'll keep the tests suppressed for now and will fix them after your CL was rolled.

Thanks!  We're rolling again now with the CL out (https://chromium-review.googlesource.com/c/chromium/src/+/839982), and then I'll re-land it in Skia and we'll roll again with that patch.

We can set a #define to disable this feature in Chromium if necessary, by reverting https://chromium-review.googlesource.com/c/chromium/src/+/836807.  That might be a more focused relief valve than reverting the whole roll, should the need come up again.  I'll make sure to leave support for that #define in until this is all sorted out.

Thanks everyone for investigating. Reza, assigning to you to follow up.

 Issue 796766  has been merged into this issue.

The CL to fix this is ready at https://chromium-review.googlesource.com/c/chromium/src/+/840829. I'll try to land after the Skia change rolled.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a52f6b77d5a4ac517e850d08005262d7d74c9d7b

commit a52f6b77d5a4ac517e850d08005262d7d74c9d7b
Author: Reza.Zakerinasab <zakerinasab@chromium.org>
Date: Fri Dec 22 18:07:00 2017

Unify ImageBitmap resize code path for premul and unpremul

After Skia rolls this recent CL to fix the alpha clamping issue in high filter
quality (https://skia-review.googlesource.com/c/skia/+/87000),
we can refactor ImageBitmap code path to unify high quality premul and unpremul.

Bug:  787034 ,  796797 
Change-Id: I2efc0c86b15b2f182e4af99afc538918d11f2f0d
Reviewed-on: https://chromium-review.googlesource.com/840829
Reviewed-by: Olivia Lai <xlai@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526021}
[modify] https://crrev.com/a52f6b77d5a4ac517e850d08005262d7d74c9d7b/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/a52f6b77d5a4ac517e850d08005262d7d74c9d7b/third_party/WebKit/LayoutTests/fast/webgl/resources/tex-image-and-sub-image-image-bitmap-utils-resize.js
[modify] https://crrev.com/a52f6b77d5a4ac517e850d08005262d7d74c9d7b/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.cpp