Revert "Call blitFatAntiRect to avoid overhead in MaskAdditiveBlitter"
This reverts commit 6d1aaca8276ff4ae2e10870f7e2c3222907cc4aa.
Reason for revert: speculative revert for Google3 diff.
Original change's description:
> Call blitFatAntiRect to avoid overhead in MaskAdditiveBlitter
>
> This results in 25% (720ns vs 560ns) speedup for
> path_fill_small_rect bench in 8888 config. Some skps have a lot of stroked
> horizontal/vertical lines (e.g., bar charts) so this improvement could
> have a great impact there. For example, cereal converts Microsoft word docx
> to PNGs on server and the sample docx has a big bar chart. That inspired
> this improvement.
>
> Bug: skia:
> Change-Id: Icf96c966edf87427b3d1f53da09a49930eda2ac1
> Reviewed-on: https://skia-review.googlesource.com/46584
> Commit-Queue: Yuqian Li <liyuqian@google.com>
> Reviewed-by: Mike Klein <mtklein@google.com>
TBR=mtklein@google.com,herb@google.com,liyuqian@google.com,reed@google.com
Change-Id: Ia30df0be874749c5f8ee0138f3d7d961d5bc3fcf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:
Reviewed-on: https://skia-review.googlesource.com/48220
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
diff --git a/src/core/SkBlitter.cpp b/src/core/SkBlitter.cpp
index 8059f6e..5eb5dd8 100644
--- a/src/core/SkBlitter.cpp
+++ b/src/core/SkBlitter.cpp
@@ -45,8 +45,9 @@
return (SkAlpha)(a * 255);
}
-void SkBlitter::blitFatAntiRect(const SkRect& rect, const SkIRect& bounds) {
- SkASSERT(rect.roundOut() == bounds && bounds.width() >= 3 && bounds.height() >= 3);
+void SkBlitter::blitFatAntiRect(const SkRect& rect) {
+ SkIRect bounds = rect.roundOut();
+ SkASSERT(bounds.width() >= 3 && bounds.height() >= 3);
int runSize = bounds.width() + 1; // +1 so we can set runs[bounds.width()] = 0
void* storage = this->allocBlitMemory(runSize * (sizeof(int16_t) + sizeof(SkAlpha)));
diff --git a/src/core/SkBlitter.h b/src/core/SkBlitter.h
index e5375a5..c280ac3 100644
--- a/src/core/SkBlitter.h
+++ b/src/core/SkBlitter.h
@@ -69,9 +69,8 @@
virtual void blitAntiRect(int x, int y, int width, int height,
SkAlpha leftAlpha, SkAlpha rightAlpha);
- // Blit a rect in AA with bounds (i.e., rect.roundOut()) at least 3 x 3
- // (small rect has too many edge cases...)
- void blitFatAntiRect(const SkRect& rect, const SkIRect& bounds);
+ // Blit a rect in AA with size at least 3 x 3 (small rect has too many edge cases...)
+ void blitFatAntiRect(const SkRect& rect);
/// Blit a pattern of pixels defined by a rectangle-clipped mask;
/// typically used for text.
diff --git a/src/core/SkScanPriv.h b/src/core/SkScanPriv.h
index c85b668..f8d3bb2 100644
--- a/src/core/SkScanPriv.h
+++ b/src/core/SkScanPriv.h
@@ -117,23 +117,6 @@
return false;
}
-// Check if the path is a rect and fat enough after clipping; if so, blit it.
-static inline bool TryBlitFatAntiRect(SkBlitter* blitter, const SkPath& path, const SkIRect& clip) {
- SkRect rect;
- if (!path.isRect(&rect)) {
- return false; // not rect
- }
- if (!rect.intersect(SkRect::Make(clip))) {
- return true; // The intersection is empty. Hence consider it done.
- }
- SkIRect bounds = rect.roundOut();
- if (bounds.width() < 3 || bounds.height() < 3) {
- return false; // not fat
- }
- blitter->blitFatAntiRect(rect, bounds);
- return true;
-}
-
using FillPathFunc = std::function<void(const SkPath& path, SkBlitter* blitter, bool isInverse,
const SkIRect& ir, const SkRegion* clipRgn, const SkIRect* clipRect, bool forceRLE)>;
diff --git a/src/core/SkScan_AAAPath.cpp b/src/core/SkScan_AAAPath.cpp
index ff6bf82..e5478ea 100644
--- a/src/core/SkScan_AAAPath.cpp
+++ b/src/core/SkScan_AAAPath.cpp
@@ -1676,37 +1676,29 @@
bool forceRLE) {
FillPathFunc fillPathFunc = [](const SkPath& path, SkBlitter* blitter, bool isInverse,
const SkIRect& ir, const SkRegion* clipRgn, const SkIRect* clipRect, bool forceRLE){
- const SkIRect& clipBounds = clipRgn->getBounds();
-
// The mask blitter (where we store intermediate alpha values directly in a mask, and then
// call the real blitter once in the end to blit the whole mask) is faster than the RLE
// blitter when the blit region is small enough (i.e., canHandleRect(ir)).
// When isInverse is true, the blit region is no longer ir so we won't use the mask blitter.
// The caller may also use the forceRLE flag to force not using the mask blitter.
- // Also, when the path is a simple rect, preparing a mask and blitting it might have too
- // much overhead. Hence we'll use blitFatAntiRect to avoid the mask and its overhead.
if (MaskAdditiveBlitter::canHandleRect(ir) && !isInverse && !forceRLE) {
- // blitFatAntiRect is slower than the normal AAA flow without MaskAdditiveBlitter.
- // Hence only tryBlitFatAntiRect when MaskAdditiveBlitter would have been used.
- if (!TryBlitFatAntiRect(blitter, path, clipBounds)) {
- MaskAdditiveBlitter additiveBlitter(blitter, ir, *clipRgn, isInverse);
- aaa_fill_path(path, clipBounds, &additiveBlitter, ir.fTop, ir.fBottom,
- clipRect == nullptr, true, forceRLE);
- }
+ MaskAdditiveBlitter additiveBlitter(blitter, ir, *clipRgn, isInverse);
+ aaa_fill_path(path, clipRgn->getBounds(), &additiveBlitter, ir.fTop, ir.fBottom,
+ clipRect == nullptr, true, forceRLE);
} else if (!isInverse && path.isConvex()) {
// If the filling area is convex (i.e., path.isConvex && !isInverse), our simpler
// aaa_walk_convex_edges won't generate alphas above 255. Hence we don't need
// SafeRLEAdditiveBlitter (which is slow due to clamping). The basic RLE blitter
// RunBasedAdditiveBlitter would suffice.
RunBasedAdditiveBlitter additiveBlitter(blitter, ir, *clipRgn, isInverse);
- aaa_fill_path(path, clipBounds, &additiveBlitter, ir.fTop, ir.fBottom,
+ aaa_fill_path(path, clipRgn->getBounds(), &additiveBlitter, ir.fTop, ir.fBottom,
clipRect == nullptr, false, forceRLE);
} else {
// If the filling area might not be convex, the more involved aaa_walk_edges would
// be called and we have to clamp the alpha downto 255. The SafeRLEAdditiveBlitter
// does that at a cost of performance.
SafeRLEAdditiveBlitter additiveBlitter(blitter, ir, *clipRgn, isInverse);
- aaa_fill_path(path, clipBounds, &additiveBlitter, ir.fTop, ir.fBottom,
+ aaa_fill_path(path, clipRgn->getBounds(), &additiveBlitter, ir.fTop, ir.fBottom,
clipRect == nullptr, false, forceRLE);
}
};
diff --git a/src/core/SkScan_DAAPath.cpp b/src/core/SkScan_DAAPath.cpp
index 1a9bc5d..a12b6da 100644
--- a/src/core/SkScan_DAAPath.cpp
+++ b/src/core/SkScan_DAAPath.cpp
@@ -327,9 +327,14 @@
SkIRect clippedIR = ir;
clippedIR.intersect(clipBounds);
- // The overhead of even constructing SkCoverageDeltaList/Mask is too big.
- // So TryBlitFatAntiRect and return if it's successful.
- if (TryBlitFatAntiRect(blitter, path, clipBounds)) {
+ SkRect rect;
+ if (!isInverse && path.isRect(&rect) && clippedIR.height() >= 3 && clippedIR.width() >= 3) {
+ // The overhead of even constructing SkCoverageDeltaList/Mask is too big. So just blit.
+ bool nonEmpty = rect.intersect(SkRect::Make(clipBounds));
+ SkASSERT(nonEmpty); // do_fill_path should have already handled the empty case
+ if (nonEmpty) {
+ blitter->blitFatAntiRect(rect);
+ }
return;
}