Clean up prior backdrop-filter CL
This addresses reviewer comments from https://skia-review.googlesource.com/c/skia/+/239108 that were postponed to get the first CL landed ASAP.
Additionally, removes the now unnecessary applyCTMForBackdrop() function.
Bug: skia:9074
Change-Id: Ibc3ac4a4edd7e5546fa83145346869a68efc33f4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/239440
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 519558d..357f3f5 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -885,14 +885,14 @@
void SkCanvas::DrawDeviceWithFilter(SkBaseDevice* src, const SkImageFilter* filter,
SkBaseDevice* dst, const SkIPoint& dstOrigin,
const SkMatrix& ctm) {
- // The local bounds of the src device; all snap bounds must be intersected with this rect
+ // The local bounds of the src device; all the bounds passed to snapSpecial must be intersected
+ // with this rect.
const SkIRect srcDevRect = SkIRect::MakeWH(src->width(), src->height());
- SkPaint p;
if (!filter) {
- // All devices are currently axis aligned, so they only differ by their origin. This means
- // that we only have to copy a dst-sized block of pixels out of src and translate it to the
- // matching position relative to dst's origin.
+ // All non-filtered devices are currently axis aligned, so they only differ by their origin.
+ // This means that we only have to copy a dst-sized block of pixels out of src and translate
+ // it to the matching position relative to dst's origin.
SkIRect snapBounds = SkIRect::MakeXYWH(dstOrigin.x() - src->getOrigin().x(),
dstOrigin.y() - src->getOrigin().y(),
dst->width(), dst->height());
@@ -902,6 +902,8 @@
auto special = src->snapSpecial(snapBounds);
if (special) {
+ // The image is drawn at 1-1 scale with integer translation, so no filtering is needed.
+ SkPaint p;
dst->drawSpecial(special.get(), 0, 0, p, nullptr, SkMatrix::I());
}
return;
@@ -944,8 +946,8 @@
&layerTargetBounds);
// Map the required input into the root space, then make relative to the src device. This will
- // be conservative contents required to fill a layerInputBounds-sized surface with the backdrop
- // content (transformed back into the layer space using fromRoot).
+ // be the conservative contents required to fill a layerInputBounds-sized surface with the
+ // backdrop content (transformed back into the layer space using fromRoot).
SkIRect backdropBounds = toRoot.mapRect(SkRect::Make(layerInputBounds)).roundOut();
backdropBounds.offset(-src->getOrigin().x(), -src->getOrigin().y());
if (!backdropBounds.intersect(srcDevRect)) {
@@ -962,14 +964,21 @@
colorType = kRGBA_8888_SkColorType;
}
SkColorSpace* colorSpace = src->imageInfo().colorSpace();
+
+ SkPaint p;
if (!toRoot.isIdentity()) {
+ // Drawing the temporary and final filtered image requires a higher filter quality if the
+ // 'toRoot' transformation is not identity, in order to minimize the impact on already
+ // rendered edges/content.
+ // TODO (michaelludwig) - Explore reducing this quality, identify visual tradeoffs
+ p.setFilterQuality(kHigh_SkFilterQuality);
+
// The snapped backdrop content needs to be transformed by fromRoot into the layer space,
// and stored in a temporary surface, which is then used as the input to the actual filter.
auto tmpSurface = special->makeSurface(colorType, colorSpace, layerInputBounds.size());
if (!tmpSurface) {
return;
}
- p.setFilterQuality(kHigh_SkFilterQuality);
auto tmpCanvas = tmpSurface->getCanvas();
tmpCanvas->clear(SK_ColorTRANSPARENT);
@@ -980,6 +989,7 @@
tmpCanvas->translate(-layerInputBounds.fLeft, -layerInputBounds.fTop);
tmpCanvas->concat(fromRoot);
tmpCanvas->translate(src->getOrigin().x(), src->getOrigin().y());
+
tmpCanvas->drawImageRect(special->asImage(), special->subset(),
SkRect::Make(backdropBounds), &p, kStrict_SrcRectConstraint);
special = tmpSurface->makeImageSnapshot();
@@ -991,6 +1001,10 @@
// back into the shared layer space
layerInputBounds = backdropBounds;
layerInputBounds.offset(src->getOrigin().x(), src->getOrigin().y());
+
+ // Similar to the unfiltered case above, when toRoot is the identity, then the final
+ // draw will be 1-1 so there is no need to increase filter quality.
+ p.setFilterQuality(kNone_SkFilterQuality);
}
// Now evaluate the filter on 'special', which contains the backdrop content mapped back into
@@ -1006,9 +1020,10 @@
SkIPoint offset;
special = as_IFB(filter)->filterImage(ctx).imageAndOffset(&offset);
if (special) {
- // Draw the filtered backdrop content into the dst device. Must adjust the reported offset
- // to include the layerInputBounds origin shift, since the returned offset is in the coord
- // system defined by filterCTM, and we want it to be in that of layerMatrix.
+ // Draw the filtered backdrop content into the dst device. We add layerInputBounds origin
+ // to offset because the original value in 'offset' was relative to 'filterCTM'. 'filterCTM'
+ // had subtracted the layerInputBounds origin, so adding that back makes 'offset' relative
+ // to 'layerMatrix' (what we need it to be when drawing the image by 'toRoot').
offset += layerInputBounds.topLeft();
// Manually setting the device's CTM requires accounting for the device's origin.
@@ -1022,9 +1037,9 @@
// And because devices don't have a special-image draw function that supports arbitrary
// matrices, we are abusing the asImage() functionality here...
SkRect specialSrc = SkRect::Make(special->subset());
- auto hackImageMeh = special->asImage();
+ auto looseImage = special->asImage();
dst->drawImageRect(
- hackImageMeh.get(), &specialSrc,
+ looseImage.get(), &specialSrc,
SkRect::MakeXYWH(offset.x(), offset.y(), special->width(), special->height()),
p, kStrict_SrcRectConstraint);
}