Skip to content

Commit db80cbe

Browse files
reed-at-googleSkia Commit-Bot
authored andcommitted
change convex scan converter to be defensive
ran convex_path_* nanobench, with no appreciable perf loss (for the cases tested) Bug: 899689 Bug: skia:8606 Change-Id: Ida253c5057f38b90cda86a1717a8bb7c4a6155dc Reviewed-on: https://skia-review.googlesource.com/c/175832 Commit-Queue: Mike Reed <reed@google.com> Reviewed-by: Mike Klein <mtklein@google.com> Reviewed-by: Cary Clark <caryclark@google.com>
1 parent 4f73883 commit db80cbe

File tree

2 files changed

+33
-27
lines changed

2 files changed

+33
-27
lines changed

src/core/SkPath.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1863,6 +1863,11 @@ void SkPath::transform(const SkMatrix& matrix, SkPath* dst) const {
18631863
// convex after a transformation, so mark it as unknown here.
18641864
// However, some transformations are thought to be safe:
18651865
// axis-aligned values under scale/translate.
1866+
//
1867+
// See skbug.com/8606
1868+
// If we can land a robust convex scan-converter, we may be able to relax/remove this
1869+
// check, and keep convex paths marked as such after a general transform...
1870+
//
18661871
if (matrix.isScaleTranslate() && SkPathPriv::IsAxisAligned(*this)) {
18671872
dst->setConvexity(convexity);
18681873
} else {

src/core/SkScan_Path.cpp

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,17 @@ static bool update_edge(SkEdge* edge, int last_y) {
210210
return true;
211211
}
212212

213-
static void walk_convex_edges(SkEdge* prevHead, SkPath::FillType,
214-
SkBlitter* blitter, int start_y, int stop_y,
215-
PrePostProc proc) {
213+
// Unexpected conditions for which we need to return
214+
#define ASSERT_RETURN(cond) \
215+
do { \
216+
if (!(cond)) { \
217+
SkASSERT(false); \
218+
return; \
219+
} \
220+
} while (0)
221+
222+
// Needs Y to only change once (looser than convex in X)
223+
static void walk_simple_edges(SkEdge* prevHead, SkBlitter* blitter, int start_y, int stop_y) {
216224
validate_sort(prevHead->fNext);
217225

218226
SkEdge* leftE = prevHead->fNext;
@@ -222,31 +230,29 @@ static void walk_convex_edges(SkEdge* prevHead, SkPath::FillType,
222230
// our edge choppers for curves can result in the initial edges
223231
// not lining up, so we take the max.
224232
int local_top = SkMax32(leftE->fFirstY, riteE->fFirstY);
225-
SkASSERT(local_top >= start_y);
233+
ASSERT_RETURN(local_top >= start_y);
226234

227-
for (;;) {
235+
while (local_top < stop_y) {
228236
SkASSERT(leftE->fFirstY <= stop_y);
229237
SkASSERT(riteE->fFirstY <= stop_y);
230238

231-
if (leftE->fX > riteE->fX || (leftE->fX == riteE->fX &&
232-
leftE->fDX > riteE->fDX)) {
233-
using std::swap;
234-
swap(leftE, riteE);
235-
}
236-
237239
int local_bot = SkMin32(leftE->fLastY, riteE->fLastY);
238240
local_bot = SkMin32(local_bot, stop_y - 1);
239-
SkASSERT(local_top <= local_bot);
241+
ASSERT_RETURN(local_top <= local_bot);
240242

241243
SkFixed left = leftE->fX;
242244
SkFixed dLeft = leftE->fDX;
243245
SkFixed rite = riteE->fX;
244246
SkFixed dRite = riteE->fDX;
245247
int count = local_bot - local_top;
246-
SkASSERT(count >= 0);
248+
ASSERT_RETURN(count >= 0);
249+
247250
if (0 == (dLeft | dRite)) {
248251
int L = SkFixedRoundToInt(left);
249252
int R = SkFixedRoundToInt(rite);
253+
if (L > R) {
254+
std::swap(L, R);
255+
}
250256
if (L < R) {
251257
count += 1;
252258
blitter->blitRect(L, local_top, R - L, count);
@@ -256,6 +262,9 @@ static void walk_convex_edges(SkEdge* prevHead, SkPath::FillType,
256262
do {
257263
int L = SkFixedRoundToInt(left);
258264
int R = SkFixedRoundToInt(rite);
265+
if (L > R) {
266+
std::swap(L, R);
267+
}
259268
if (L < R) {
260269
blitter->blitH(L, local_top, R - L);
261270
}
@@ -274,26 +283,19 @@ static void walk_convex_edges(SkEdge* prevHead, SkPath::FillType,
274283

275284
if (!update_edge(leftE, local_bot)) {
276285
if (currE->fFirstY >= stop_y) {
277-
break;
286+
return; // we're done
278287
}
279288
leftE = currE;
280289
currE = currE->fNext;
290+
ASSERT_RETURN(leftE->fFirstY == local_top);
281291
}
282292
if (!update_edge(riteE, local_bot)) {
283293
if (currE->fFirstY >= stop_y) {
284-
break;
294+
return; // we're done
285295
}
286296
riteE = currE;
287297
currE = currE->fNext;
288-
}
289-
290-
SkASSERT(leftE);
291-
SkASSERT(riteE);
292-
293-
// check our bottom clip
294-
SkASSERT(local_top == local_bot + 1);
295-
if (local_top >= stop_y) {
296-
break;
298+
ASSERT_RETURN(riteE->fFirstY == local_top);
297299
}
298300
}
299301
}
@@ -467,7 +469,7 @@ void sk_fill_path(const SkPath& path, const SkIRect& clipRect, SkBlitter* blitte
467469

468470
// count >= 2 is required as the convex walker does not handle missing right edges
469471
if (path.isConvex() && (nullptr == proc) && count >= 2) {
470-
walk_convex_edges(&headEdge, path.getFillType(), blitter, start_y, stop_y, nullptr);
472+
walk_simple_edges(&headEdge, blitter, start_y, stop_y);
471473
} else {
472474
walk_edges(&headEdge, path.getFillType(), blitter, start_y, stop_y, proc,
473475
shiftedClip.right());
@@ -733,8 +735,7 @@ static void sk_fill_triangle(const SkPoint pts[], const SkIRect* clipRect,
733735
if (clipRect && start_y < clipRect->fTop) {
734736
start_y = clipRect->fTop;
735737
}
736-
walk_convex_edges(&headEdge, SkPath::kEvenOdd_FillType, blitter, start_y, stop_y, nullptr);
737-
// walk_edges(&headEdge, SkPath::kEvenOdd_FillType, blitter, start_y, stop_y, nullptr);
738+
walk_simple_edges(&headEdge, blitter, start_y, stop_y);
738739
}
739740

740741
void SkScan::FillTriangle(const SkPoint pts[], const SkRasterClip& clip,

0 commit comments

Comments
 (0)