Skip to content

Commit 26d8d77

Browse files
reed-at-googleSkia Commit-Bot
authored andcommitted
don't trust convexity with affine transforms
In theory, a convex shape transformed by an affine matrix should still be convex. However, due to numerical nastiness of floats, when we try to determine if something is convex, we can get different answers pre and post a transformation (think of two line segments nearly colinear). Convex paths take a faster scan converter, but it is only well behaved if the path is, in fact, convex. Thus we have to be conservative about which paths we mark as convex. This bug found a case where a "convex" path, after going through a transform, became (according to our measure) non-convex. The bug was that we *thought* that once convex always convex, but in reality it was not. The fix (hack) is to notice when we transform by an affine matrix (we're still assuming/hoping that scaling and translate keep things convex (1)...) and mark the convexity as "unknown", forcing us to re-compute it. This will slow down these paths, since it costs something to compute convexity. Hopefully non-scale-translate transforms are rare, so we won't notice the speed loss too much. (1) This is not proven. If we find scaling/translation to break our notion of convexity, we'll need to get more aggressive/clever to find a fix. Bug: 899689 Change-Id: I5921eca247428bf89380bc2395fe373fa70deb1d Reviewed-on: https://skia-review.googlesource.com/c/173080 Commit-Queue: Mike Reed <reed@google.com> Reviewed-by: Cary Clark <caryclark@google.com> Reviewed-by: Jim Van Verth <jvanverth@google.com>
1 parent b8c3638 commit 26d8d77

File tree

2 files changed

+70
-0
lines changed

2 files changed

+70
-0
lines changed

src/core/SkPath.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1841,6 +1841,13 @@ void SkPath::transform(const SkMatrix& matrix, SkPath* dst) const {
18411841
dst->fIsVolatile = fIsVolatile;
18421842
}
18431843

1844+
if (!matrix.isScaleTranslate()) {
1845+
// If we're rotated/skewed/etc. we can't trust that our convexity stays put.
1846+
// ... just because convexity is fragle with segments are *nearly* colinear...
1847+
// See Path_crbug_899689 test, which asserts w/o this check.
1848+
dst->fConvexity = kUnknown_Convexity;
1849+
}
1850+
18441851
if (SkPathPriv::kUnknown_FirstDirection == fFirstDirection) {
18451852
dst->fFirstDirection = SkPathPriv::kUnknown_FirstDirection;
18461853
} else {

tests/PathTest.cpp

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5204,3 +5204,66 @@ DEF_TEST(Path_increserve_handle_neg_crbug_883666, r) {
52045204
shallowPath.incReserve(0xffffffff);
52055205
}
52065206

5207+
DEF_TEST(Path_crbug_899689, r) {
5208+
SkBitmap bitmap;
5209+
bitmap.allocN32Pixels(24, 24);
5210+
SkCanvas canvas(bitmap);
5211+
5212+
SkPaint paint;
5213+
paint.setAntiAlias(true);
5214+
paint.setStyle(SkPaint::kFill_Style);
5215+
5216+
// This is monotone in both x and y, but has a tiny concavity
5217+
SkPath path;
5218+
path.moveTo(-1,-1);
5219+
path.lineTo(0, 0);
5220+
path.lineTo(0, 0.5e-10f);
5221+
path.lineTo(0.1e-10f, 1.1e-10f);
5222+
path.lineTo(1.5e-10f, 1.1e-10f);
5223+
path.lineTo(1.5e-10f, 2.5e-10f);
5224+
path.lineTo(0.9f, 1);
5225+
path.lineTo(-1, 1);
5226+
path.close();
5227+
5228+
#if 0
5229+
// If asked, Skia is going to declare it convex
5230+
if(path.isConvex()) {
5231+
printf("convex\n");
5232+
} else {
5233+
printf("not convex\n");
5234+
}
5235+
#endif
5236+
5237+
SkMatrix m;
5238+
m.setRotate(-45);
5239+
m.postScale(10e10f, 10e10f);
5240+
m.postSkew(-1, 0);
5241+
m.postTranslate(1, 10);
5242+
path.transform(m);
5243+
5244+
#if 0
5245+
// The convexity flag is going to survive all affine transforms
5246+
// Even those that will enlarge the concavity and make the path
5247+
// non-monotone.
5248+
// As demonstrated here
5249+
if(path.isConvex()) {
5250+
printf("convex\n");
5251+
} else {
5252+
printf("not convex\n");
5253+
}
5254+
5255+
// This used to print "convex" (and then assert) before the fix.
5256+
#endif
5257+
5258+
// We'll use the path as a clip region
5259+
canvas.clipPath(path);
5260+
5261+
// And now we'll just draw a simple triangle.
5262+
SkPath path2;
5263+
path2.moveTo(15.5f, 15);
5264+
path2.lineTo(50.5f, 50);
5265+
path2.lineTo(-19.5f, 50);
5266+
path2.close();
5267+
canvas.drawPath(path2, paint);
5268+
}
5269+

0 commit comments

Comments
 (0)