Clip vertical edges outside crop rect during convex edge walk
Bug: chromium:1239558 Change-Id: Ibc4655adaa72d1abf306940dd8b5e2f6a8b0edd9 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446923 Reviewed-by: Mike Reed <reed@google.com> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This commit is contained in:
parent
5da061a53b
commit
c7ffd5e680
@ -1218,23 +1218,30 @@ static void aaa_walk_convex_edges(SkAnalyticEdge* prevHead,
|
||||
fixed_to_alpha(SkFixedMul(partialBot, partialRite)));
|
||||
}
|
||||
}
|
||||
} else { // left and rite are within the same pixel
|
||||
if (partialTop > 0) {
|
||||
blitter->blitAntiH(fullLeft - 1,
|
||||
fullTop - 1,
|
||||
1,
|
||||
fixed_to_alpha(SkFixedMul(partialTop, rite - left)));
|
||||
blitter->flush_if_y_changed(y, y + partialTop);
|
||||
}
|
||||
if (fullBot > fullTop) {
|
||||
blitter->getRealBlitter()->blitV(
|
||||
fullLeft - 1, fullTop, fullBot - fullTop, fixed_to_alpha(rite - left));
|
||||
}
|
||||
if (partialBot > 0) {
|
||||
blitter->blitAntiH(fullLeft - 1,
|
||||
fullBot,
|
||||
1,
|
||||
fixed_to_alpha(SkFixedMul(partialBot, rite - left)));
|
||||
} else {
|
||||
// Normal conditions, this means left and rite are within the same pixel, but if
|
||||
// both left and rite were < leftBounds or > rightBounds, both edges are clipped and
|
||||
// we should not do any blitting (particularly since the negative width saturates to
|
||||
// full alpha).
|
||||
SkFixed width = rite - left;
|
||||
if (width > 0) {
|
||||
if (partialTop > 0) {
|
||||
blitter->blitAntiH(fullLeft - 1,
|
||||
fullTop - 1,
|
||||
1,
|
||||
fixed_to_alpha(SkFixedMul(partialTop, width)));
|
||||
blitter->flush_if_y_changed(y, y + partialTop);
|
||||
}
|
||||
if (fullBot > fullTop) {
|
||||
blitter->getRealBlitter()->blitV(
|
||||
fullLeft - 1, fullTop, fullBot - fullTop, fixed_to_alpha(width));
|
||||
}
|
||||
if (partialBot > 0) {
|
||||
blitter->blitAntiH(fullLeft - 1,
|
||||
fullBot,
|
||||
1,
|
||||
fixed_to_alpha(SkFixedMul(partialBot, width)));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -126,6 +126,54 @@ static void test_crbug_140803() {
|
||||
canvas.drawPath(SkPath().moveTo(2762, 20).quadTo(11, 21702, 10, 21706), paint);
|
||||
}
|
||||
|
||||
static void test_crbug_1239558(skiatest::Reporter* reporter) {
|
||||
SkBitmap bm;
|
||||
bm.allocN32Pixels(256, 256);
|
||||
|
||||
SkCanvas canvas(bm);
|
||||
canvas.clear(SK_ColorWHITE);
|
||||
|
||||
// This creates a single cubic where the control points form an extremely skinny, vertical
|
||||
// triangle contained within the x=0 column of pixels. Since it is convex (ignoring the leading
|
||||
// moveTo's) it uses the convex aaa optimized edge walking algorithm after clipping the path to
|
||||
// the device bounds. However, due to fixed-point math while walking these edges, the edge
|
||||
// walking evaluates to coords that are very slightly less than 0 (i.e. 0.0012). Both the left
|
||||
// and right edges would be out of bounds, but the edge walking is optimized to only clamp the
|
||||
// left edge to the left bounds, and the right edge to the right bounds. After this clamping,
|
||||
// the left and right edges are no longer sorted. This then led to incorrect behavior in various
|
||||
// forms (described below).
|
||||
SkPath path;
|
||||
path.setFillType(SkPathFillType::kWinding);
|
||||
path.moveTo(7.00649e-45f, 2.f);
|
||||
path.moveTo(0.0160219f, 7.45063e-09f);
|
||||
path.moveTo(192.263f, 8.40779e-44f);
|
||||
path.moveTo(7.34684e-40f, 194.25f);
|
||||
path.moveTo(2.3449e-38f, 6.01858e-36f);
|
||||
path.moveTo(7.34684e-40f, 194.25f);
|
||||
path.cubicTo(5.07266e-39f, 56.0488f,
|
||||
0.0119172f, 0.f,
|
||||
7.34684e-40f, 194.25f);
|
||||
|
||||
SkPaint paint;
|
||||
paint.setColor(SK_ColorRED);
|
||||
paint.setAntiAlias(true);
|
||||
// On debug builds, the inverted left/right edges led to a negative coverage that triggered an
|
||||
// assert while converting to a uint8 alpha value. On release builds with UBSAN, it would
|
||||
// detect a negative left shift when computing the pixel address and crash. On regular release
|
||||
// builds it would write a saturate coverage value to pixels that wrapped around to the far edge
|
||||
canvas.drawPath(path, paint);
|
||||
|
||||
// UBSAN and debug builds would fail inside the drawPath() call above, but detect the incorrect
|
||||
// memory access on release builds so that the test would fail. Given the path, it should only
|
||||
// touch pixels with x=0 but the incorrect addressing would wrap to the right edge.
|
||||
for (int y = 0; y < 256; ++y) {
|
||||
if (bm.getColor(255, y) != SK_ColorWHITE) {
|
||||
REPORTER_ASSERT(reporter, false, "drawPath modified incorrect pixels");
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Need to exercise drawing an inverse-path whose bounds intersect the clip,
|
||||
// but whose edges do not (since its a quad which draws only in the bottom half
|
||||
// of its bounds).
|
||||
@ -368,6 +416,7 @@ DEF_TEST(DrawPath, reporter) {
|
||||
test_crbug_165432(reporter);
|
||||
test_crbug_472147_simple(reporter);
|
||||
test_crbug_472147_actual(reporter);
|
||||
test_crbug_1239558(reporter);
|
||||
test_big_aa_rect(reporter);
|
||||
test_halfway();
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user