From 320a40d7733979703bdf675c31108255e011e34e Mon Sep 17 00:00:00 2001 From: reed Date: Tue, 2 Aug 2016 06:12:06 -0700 Subject: 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 --- tests/ShaderTest.cpp | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'tests/ShaderTest.cpp') diff --git a/tests/ShaderTest.cpp b/tests/ShaderTest.cpp index d3f74ffb3d..83240478ad 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 @@ static void check_isabitmap(skiatest::Reporter* reporter, SkShader* shader, 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 @@ DEF_TEST(Shader_isABitmap, reporter) { 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); } -- cgit v1.2.3