clamp to 0 in repeat and mirror image tilers

If we were doing this math with real numbers or even just doubles, these
clamps wouldn't be necessary.  But we're favoring speed over accuracy
here when we emulate fmod() and some of those inaccuracies end up with
values outside the [0,tile) range, negative!

To keep the spirit of fast over 100% accurate, I've just added a safety
clamp to 0.  The case in the unit test now returns 0 where it should
really return something like 7 or 8, but at least we won't try to read
_way_ outside the image buffer.

BUG=chromium:749260

Change-Id: Ifc5cfe69798beccbb2a16547510158576e06eb3a
Reviewed-on: https://skia-review.googlesource.com/29580
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Mike Klein <mtklein@chromium.org>
This commit is contained in:
Mike Klein 2017-08-01 14:51:44 -04:00 committed by Skia Commit-Bot
parent dff5d4368d
commit 249ee1f985
4 changed files with 4843 additions and 4709 deletions

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

View File

@ -1079,19 +1079,21 @@ SI F ulp_before(F f) {
return unaligned_load<F>(&bits); return unaligned_load<F>(&bits);
} }
// We make sure to funnel all three tilers through exclusive_clamp() so that we're guaranteed
// to be in [0,ctx->scale), even in the presence of bugs or floating point precision issues.
SI F exclusive_clamp(F v, const SkJumper_TileCtx* ctx) { SI F exclusive_clamp(F v, const SkJumper_TileCtx* ctx) {
v = max(0,v); v = max(0,v);
return min(v, ulp_before(ctx->scale)); return min(v, ulp_before(ctx->scale));
} }
SI F exclusive_repeat(F v, const SkJumper_TileCtx* ctx) { SI F exclusive_repeat(F v, const SkJumper_TileCtx* ctx) {
v = v - floor_(v*ctx->invScale)*ctx->scale; v = v - floor_(v*ctx->invScale)*ctx->scale;
return min(v, ulp_before(ctx->scale)); return exclusive_clamp(v, ctx);
} }
SI F exclusive_mirror(F v, const SkJumper_TileCtx* ctx) { SI F exclusive_mirror(F v, const SkJumper_TileCtx* ctx) {
auto limit = ctx->scale; auto limit = ctx->scale;
auto invLimit = ctx->invScale; auto invLimit = ctx->invScale;
v = abs_( (v-limit) - (limit+limit)*floor_((v-limit)*(invLimit*0.5f)) - limit ); v = abs_( (v-limit) - (limit+limit)*floor_((v-limit)*(invLimit*0.5f)) - limit );
return min(v, ulp_before(limit)); return exclusive_clamp(v, ctx);
} }
// Clamp x or y to [0,limit) == [0,limit - 1 ulp] (think, sampling from images). // Clamp x or y to [0,limit) == [0,limit - 1 ulp] (think, sampling from images).
STAGE(clamp_x) { r = exclusive_clamp (r, (const SkJumper_TileCtx*)ctx); } STAGE(clamp_x) { r = exclusive_clamp (r, (const SkJumper_TileCtx*)ctx); }

View File

@ -262,3 +262,31 @@ DEF_TEST(SkRasterPipeline_2d, r) {
REPORTER_ASSERT(r, ((rgba[2] >> 8) & 0xff) == 128); REPORTER_ASSERT(r, ((rgba[2] >> 8) & 0xff) == 128);
REPORTER_ASSERT(r, ((rgba[3] >> 8) & 0xff) == 128); REPORTER_ASSERT(r, ((rgba[3] >> 8) & 0xff) == 128);
} }
DEF_TEST(SkRasterPipeline_repeat_tiling, r) {
// Repeat tiling works roughly like
// v' = v - floor(v / limit) * limit
//
// If v = 19133558.0f and limit = 9.0f, that's
//
// v' = 19133558.0f - floor(19133558.0f / 9.0f) * 9.0f
//
// The problem comes with that division term. In infinite precision,
// that'd be 2125950 + 8/9, but the nearest float is 2125951.0f.
//
// Then 2125951.0f * 9.0f = 19133559.0f, which is greater than v,
// so v' becomes negative. :'(
// Here's a regression test to make sure this doesn't happen.
float in [4] = {19133558.0f,0,0,0};
float out[4 * SkJumper_kMaxStride];
SkJumper_TileCtx tile = { 9.0f, 1/9.0f };
SkRasterPipeline_<256> p;
p.append(SkRasterPipeline::uniform_color, in);
p.append(SkRasterPipeline::repeat_x, &tile);
p.append(SkRasterPipeline::store_rgba, out);
p.run(0,0,1,1);
REPORTER_ASSERT(r, 0.0f <= out[0] && out[0] < 9.0f);
}