Always return ImageShader, even from SkShader::MakeBitmapShader
Lessons learned
1. ImageShader (correctly) always compresses (typically via PNG) during serialization. This has the surprise results of
- if the image was marked opaque, but has some non-opaque pixels (i.e. bug in blitter or caller), then compressing may "fix" those pixels, making the deserialized version draw differently. bug filed.
- 565 compressess/decompresses to 8888 (at least on Mac), which draws differently (esp. under some filters). bug filed.
2. BitmapShader did not enforce a copy for mutable bitmaps, but ImageShader does (since it creates an Image). Thus the former would see subsequent changes to the pixels after shader creation, while the latter does not, hence the change to the BlitRow test to avoid this modify-after-create pattern. I sure hope this prev. behavior was a bug/undefined-behavior, since this CL changes that.
BUG=skia:5595
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2195893002
Review-Url: https://codereview.chromium.org/2195893002
diff --git a/tests/ShaderTest.cpp b/tests/ShaderTest.cpp
index d3f74ff..8324047 100644
--- a/tests/ShaderTest.cpp
+++ b/tests/ShaderTest.cpp
@@ -15,8 +15,7 @@
static void check_isabitmap(skiatest::Reporter* reporter, SkShader* shader,
int expectedW, int expectedH,
SkShader::TileMode expectedX, SkShader::TileMode expectedY,
- const SkMatrix& expectedM,
- bool expectedImage) {
+ const SkMatrix& expectedM) {
SkBitmap bm;
SkShader::TileMode tileModes[2];
SkMatrix localM;
@@ -32,14 +31,12 @@
tileModes[0] = tileModes[1] = (SkShader::TileMode)99;
SkImage* image = shader->isAImage(&localM, tileModes);
- REPORTER_ASSERT(reporter, (image != nullptr) == expectedImage);
- if (image) {
- REPORTER_ASSERT(reporter, image->width() == expectedW);
- REPORTER_ASSERT(reporter, image->height() == expectedH);
- REPORTER_ASSERT(reporter, localM == expectedM);
- REPORTER_ASSERT(reporter, tileModes[0] == expectedX);
- REPORTER_ASSERT(reporter, tileModes[1] == expectedY);
- }
+ REPORTER_ASSERT(reporter, image);
+ REPORTER_ASSERT(reporter, image->width() == expectedW);
+ REPORTER_ASSERT(reporter, image->height() == expectedH);
+ REPORTER_ASSERT(reporter, localM == expectedM);
+ REPORTER_ASSERT(reporter, tileModes[0] == expectedX);
+ REPORTER_ASSERT(reporter, tileModes[1] == expectedY);
}
DEF_TEST(Shader_isABitmap, reporter) {
@@ -55,6 +52,6 @@
auto shader0 = SkShader::MakeBitmapShader(bm, tmx, tmy, &localM);
auto shader1 = SkImage::MakeFromBitmap(bm)->makeShader(tmx, tmy, &localM);
- check_isabitmap(reporter, shader0.get(), W, H, tmx, tmy, localM, false);
- check_isabitmap(reporter, shader1.get(), W, H, tmx, tmy, localM, true);
+ check_isabitmap(reporter, shader0.get(), W, H, tmx, tmy, localM);
+ check_isabitmap(reporter, shader1.get(), W, H, tmx, tmy, localM);
}