AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix end-of-pattern matching for Skia recording optimization.

Browse Source 
The recorder optimizer's pattern matcher was accepting command sequences
when it shouldn't have.

In the submitted case, and the pattern matcher was looking for:
	saveLayer, drawBitmap, restore
and in the rendering for the submitted case, the sequence of commands
were:
	saveLayer, drawBitmap, drawBitmap, restore

This sequence was improperly accepted by the matcher, and the optimizer
reduced the sequence to:
	drawBitmap, drawBitmap
where the opacity from the saveLayer paint argument was applied
to the first drawBitmap only.

The user-visible effect in Chrome was a flashing effect on an image
caused by incorrect (too-high) opacity.

The patch adds a Skia test to check for pixel colour values in
a similarly structured recording.  All other Skia tests pass.
Blink layout tests also pass with this change.

BUG=chromium:344987
R=robertphillips@google.com, reed@google.com, mtklein@google.com

Author: dneto@chromium.org

Review URL: https://codereview.chromium.org/430503004
This commit is contained in:
dneto 2014-07-30 15:42:22 -07:00 committed by Commit bot
parent 3d822c29d4
commit 3f22e8c44a
2 changed files with 48 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/core/SkPictureRecord.cpp
View File

@ -319,7 +319,6 @@ static bool match(SkWriter32* writer, uint32_t offset,
return false;
}
curOffset += curSize;
if (curOffset < writer->bytesWritten()) {
// Something else between the last command and the end of the stream
return false;

48
tests/PictureTest.cpp
View File

@ -1610,3 +1610,51 @@ DEF_TEST(Canvas_EmptyBitmap, r) {
test_draw_bitmaps(&canvas);
}
DEF_TEST(DontOptimizeSaveLayerDrawDrawRestore, reporter) {
// This test is from crbug.com/344987.
// The commands are:
// saveLayer with paint that modifies alpha
// drawBitmapRectToRect
// drawBitmapRectToRect
// restore
// The bug was that this structure was modified so that:
// - The saveLayer and restore were eliminated
// - The alpha was only applied to the first drawBitmapRectToRect
// This test draws blue and red squares inside a 50% transparent
// layer. Both colours should show up muted.
// When the bug is present, the red square (the second bitmap)
// shows upwith full opacity.
SkBitmap blueBM;
make_bm(&blueBM, 100, 100, SkColorSetARGB(255, 0, 0, 255), true);
SkBitmap redBM;
make_bm(&redBM, 100, 100, SkColorSetARGB(255, 255, 0, 0), true);
SkPaint semiTransparent;
semiTransparent.setAlpha(0x80);
SkPictureRecorder recorder;
SkCanvas* canvas = recorder.beginRecording(100, 100);
canvas->drawARGB(0, 0, 0, 0);
canvas->saveLayer(0, &semiTransparent);
canvas->drawBitmap(blueBM, 25, 25);
canvas->drawBitmap(redBM, 50, 50);
canvas->restore();
SkAutoTUnref<SkPicture> picture(recorder.endRecording());
// Now replay the picture back on another canvas
// and check a couple of its pixels.
SkBitmap replayBM;
make_bm(&replayBM, 100, 100, SK_ColorBLACK, false);
SkCanvas replayCanvas(replayBM);
picture->draw(&replayCanvas);
replayCanvas.flush();
// With the bug present, at (55, 55) we would get a fully opaque red
// intead of a dark red.
REPORTER_ASSERT(reporter, replayBM.getColor(30, 30) == 0xff000080);
REPORTER_ASSERT(reporter, replayBM.getColor(55, 55) == 0xff800000);
}