Handle inverted rects in SkRRect creation methods

An alternative way of addressing this is to alter SkCanvas::drawRoundRect to just reject isEmpty (i.e., un-sorted or truly empty) input rects.

BUG=skia:3786

Review URL: https://codereview.chromium.org/1373293002
This commit is contained in:
robertphillips 2015-09-29 11:24:07 -07:00 committed by Commit bot
parent 3d096654b9
commit 05302f8f24
4 changed files with 126 additions and 42 deletions

View File

@ -18,6 +18,15 @@
namespace skiagm {
static SkColor gen_color(SkRandom* rand) {
SkScalar hsv[3];
hsv[0] = rand->nextRangeF(0.0f, 360.0f);
hsv[1] = rand->nextRangeF(0.75f, 1.0f);
hsv[2] = rand->nextRangeF(0.75f, 1.0f);
return sk_tool_utils::color_to_565(SkHSVToColor(hsv));
}
class RoundRectGM : public GM {
public:
RoundRectGM() {
@ -127,19 +136,10 @@ protected:
}
}
SkColor genColor(SkRandom* rand) {
SkScalar hsv[3];
hsv[0] = rand->nextRangeF(0.0f, 360.0f);
hsv[1] = rand->nextRangeF(0.75f, 1.0f);
hsv[2] = rand->nextRangeF(0.75f, 1.0f);
return sk_tool_utils::color_to_565(SkHSVToColor(hsv));
}
void onDraw(SkCanvas* canvas) override {
SkRandom rand(1);
canvas->translate(20 * SK_Scalar1, 20 * SK_Scalar1);
SkRect rect = SkRect::MakeLTRB(-20, -30, 20, 30);
const SkRect rect = SkRect::MakeLTRB(-20, -30, 20, 30);
SkRRect circleRect;
circleRect.setRectXY(rect, 5, 5);
@ -167,7 +167,7 @@ protected:
3 * SK_Scalar1 / 4);
canvas->concat(mat);
SkColor color = genColor(&rand);
SkColor color = gen_color(&rand);
fPaints[i].setColor(color);
canvas->drawRect(rect, rectPaint);
@ -192,7 +192,7 @@ protected:
canvas->translate(kXStart + SK_Scalar1 * kXStep * 2.55f + SK_Scalar1 / 4,
kYStart + SK_Scalar1 * kYStep * i + 3 * SK_Scalar1 / 4);
SkColor color = genColor(&rand);
SkColor color = gen_color(&rand);
fPaints[i].setColor(color);
canvas->drawRect(rect, rectPaint);
@ -212,7 +212,7 @@ protected:
kYStart + SK_Scalar1 * kYStep * i + 3 * SK_Scalar1 / 4 +
SK_ScalarHalf * kYStep);
SkColor color = genColor(&rand);
SkColor color = gen_color(&rand);
fPaints[i].setColor(color);
canvas->drawRect(rect, rectPaint);
@ -231,7 +231,7 @@ protected:
canvas->translate(kXStart + SK_Scalar1 * kXStep * 3.25f + SK_Scalar1 / 4,
kYStart + SK_Scalar1 * kYStep * i + 3 * SK_Scalar1 / 4);
SkColor color = genColor(&rand);
SkColor color = gen_color(&rand);
fPaints[i].setColor(color);
canvas->drawRRect(circleRect, fPaints[i]);
@ -250,7 +250,7 @@ protected:
kYStart + SK_Scalar1 * kYStep * i + 3 * SK_Scalar1 / 4 +
SK_ScalarHalf * kYStep);
SkColor color = genColor(&rand);
SkColor color = gen_color(&rand);
fPaints[i].setColor(color);
canvas->drawRRect(circleRect, fPaints[i]);
@ -275,7 +275,7 @@ protected:
kYStart + SK_Scalar1 * kYStep * i + 3 * SK_Scalar1 / 4 +
SK_ScalarHalf * kYStep);
SkColor color = genColor(&rand);
SkColor color = gen_color(&rand);
fPaints[i].setColor(color);
fPaints[i].setShader(shader);
@ -310,7 +310,7 @@ protected:
kYStart + SK_Scalar1 * kYStep * i + 3 * SK_Scalar1 / 4 +
SK_ScalarHalf * kYStep);
SkColor color = genColor(&rand);
SkColor color = gen_color(&rand);
SkPaint p;
p.setAntiAlias(true);
@ -323,6 +323,24 @@ protected:
}
}
// test old entry point (skbug.com/3786)
{
canvas->save();
canvas->translate(kXStart + SK_Scalar1 * kXStep * 5 + SK_Scalar1 / 4,
kYStart + SK_Scalar1 * kYStep * 4 + SK_Scalar1 / 4 +
SK_ScalarHalf * kYStep);
const SkColor color = gen_color(&rand);
SkPaint p;
p.setColor(color);
const SkRect oooRect = { 20, 30, -20, -30 }; // intentionally out of order
canvas->drawRoundRect(oooRect, 10, 10, p);
canvas->restore();
}
}
private:

