Unit test to demonstrate Galaxy S6 MSAA readPixels bug

After lots of tinkering, this is the smallest test case I've
found to reproduce the bug. Frequently, bitmap b4 will be
cleared to blue, but not contain the oval drawn right before.
Bitmap b5 will contain the oval, so the diff of the two
bitmaps will detect the error.

There may be something else that can be removed, but I
haven't found it.

Interesting note:

In the original version, all surfaces were cleared to
black. When I started trying to change the clear colors to
figure out if we were getting an old surface, the bug went
away. In particular, the first clear color is irrelevant,
but the last three clears must all be the same color. If
any are different, the bug doesn't occur.

Bug: skia:6653
Change-Id: Iacafcc140b60594fab208e82987b0f37416975f3
Reviewed-on: https://skia-review.googlesource.com/19817
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>

4 files changed, 99 insertions, 1 deletions

diff --git a/gn/tests.gni b/gn/tests.gni
index 1b438f44ce..f961c84002 100644
--- a/gn/tests.gni
+++ b/gn/tests.gni
@@ -203,6 +203,7 @@ tests_sources = [
   "$_tests/SkBlend_optsTest.cpp",
   "$_tests/skbug5221.cpp",
   "$_tests/skbug6389.cpp",
+  "$_tests/skbug6653.cpp",
   "$_tests/SkColor4fTest.cpp",
   "$_tests/SkDOMTest.cpp",
   "$_tests/SkFixed15Test.cpp",
diff --git a/infra/bots/recipes/test.expected/Test-Android-Clang-GalaxyS6-GPU-MaliT760-arm64-Debug-Android.json b/infra/bots/recipes/test.expected/Test-Android-Clang-GalaxyS6-GPU-MaliT760-arm64-Debug-Android.json
index 1c87c68413..dae3e335d5 100644
--- a/infra/bots/recipes/test.expected/Test-Android-Clang-GalaxyS6-GPU-MaliT760-arm64-Debug-Android.json
+++ b/infra/bots/recipes/test.expected/Test-Android-Clang-GalaxyS6-GPU-MaliT760-arm64-Debug-Android.json
@@ -702,7 +702,7 @@
       "python",
       "-u",
       "\nimport shutil\nimport sys\nshutil.copy(sys.argv[1], sys.argv[2])\n",
-      "set -x; /data/local/tmp/dm --undefok --resourcePath /sdcard/revenge_of_the_skiabot/resources --skps /sdcard/revenge_of_the_skiabot/skps --images /sdcard/revenge_of_the_skiabot/images/dm --colorImages /sdcard/revenge_of_the_skiabot/images/colorspace --nameByHash --properties gitHash abc123 builder Test-Android-Clang-GalaxyS6-GPU-MaliT760-arm64-Debug-Android swarming_bot_id skia-bot-123 swarming_task_id 123456 --svgs /sdcard/revenge_of_the_skiabot/svgs --key arch arm64 compiler Clang configuration Debug cpu_or_gpu GPU cpu_or_gpu_value MaliT760 extra_config Android model GalaxyS6 os Android --uninterestingHashesFile /sdcard/revenge_of_the_skiabot/uninteresting_hashes.txt --writePath /sdcard/revenge_of_the_skiabot/dm_out --nocpu --randomProcessorTest --config 8888 srgb gles glesdft glessrgb glesmsaa4 serialize-8888 tiles_rt-8888 pic-8888 --src tests gm image colorImage svg --blacklist glessrgb image _ _ _ test _ GrShape serialize-8888 gm _ bleed_image serialize-8888 gm _ c_gms serialize-8888 gm _ colortype serialize-8888 gm _ colortype_xfermodes serialize-8888 gm _ drawfilter serialize-8888 gm _ fontmgr_bounds_0.75_0 serialize-8888 gm _ fontmgr_bounds_1_-0.25 serialize-8888 gm _ fontmgr_bounds serialize-8888 gm _ fontmgr_match serialize-8888 gm _ fontmgr_iter serialize-8888 gm _ imagemasksubset serialize-8888 gm _ bitmapfilters serialize-8888 gm _ bitmapshaders serialize-8888 gm _ bleed serialize-8888 gm _ bleed_alpha_bmp serialize-8888 gm _ bleed_alpha_bmp_shader serialize-8888 gm _ convex_poly_clip serialize-8888 gm _ extractalpha serialize-8888 gm _ filterbitmap_checkerboard_32_32_g8 serialize-8888 gm _ filterbitmap_image_mandrill_64 serialize-8888 gm _ shadows serialize-8888 gm _ simpleaaclip_aaclip serialize-8888 gm _ composeshader_bitmap serialize-8888 gm _ scaled_tilemodes_npot serialize-8888 gm _ scaled_tilemodes serialize-8888 gm _ typefacerendering_pfaMac serialize-8888 gm _ parsedpaths serialize-8888 gm _ ImageGeneratorExternal_rect serialize-8888 gm _ ImageGeneratorExternal_shader serialize-8888 gm _ shadow_utils serialize-8888 gm _ makecolorspace serialize-8888 gm _ bleed_alpha_image serialize-8888 gm _ bleed_alpha_image_shader serialize-8888 gm _ verylargebitmap serialize-8888 gm _ verylarge_picture_image pic-8888 gm _ drawfilter pic-8888 gm _ image-cacherator-from-picture serialize-8888 gm _ image-cacherator-from-picture pic-8888 gm _ image-cacherator-from-raster serialize-8888 gm _ image-cacherator-from-raster pic-8888 gm _ image-cacherator-from-ctable serialize-8888 gm _ image-cacherator-from-ctable pic-8888 gm _ gamut serialize-8888 gm _ gamut pic-8888 gm _ complexclip4_bw serialize-8888 gm _ complexclip4_bw pic-8888 gm _ complexclip4_aa serialize-8888 gm _ complexclip4_aa tiles_rt-8888 gm _ complexclip4_bw tiles_rt-8888 gm _ complexclip4_aa _ image _ interlaced1.png _ image _ interlaced2.png _ image _ interlaced3.png _ image _ .arw _ image _ .cr2 _ image _ .dng _ image _ .nef _ image _ .nrw _ image _ .orf _ image _ .raf _ image _ .rw2 _ image _ .pef _ image _ .srw _ image _ .ARW _ image _ .CR2 _ image _ .DNG _ image _ .NEF _ image _ .NRW _ image _ .ORF _ image _ .RAF _ image _ .RW2 _ image _ .PEF _ image _ .SRW --match ~SpecialImage; echo $? >/data/local/tmp/rc",
+      "set -x; /data/local/tmp/dm --undefok --resourcePath /sdcard/revenge_of_the_skiabot/resources --skps /sdcard/revenge_of_the_skiabot/skps --images /sdcard/revenge_of_the_skiabot/images/dm --colorImages /sdcard/revenge_of_the_skiabot/images/colorspace --nameByHash --properties gitHash abc123 builder Test-Android-Clang-GalaxyS6-GPU-MaliT760-arm64-Debug-Android swarming_bot_id skia-bot-123 swarming_task_id 123456 --svgs /sdcard/revenge_of_the_skiabot/svgs --key arch arm64 compiler Clang configuration Debug cpu_or_gpu GPU cpu_or_gpu_value MaliT760 extra_config Android model GalaxyS6 os Android --uninterestingHashesFile /sdcard/revenge_of_the_skiabot/uninteresting_hashes.txt --writePath /sdcard/revenge_of_the_skiabot/dm_out --nocpu --randomProcessorTest --config 8888 srgb gles glesdft glessrgb glesmsaa4 serialize-8888 tiles_rt-8888 pic-8888 --src tests gm image colorImage svg --blacklist glessrgb image _ _ _ test _ GrShape serialize-8888 gm _ bleed_image serialize-8888 gm _ c_gms serialize-8888 gm _ colortype serialize-8888 gm _ colortype_xfermodes serialize-8888 gm _ drawfilter serialize-8888 gm _ fontmgr_bounds_0.75_0 serialize-8888 gm _ fontmgr_bounds_1_-0.25 serialize-8888 gm _ fontmgr_bounds serialize-8888 gm _ fontmgr_match serialize-8888 gm _ fontmgr_iter serialize-8888 gm _ imagemasksubset serialize-8888 gm _ bitmapfilters serialize-8888 gm _ bitmapshaders serialize-8888 gm _ bleed serialize-8888 gm _ bleed_alpha_bmp serialize-8888 gm _ bleed_alpha_bmp_shader serialize-8888 gm _ convex_poly_clip serialize-8888 gm _ extractalpha serialize-8888 gm _ filterbitmap_checkerboard_32_32_g8 serialize-8888 gm _ filterbitmap_image_mandrill_64 serialize-8888 gm _ shadows serialize-8888 gm _ simpleaaclip_aaclip serialize-8888 gm _ composeshader_bitmap serialize-8888 gm _ scaled_tilemodes_npot serialize-8888 gm _ scaled_tilemodes serialize-8888 gm _ typefacerendering_pfaMac serialize-8888 gm _ parsedpaths serialize-8888 gm _ ImageGeneratorExternal_rect serialize-8888 gm _ ImageGeneratorExternal_shader serialize-8888 gm _ shadow_utils serialize-8888 gm _ makecolorspace serialize-8888 gm _ bleed_alpha_image serialize-8888 gm _ bleed_alpha_image_shader serialize-8888 gm _ verylargebitmap serialize-8888 gm _ verylarge_picture_image pic-8888 gm _ drawfilter pic-8888 gm _ image-cacherator-from-picture serialize-8888 gm _ image-cacherator-from-picture pic-8888 gm _ image-cacherator-from-raster serialize-8888 gm _ image-cacherator-from-raster pic-8888 gm _ image-cacherator-from-ctable serialize-8888 gm _ image-cacherator-from-ctable pic-8888 gm _ gamut serialize-8888 gm _ gamut pic-8888 gm _ complexclip4_bw serialize-8888 gm _ complexclip4_bw pic-8888 gm _ complexclip4_aa serialize-8888 gm _ complexclip4_aa tiles_rt-8888 gm _ complexclip4_bw tiles_rt-8888 gm _ complexclip4_aa _ image _ interlaced1.png _ image _ interlaced2.png _ image _ interlaced3.png _ image _ .arw _ image _ .cr2 _ image _ .dng _ image _ .nef _ image _ .nrw _ image _ .orf _ image _ .raf _ image _ .rw2 _ image _ .pef _ image _ .srw _ image _ .ARW _ image _ .CR2 _ image _ .DNG _ image _ .NEF _ image _ .NRW _ image _ .ORF _ image _ .RAF _ image _ .RW2 _ image _ .PEF _ image _ .SRW --match ~SpecialImage ~skbug6653; echo $? >/data/local/tmp/rc",
       "[START_DIR]/tmp/dm.sh"
     ],
     "env": {
diff --git a/infra/bots/recipes/test.py b/infra/bots/recipes/test.py
index 3e96572212..65b194d2d8 100644
--- a/infra/bots/recipes/test.py
+++ b/infra/bots/recipes/test.py
@@ -405,6 +405,7 @@ def dm_flags(bot):
 
   if 'GalaxyS6' in bot:
     match.append('~SpecialImage') # skia:6338
+    match.append('~skbug6653') # skia:6653
 
   if 'GalaxyS7_G930A' in bot:
     match.append('~WritePixels') # skia:6427
diff --git a/tests/skbug6653.cpp b/tests/skbug6653.cpp
new file mode 100644
index 0000000000..eaf166c3e3
--- /dev/null
+++ b/tests/skbug6653.cpp
@@ -0,0 +1,96 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkTypes.h"
+
+#include "SkCanvas.h"
+#include "SkSurface.h"
+
+#if SK_SUPPORT_GPU
+
+#include "GrContext.h"
+#include "GrTest.h"
+#include "Test.h"
+
+static SkBitmap read_pixels(sk_sp<SkSurface> surface) {
+    SkBitmap bmp;
+    bmp.allocN32Pixels(surface->width(), surface->height());
+    if (!surface->getCanvas()->readPixels(bmp, 0, 0)) {
+        SkDebugf("readPixels failed\n");
+    }
+    return bmp;
+}
+
+static sk_sp<SkSurface> make_surface(GrContext* context) {
+    SkImageInfo info = SkImageInfo::Make(50, 50, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
+    return SkSurface::MakeRenderTarget(context, SkBudgeted::kNo, info, 4,
+                                       kBottomLeft_GrSurfaceOrigin, nullptr);
+}
+
+// Tests that readPixels returns up-to-date results. Demonstrates a bug on Galaxy S6
+// (Mali T760), in MSAA mode.
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(skbug6653, reporter, ctxInfo) {
+    GrContext* ctx = ctxInfo.grContext();
+    SkRect rect = SkRect::MakeWH(50, 50);
+
+    SkPaint paint;
+    paint.setColor(SK_ColorWHITE);
+    paint.setStrokeWidth(5);
+    paint.setStyle(SkPaint::kStroke_Style);
+
+    // The one device that fails this test (Galaxy S6) does so in a flaky fashion. Trying many
+    // times makes it more likely to fail. Also, interacting with the phone (eg swiping between
+    // different home screens) while the test is running makes it fail close to 100%.
+    static const int kNumIterations = 50;
+
+    for (int i = 0; i < kNumIterations; ++i) {
+        auto s0 = make_surface(ctx);
+
+        auto s1 = make_surface(ctx);
+        s1->getCanvas()->clear(SK_ColorBLACK);
+        s1->getCanvas()->drawOval(rect, paint);
+        SkBitmap b1 = read_pixels(s1);
+        s1 = nullptr;
+
+        // The bug requires that all three of the following surfaces are cleared to the same color
+        auto s2 = make_surface(ctx);
+        s2->getCanvas()->clear(SK_ColorBLUE);
+        SkBitmap b2 = read_pixels(s2);
+        s2 = nullptr;
+
+        auto s3 = make_surface(ctx);
+        s3->getCanvas()->clear(SK_ColorBLUE);
+        SkBitmap b3 = read_pixels(s3);
+        s0->getCanvas()->drawBitmap(b3, 0, 0);
+        s3 = nullptr;
+
+        auto s4 = make_surface(ctx);
+        s4->getCanvas()->clear(SK_ColorBLUE);
+        s4->getCanvas()->drawOval(rect, paint);
+
+        // When this fails, b4 will "succeed", but return an empty bitmap (containing just the
+        // clear color). Regardless, b5 will contain the oval that was just drawn, so diffing the
+        // two bitmaps tests for the failure case.
+        SkBitmap b4 = read_pixels(s4);
+        SkBitmap b5 = read_pixels(s4);
+
+        bool match = true;
+        for (int y = 0; y < b4.height() && match; ++y) {
+            for (int x = 0; x < b4.width() && match; ++x) {
+                uint32_t pixelA = *b4.getAddr32(x, y);
+                uint32_t pixelB = *b5.getAddr32(x, y);
+                if (pixelA != pixelB) {
+                    match = false;
+                }
+            }
+        }
+
+        REPORTER_ASSERT(reporter, match);
+    }
+}
+
+#endif