Skip to content

Commit 04b9443

Browse files
lhkbobSkia Commit-Bot
authored andcommitted
Fix SkRRect::ConservativeIntersect when inputs share corner
Encountered while debugging new GrClipStack. Basically, with the old checks, if aCorner == bCorner but A was a rect, we'd end up rejecting it as not a viable intersection (failing the testCorner == aCorner checks), but wouldn't fall through to the testCorner == bCorner checks. Instead of splitting the if-elseif-else into separate ifs, I chose to add an additional if case that lets the ellipse containment check match a few more cases when we know the two ellipses share an anchor point. Change-Id: I6c8bc9a30d19a56f25da7d9be7832ff72dde1765 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315636 Reviewed-by: Jim Van Verth <jvanverth@google.com> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
1 parent 9bfe92a commit 04b9443

File tree

2 files changed

+32
-1
lines changed

2 files changed

+32
-1
lines changed

src/core/SkRRect.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -790,7 +790,22 @@ SkRRect SkRRectPriv::ConservativeIntersect(const SkRRect& a, const SkRRect& b) {
790790
SkPoint aCorner = getCorner(a.rect(), corner);
791791
SkPoint bCorner = getCorner(b.rect(), corner);
792792

793-
if (test == aCorner) {
793+
if (test == aCorner && test == bCorner) {
794+
// The round rects share a corner anchor, so pick A or B such that its X and Y radii
795+
// are both larger than the other rrect's, or return false if neither A or B has the max
796+
// corner radii (this is more permissive than the single corner tests below).
797+
SkVector aRadii = a.radii(corner);
798+
SkVector bRadii = b.radii(corner);
799+
if (aRadii.fX >= bRadii.fX && aRadii.fY >= bRadii.fY) {
800+
*radii = aRadii;
801+
return true;
802+
} else if (bRadii.fX >= aRadii.fX && bRadii.fY >= aRadii.fY) {
803+
*radii = bRadii;
804+
return true;
805+
} else {
806+
return false;
807+
}
808+
} else if (test == aCorner) {
794809
// Test that A's ellipse is contained by B. This is a non-trivial function to evaluate
795810
// so we resrict it to when the corners have the same radii. If not, we use the more
796811
// conservative test that the extreme point of A's bounding box is contained in B.

tests/RoundRectTest.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,6 +1207,22 @@ static void test_conservative_intersection(skiatest::Reporter* reporter) {
12071207
verify_success(reporter, a, make_inset(a, 1.f, 1.f), kB, kB, kB, kB);
12081208
verify_success(reporter, make_inset(b, 2.f, 2.f), b, kA, kA, kA, kA);
12091209

1210+
// A rectangle exactly matching the corners of the rrect bounds keeps the rrect radii,
1211+
// regardless of whether or not it's the 1st or 2nd arg to ConservativeIntersect.
1212+
SkRRect c = SkRRect::MakeRectXY({0.f, 0.f, 10.f, 10.f}, 2.f, 2.f);
1213+
SkRRect cT = SkRRect::MakeRect({0.f, 0.f, 10.f, 5.f});
1214+
verify_success(reporter, c, cT, kA, kA, kRect, kRect);
1215+
verify_success(reporter, cT, c, kB, kB, kRect, kRect);
1216+
SkRRect cB = SkRRect::MakeRect({0.f, 5.f, 10.f, 10.});
1217+
verify_success(reporter, c, cB, kRect, kRect, kA, kA);
1218+
verify_success(reporter, cB, c, kRect, kRect, kB, kB);
1219+
SkRRect cL = SkRRect::MakeRect({0.f, 0.f, 5.f, 10.f});
1220+
verify_success(reporter, c, cL, kA, kRect, kRect, kA);
1221+
verify_success(reporter, cL, c, kB, kRect, kRect, kB);
1222+
SkRRect cR = SkRRect::MakeRect({5.f, 0.f, 10.f, 10.f});
1223+
verify_success(reporter, c, cR, kRect, kA, kA, kRect);
1224+
verify_success(reporter, cR, c, kRect, kB, kB, kRect);
1225+
12101226
// Failed intersection operations:
12111227

12121228
// A and B's bounds do not intersect

0 commit comments

Comments
 (0)