add rounding-using-doubles methods on SkScalar and SkRect

Inspired by the excellent repro case for https://crbug.com/364224

patch from issue 265933010

BUG=skia:
R=bungeman@google.com

Author: reed@google.com

Review URL: https://codereview.chromium.org/267003002

git-svn-id: http://skia.googlecode.com/svn/trunk@14566 2bbb7eff-a529-9590-31e7-b0007b416f81
This commit is contained in:
commit-bot@chromium.org 2014-05-05 16:04:42 +00:00
parent 3df4e95402
commit 4e332f82fc
5 changed files with 102 additions and 1 deletions

View File

@ -731,6 +731,24 @@ struct SK_API SkRect {
SkScalarRoundToInt(fRight), SkScalarRoundToInt(fBottom));
}
/**
* Variant of round() that explicitly performs the rounding step (i.e. floor(x + 0.5)) using
* double instead of SkScalar (float). It does this by calling SkDScalarRoundToInt(), which
* may be slower than calling SkScalarRountToInt(), but gives slightly more accurate results.
*
* e.g.
* SkScalar x = 0.49999997f;
* int ix = SkScalarRoundToInt(x);
* SkASSERT(0 == ix); // <--- fails
* ix = SkDScalarRoundToInt(x);
* SkASSERT(0 == ix); // <--- succeeds
*/
void dround(SkIRect* dst) const {
SkASSERT(dst);
dst->set(SkDScalarRoundToInt(fLeft), SkDScalarRoundToInt(fTop),
SkDScalarRoundToInt(fRight), SkDScalarRoundToInt(fBottom));
}
/**
* Set the dst rectangle by rounding "out" this rectangle, choosing the
* SkScalarFloor of top and left, and the SkScalarCeil of right and bottom.

View File

@ -83,6 +83,26 @@ static inline bool SkScalarIsFinite(float x) {
#define SkScalarRoundToInt(x) sk_float_round2int(x)
#define SkScalarTruncToInt(x) static_cast<int>(x)
/**
* Variant of SkScalarRoundToInt, that performs the rounding step (adding 0.5) explicitly using
* double, to avoid possibly losing the low bit(s) of the answer before calling floor().
*
* This routine will likely be slower than SkScalarRoundToInt(), and should only be used when the
* extra precision is known to be valuable.
*
* In particular, this catches the following case:
* SkScalar x = 0.49999997;
* int ix = SkScalarRoundToInt(x);
* SkASSERT(0 == ix); // <--- fails
* ix = SkDScalarRoundToInt(x);
* SkASSERT(0 == ix); // <--- succeeds
*/
static inline int SkDScalarRoundToInt(SkScalar x) {
double xx = x;
xx += 0.5;
return (int)floor(xx);
}
/** Returns the absolute value of the specified SkScalar
*/
#define SkScalarAbs(x) sk_float_abs(x)

View File

@ -602,7 +602,11 @@ void SkScan::FillPath(const SkPath& path, const SkRegion& origClip,
// don't reference "origClip" any more, just use clipPtr
SkIRect ir;
path.getBounds().round(&ir);
// We deliberately call dround() instead of round(), since we can't afford to generate a
// bounds that is tighter than the corresponding SkEdges. The edge code basically converts
// the floats to fixed, and then "rounds". If we called round() instead of dround() here,
// we could generate the wrong ir for values like 0.4999997.
path.getBounds().dround(&ir);
if (ir.isEmpty()) {
if (path.isInverseFillType()) {
blitter->blitRegion(*clipPtr);

View File

@ -20,6 +20,49 @@
#include "SkWriter32.h"
#include "Test.h"
static void make_path_crbug364224(SkPath* path) {
path->reset();
path->moveTo(3.747501373f, 2.724499941f);
path->lineTo(3.747501373f, 3.75f);
path->cubicTo(3.747501373f, 3.88774991f, 3.635501385f, 4.0f, 3.497501373f, 4.0f);
path->lineTo(0.7475013733f, 4.0f);
path->cubicTo(0.6095013618f, 4.0f, 0.4975013733f, 3.88774991f, 0.4975013733f, 3.75f);
path->lineTo(0.4975013733f, 1.0f);
path->cubicTo(0.4975013733f, 0.8622499704f, 0.6095013618f, 0.75f, 0.7475013733f,0.75f);
path->lineTo(3.497501373f, 0.75f);
path->cubicTo(3.50275135f, 0.75f, 3.5070014f, 0.7527500391f, 3.513001442f, 0.753000021f);
path->lineTo(3.715001345f, 0.5512499809f);
path->cubicTo(3.648251295f, 0.5194999576f, 3.575501442f, 0.4999999702f, 3.497501373f, 0.4999999702f);
path->lineTo(0.7475013733f, 0.4999999702f);
path->cubicTo(0.4715013802f, 0.4999999702f, 0.2475013733f, 0.7239999771f, 0.2475013733f, 1.0f);
path->lineTo(0.2475013733f, 3.75f);
path->cubicTo(0.2475013733f, 4.026000023f, 0.4715013504f, 4.25f, 0.7475013733f, 4.25f);
path->lineTo(3.497501373f, 4.25f);
path->cubicTo(3.773501396f, 4.25f, 3.997501373f, 4.026000023f, 3.997501373f, 3.75f);
path->lineTo(3.997501373f, 2.474750042f);
path->lineTo(3.747501373f, 2.724499941f);
path->close();
}
static void make_path_crbug364224_simplified(SkPath* path) {
path->moveTo(3.747501373f, 2.724499941f);
path->cubicTo(3.648251295f, 0.5194999576f, 3.575501442f, 0.4999999702f, 3.497501373f, 0.4999999702f);
path->close();
}
static void test_path_crbug364224() {
SkPath path;
SkPaint paint;
SkAutoTUnref<SkSurface> surface(SkSurface::NewRasterPMColor(84, 88));
SkCanvas* canvas = surface->getCanvas();
make_path_crbug364224_simplified(&path);
canvas->drawPath(path, paint);
make_path_crbug364224(&path);
canvas->drawPath(path, paint);
}
static void make_path0(SkPath* path) {
// from * https://code.google.com/p/skia/issues/detail?id=1706
@ -3326,6 +3369,8 @@ public:
};
DEF_TEST(Paths, reporter) {
test_path_crbug364224();
SkTSize<SkScalar>::Make(3,4);
SkPath p, empty;

View File

@ -12,6 +12,19 @@
#include "SkRect.h"
#include "Test.h"
static void test_roundtoint(skiatest::Reporter* reporter) {
SkScalar x = 0.49999997;
int ix = SkScalarRoundToInt(x);
// We "should" get 0, since x < 0.5, but we don't due to float addition rounding up the low
// bit after adding 0.5.
REPORTER_ASSERT(reporter, 1 == ix);
// This version explicitly performs the +0.5 step using double, which should avoid losing the
// low bits.
ix = SkDScalarRoundToInt(x);
REPORTER_ASSERT(reporter, 0 == ix);
}
struct PointSet {
const SkPoint* fPts;
size_t fCount;
@ -187,4 +200,5 @@ static void test_isfinite(skiatest::Reporter* reporter) {
DEF_TEST(Scalar, reporter) {
test_isfinite(reporter);
test_roundtoint(reporter);
}