AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

inline helpers in SkDashPath.cpp cull_path()

Browse Source 
Following up on a bug report, I'm learning how cull_path() in
SkDashPath.cpp works by refactoring it.  This first CL inlines and
removes its helper functions between() and contains_inclusive().

First thing to notice is that the old between() function is checking
`a <= b <= c || a >= b >= c` in a generic way, ignoring that `a` and `c`
are always rectangle bounds.  Since bounds are sorted, we really only
ever need test lo <= v <= hi, for {left,x,right} or {top,y,bottom}.

So rewrite between() as

   auto between = [](SkScalar lo, SkScalar v, SkScalar hi) {
       return lo <= v && v <= hi;
   };

Then notice that contains_inclusive() is actually now just

   auto contains_inclusive = [](const SkRect& bounds, const SkPoint& pt) {
        return between(bounds.fLeft, pt.fX, bounds.fRight )
            && between(bounds.fTop , pt.fY, bounds.fBottom);
   };

And then notice that we're using the same inputs to both the original
calls to between() and the original call to contains_inclusive()... if
we inline everything it's really just two calls to between() to `&&`
together later to make contains_inclusive().

Finally, once it's all inlined, it's no clearer to keep between() as its
own standalone lambda than to just write out two lines for `inX` and `inY`.

This removes the between() approximation added when between() itself
was added in https://skia-review.googlesource.com/c/skia/+/84862/.  I'm
skeptical that it was ever faster than the comparisons I've got written
here now, but it's not clear what Cary was timing when writing that CL,
nor how much of that speedup comes from this between() approximation or
and how much more generally from the added rectangle specialization.
(I've updated the comment to mention that rectangle specialization too.)

I did run calmbench, which doesn't think this is interesting perf-wise:

    cull_path (compared to master) is likely
        8.31% faster in constXTile_CC
    JSON results available in /var/tmp/bench_cull_path_master.json
    Compared 731 benches. 1 of them seem to be significantly differrent.

Bug: skia:9331
Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Change-Id: I1b6d2c1cd8ee86df6c46b5c6a8c02634b6bb0238
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/233996
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This commit is contained in:
Mike Klein 2019-08-12 12:09:28 -04:00 committed by Skia Commit-Bot
parent f815301b1a
commit e2556a43ad
1 changed files with 9 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
src/utils/SkDashPath.cpp
View File

@ -151,25 +151,9 @@ static bool clip_line(SkPoint pts[2], const SkRect& bounds, SkScalar intervalLen
return true;
}
static bool contains_inclusive(const SkRect& rect, const SkPoint& pt) {
return rect.fLeft <= pt.fX && pt.fX <= rect.fRight &&
rect.fTop <= pt.fY && pt.fY <= rect.fBottom;
}
// Returns true is b is between a and c, that is: a <= b <= c, or a >= b >= c.
// Can perform this test with one branch by observing that, relative to b,
// the condition is true only if one side is positive and one side is negative.
// If the numbers are very small, the optimization may return the wrong result
// because the multiply may generate a zero where the simple compare does not.
// For this reason the assert does not fire when all three numbers are near zero.
static bool between(SkScalar a, SkScalar b, SkScalar c) {
SkASSERT(((a <= b && b <= c) || (a >= b && b >= c)) == ((a - b) * (c - b) <= 0)
|| (SkScalarNearlyZero(a) && SkScalarNearlyZero(b) && SkScalarNearlyZero(c)));
return (a - b) * (c - b) <= 0;
}
// Only handles lines for now. If returns true, dstPath is the new (smaller)
// path. If returns false, then dstPath parameter is ignored.
// Handles only lines and rects.
// If cull_path() returns true, dstPath is the new smaller path,
// otherwise dstPath is ignored.
static bool cull_path(const SkPath& srcPath, const SkStrokeRec& rec,
const SkRect* cullRect, SkScalar intervalLength,
SkPath* dstPath) {
@ -196,10 +180,13 @@ static bool cull_path(const SkPath& srcPath, const SkStrokeRec& rec,
SkScalar priorLength = 0;
while (SkPath::kLine_Verb == iter.next(pts)) {
SkVector v = pts[1] - pts[0];
// if line is entirely outside clip rect, skip it
if (v.fX ? between(bounds.fTop, pts[0].fY, bounds.fBottom) :
between(bounds.fLeft, pts[0].fX, bounds.fRight)) {
bool skipMoveTo = contains_inclusive(bounds, pts[0]);
bool inY = bounds.fTop <= pts[0].fY && pts[0].fY <= bounds.fBottom,
inX = bounds.fLeft <= pts[0].fX && pts[0].fX <= bounds.fRight ;
if (v.fX ? inY : inX) {
bool skipMoveTo = inX && inY;
if (clip_line(pts, bounds, intervalLength,
SkScalarMod(priorLength, intervalLength))) {
if (0 == priorLength || !skipMoveTo) {