View File

@ -127,12 +127,14 @@ public:
* Set this RR to match the supplied rect. All radii will be 0.
*/
void setRect(const SkRect& rect) {
if (rect.isEmpty()) {
fRect = rect;
fRect.sort();
if (fRect.isEmpty()) {
this->setEmpty();
return;
}
fRect = rect;
memset(fRadii, 0, sizeof(fRadii));
fType = kRect_Type;
@ -156,15 +158,17 @@ public:
* width and all y radii will equal half the height.
*/
void setOval(const SkRect& oval) {
if (oval.isEmpty()) {
fRect = oval;
fRect.sort();
if (fRect.isEmpty()) {
this->setEmpty();
return;
}
SkScalar xRad = SkScalarHalf(oval.width());
SkScalar yRad = SkScalarHalf(oval.height());
SkScalar xRad = SkScalarHalf(fRect.width());
SkScalar yRad = SkScalarHalf(fRect.height());
fRect = oval;
for (int i = 0; i < 4; ++i) {
fRadii[i].set(xRad, yRad);
}

View File

@ -11,7 +11,10 @@
///////////////////////////////////////////////////////////////////////////////
void SkRRect::setRectXY(const SkRect& rect, SkScalar xRad, SkScalar yRad) {
if (rect.isEmpty() || !rect.isFinite()) {
fRect = rect;
fRect.sort();
if (fRect.isEmpty() || !fRect.isFinite()) {
this->setEmpty();
return;
}
@ -25,14 +28,13 @@ void SkRRect::setRectXY(const SkRect& rect, SkScalar xRad, SkScalar yRad) {
return;
}
if (rect.width() < xRad+xRad || rect.height() < yRad+yRad) {
SkScalar scale = SkMinScalar(rect.width() / (xRad + xRad), rect.height() / (yRad + yRad));
if (fRect.width() < xRad+xRad || fRect.height() < yRad+yRad) {
SkScalar scale = SkMinScalar(fRect.width() / (xRad + xRad), fRect.height() / (yRad + yRad));
SkASSERT(scale < SK_Scalar1);
xRad = SkScalarMul(xRad, scale);
yRad = SkScalarMul(yRad, scale);
}
fRect = rect;
for (int i = 0; i < 4; ++i) {
fRadii[i].set(xRad, yRad);
}
@ -47,7 +49,10 @@ void SkRRect::setRectXY(const SkRect& rect, SkScalar xRad, SkScalar yRad) {
void SkRRect::setNinePatch(const SkRect& rect, SkScalar leftRad, SkScalar topRad,
SkScalar rightRad, SkScalar bottomRad) {
if (rect.isEmpty() || !rect.isFinite()) {
fRect = rect;
fRect.sort();
if (fRect.isEmpty() || !fRect.isFinite()) {
this->setEmpty();
return;
}
@ -64,11 +69,11 @@ void SkRRect::setNinePatch(const SkRect& rect, SkScalar leftRad, SkScalar topRad
bottomRad = SkMaxScalar(bottomRad, 0);
SkScalar scale = SK_Scalar1;
if (leftRad + rightRad > rect.width()) {
scale = rect.width() / (leftRad + rightRad);
if (leftRad + rightRad > fRect.width()) {
scale = fRect.width() / (leftRad + rightRad);
}
if (topRad + bottomRad > rect.height()) {
scale = SkMinScalar(scale, rect.height() / (topRad + bottomRad));
if (topRad + bottomRad > fRect.height()) {
scale = SkMinScalar(scale, fRect.height() / (topRad + bottomRad));
}
if (scale < SK_Scalar1) {
@ -79,7 +84,7 @@ void SkRRect::setNinePatch(const SkRect& rect, SkScalar leftRad, SkScalar topRad
}
if (leftRad == rightRad && topRad == bottomRad) {
if (leftRad >= SkScalarHalf(rect.width()) && topRad >= SkScalarHalf(rect.height())) {
if (leftRad >= SkScalarHalf(fRect.width()) && topRad >= SkScalarHalf(fRect.height())) {
fType = kOval_Type;
} else if (0 == leftRad || 0 == topRad) {
// If the left and (by equality check above) right radii are zero then it is a rect.
@ -96,7 +101,6 @@ void SkRRect::setNinePatch(const SkRect& rect, SkScalar leftRad, SkScalar topRad
fType = kNinePatch_Type;
}
fRect = rect;
fRadii[kUpperLeft_Corner].set(leftRad, topRad);
fRadii[kUpperRight_Corner].set(rightRad, topRad);
fRadii[kLowerRight_Corner].set(rightRad, bottomRad);
@ -138,7 +142,10 @@ static double compute_min_scale(double rad1, double rad2, double limit, double c
}
void SkRRect::setRectRadii(const SkRect& rect, const SkVector radii[4]) {
if (rect.isEmpty() || !rect.isFinite()) {
fRect = rect;
fRect.sort();
if (fRect.isEmpty() || !fRect.isFinite()) {
this->setEmpty();
return;
}
@ -148,7 +155,6 @@ void SkRRect::setRectRadii(const SkRect& rect, const SkVector radii[4]) {
return;
}
fRect = rect;
memcpy(fRadii, radii, sizeof(fRadii));
bool allCornersSquare = true;
@ -184,10 +190,10 @@ void SkRRect::setRectRadii(const SkRect& rect, const SkVector radii[4]) {
// If f < 1, then all corner radii are reduced by multiplying them by f."
double scale = 1.0;
scale = compute_min_scale(fRadii[0].fX, fRadii[1].fX, rect.width(), scale);
scale = compute_min_scale(fRadii[1].fY, fRadii[2].fY, rect.height(), scale);
scale = compute_min_scale(fRadii[2].fX, fRadii[3].fX, rect.width(), scale);
scale = compute_min_scale(fRadii[3].fY, fRadii[0].fY, rect.height(), scale);
scale = compute_min_scale(fRadii[0].fX, fRadii[1].fX, fRect.width(), scale);
scale = compute_min_scale(fRadii[1].fY, fRadii[2].fY, fRect.height(), scale);
scale = compute_min_scale(fRadii[2].fX, fRadii[3].fX, fRect.width(), scale);
scale = compute_min_scale(fRadii[3].fY, fRadii[0].fY, fRect.height(), scale);
if (scale < 1.0) {
for (int i = 0; i < 4; ++i) {
@ -204,8 +210,8 @@ void SkRRect::setRectRadii(const SkRect& rect, const SkVector radii[4]) {
// path.addRRect(rrect);
// rrect.rect() != path.getBounds()
for (int i = 0; i < 4; ++i) {
fRadii[i].fX = clamp_radius_check_predicates(fRadii[i].fX, rect.fLeft, rect.fRight);
fRadii[i].fY = clamp_radius_check_predicates(fRadii[i].fY, rect.fTop, rect.fBottom);
fRadii[i].fX = clamp_radius_check_predicates(fRadii[i].fX, fRect.fLeft, fRect.fRight);
fRadii[i].fY = clamp_radius_check_predicates(fRadii[i].fY, fRect.fTop, fRect.fBottom);
}
// At this point we're either oval, simple, or complex (not empty or rect).
this->computeType();

View File

@ -47,6 +47,61 @@ static void test_empty_crbug_458524(skiatest::Reporter* reporter) {
REPORTER_ASSERT(reporter, SkRRect::kEmpty_Type == other.getType());
}
// Test that all the SkRRect entry points correctly handle un-sorted and
// zero-sized input rects
static void test_empty(skiatest::Reporter* reporter) {
static const SkRect oooRects[] = { // out of order
{ 100, 0, 0, 100 }, // ooo horizontal
{ 0, 100, 100, 0 }, // ooo vertical
{ 100, 100, 0, 0 }, // ooo both
};
static const SkRect emptyRects[] = {
{ 100, 100, 100, 200 }, // empty horizontal
{ 100, 100, 200, 100 }, // empty vertical
{ 100, 100, 100, 100 }, // empty both
{ 0, 0, 0, 0 } // setEmpty-empty
};
static const SkVector radii[4] = { { 0, 1 }, { 2, 3 }, { 4, 5 }, { 6, 7 } };
SkRRect r;
for (size_t i = 0; i < SK_ARRAY_COUNT(oooRects); ++i) {
r.setRect(oooRects[i]);
REPORTER_ASSERT(reporter, !r.isEmpty());
r.setOval(oooRects[i]);
REPORTER_ASSERT(reporter, !r.isEmpty());
r.setRectXY(oooRects[i], 1, 2);
REPORTER_ASSERT(reporter, !r.isEmpty());
r.setNinePatch(oooRects[i], 0, 1, 2, 3);
REPORTER_ASSERT(reporter, !r.isEmpty());
r.setRectRadii(oooRects[i], radii);
REPORTER_ASSERT(reporter, !r.isEmpty());
}
for (size_t i = 0; i < SK_ARRAY_COUNT(emptyRects); ++i) {
r.setRect(emptyRects[i]);
REPORTER_ASSERT(reporter, r.isEmpty());
r.setOval(emptyRects[i]);
REPORTER_ASSERT(reporter, r.isEmpty());
r.setRectXY(emptyRects[i], 1, 2);
REPORTER_ASSERT(reporter, r.isEmpty());
r.setNinePatch(emptyRects[i], 0, 1, 2, 3);
REPORTER_ASSERT(reporter, r.isEmpty());
r.setRectRadii(emptyRects[i], radii);
REPORTER_ASSERT(reporter, r.isEmpty());
}
}
static const SkScalar kWidth = 100.0f;
static const SkScalar kHeight = 100.0f;
@ -679,4 +734,5 @@ DEF_TEST(RoundRect, reporter) {
test_issue_2696(reporter);
test_tricky_radii(reporter);
test_empty_crbug_458524(reporter);
test_empty(reporter);
}