Reland "Track device coordinate space as matrix"
This reverts commit 915b779f9cfd5a885154bf4cb6b65230ed3ef5de.
Reason for revert: finally coming back to this, figure out what's wrong on Android
Original change's description:
> Revert "Track device coordinate space as matrix"
>
> This reverts commit b74d5548a43509a6af55b6ce37d61901f1555871.
>
> Reason for revert: see if this fixes the android roll
>
> Original change's description:
> > Track device coordinate space as matrix
> >
> > This is a required step to be able to cleanly draw image filtered
> > device layers with arbitrary matrices, instead of relying on
> > SkMatrixImageFilter to apply the transformation.
> >
> > Bug: skia:9545
> > Change-Id: I8d84679a281538875cf4a1b73565294fb7f89c86
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/249076
> > Commit-Queue: Michael Ludwig <michaelludwig@google.com>
> > Reviewed-by: Mike Reed <reed@google.com>
>
> TBR=robertphillips@google.com,reed@google.com,michaelludwig@google.com
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: skia:9545
> Change-Id: Ie374a7500cfbff35cb0782beb863086e118a005a
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/249986
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
TBR=robertphillips@google.com,reed@google.com,michaelludwig@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: skia:9545
Change-Id: If31a9be86cb340a0874533c044c19b6787d5f176
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/272340
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp
index fdd776f..7b56ef6 100644
--- a/src/core/SkBitmapDevice.cpp
+++ b/src/core/SkBitmapDevice.cpp
@@ -826,3 +826,7 @@
return ClipType::kComplex;
}
}
+
+SkIRect SkBitmapDevice::onDevClipBounds() const {
+ return fRCStack.rc().getBounds();
+}
diff --git a/src/core/SkBitmapDevice.h b/src/core/SkBitmapDevice.h
index 76a18ed..b9e502d 100644
--- a/src/core/SkBitmapDevice.h
+++ b/src/core/SkBitmapDevice.h
@@ -133,6 +133,7 @@
void onAsRgnClip(SkRegion*) const override;
void validateDevBounds(const SkIRect& r) override;
ClipType onGetClipType() const override;
+ SkIRect onDevClipBounds() const override;
virtual void drawBitmap(const SkBitmap&, const SkMatrix&, const SkRect* dstOrNull,
const SkPaint&);
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 6e4d534..cdd170a 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -290,8 +290,6 @@
return false;
}
- int getX() const { return fDevice->getOrigin().x(); }
- int getY() const { return fDevice->getOrigin().y(); }
const SkPaint* getPaint() const { return fPaint; }
SkBaseDevice* fDevice;
@@ -634,7 +632,7 @@
if (!d) {
return SkIRect::MakeEmpty();
}
- return SkIRect::MakeXYWH(d->getOrigin().x(), d->getOrigin().y(), d->width(), d->height());
+ return d->getGlobalBounds();
}
SkBaseDevice* SkCanvas::getDevice() const {
@@ -912,7 +910,8 @@
// 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());
-
+ // TODO(michaelludwig) - Update this function to use the relative transforms between src and
+ // dst; for now, since devices never have complex transforms, we can keep using getOrigin().
if (!filter) {
// 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
@@ -1224,27 +1223,26 @@
}
void SkCanvas::internalSaveBehind(const SkRect* localBounds) {
- SkIRect devBounds;
- if (localBounds) {
- SkRect tmp;
- fMCRec->fMatrix.mapRect(&tmp, *localBounds);
- if (!devBounds.intersect(tmp.round(), this->getDeviceClipBounds())) {
- devBounds.setEmpty();
- }
- } else {
- devBounds = this->getDeviceClipBounds();
- }
- if (devBounds.isEmpty()) {
- return;
- }
-
SkBaseDevice* device = this->getTopDevice();
if (nullptr == device) { // Do we still need this check???
return;
}
- // need the bounds relative to the device itself
- devBounds.offset(-device->fOrigin.fX, -device->fOrigin.fY);
+ // Map the local bounds into the top device's coordinate space (this is not
+ // necessarily the full global CTM transform).
+ SkIRect devBounds;
+ if (localBounds) {
+ SkRect tmp;
+ device->localToDevice().mapRect(&tmp, *localBounds);
+ if (!devBounds.intersect(tmp.round(), device->devClipBounds())) {
+ devBounds.setEmpty();
+ }
+ } else {
+ devBounds = device->devClipBounds();
+ }
+ if (devBounds.isEmpty()) {
+ return;
+ }
// This is getting the special image from the current device, which is then drawn into (both by
// a client, and the drawClippedToSaveBehind below). Since this is not saving a layer, with its
@@ -1304,14 +1302,13 @@
*/
if (layer) {
if (fMCRec) {
- const SkIPoint& origin = layer->fDevice->getOrigin();
layer->fDevice->setImmutable();
- this->internalDrawDevice(layer->fDevice.get(), origin.x(), origin.y(),
- layer->fPaint.get(),
+ // At this point, 'layer' has been removed from the device stack, so the devices that
+ // internalDrawDevice sees are the destinations that 'layer' is drawn into.
+ this->internalDrawDevice(layer->fDevice.get(), layer->fPaint.get(),
layer->fClipImage.get(), layer->fClipMatrix);
// restore what we smashed in internalSaveLayer
this->internalSetMatrix(layer->fStashedMatrix);
- // reset this, since internalDrawDevice will have set it to true
delete layer;
} else {
// we're at the root
@@ -1389,7 +1386,17 @@
*rowBytes = pmap.rowBytes();
}
if (origin) {
- *origin = this->getTopDevice()->getOrigin();
+ // If the caller requested the origin, they presumably are expecting the returned pixels to
+ // be axis-aligned with the root canvas. If the top level device isn't axis aligned, that's
+ // not the case. Until we update accessTopLayerPixels() to accept a coord space matrix
+ // instead of an origin, just don't expose the pixels in that case. Note that this means
+ // that layers with complex coordinate spaces can still report their pixels if the caller
+ // does not ask for the origin (e.g. just to dump its output to a file, etc).
+ if (this->getTopDevice()->isPixelAlignedToGlobal()) {
+ *origin = this->getTopDevice()->getOrigin();
+ } else {
+ return nullptr;
+ }
}
return pmap.writable_addr();
}
@@ -1409,7 +1416,7 @@
SkASSERT(src == dst);
}
-void SkCanvas::internalDrawDevice(SkBaseDevice* srcDev, int x, int y, const SkPaint* paint,
+void SkCanvas::internalDrawDevice(SkBaseDevice* srcDev, const SkPaint* paint,
SkImage* clipImage, const SkMatrix& clipMatrix) {
SkPaint tmp;
if (nullptr == paint) {
@@ -1424,7 +1431,10 @@
srcDev->imageInfo().colorSpace());
paint = &draw.paint();
SkImageFilter* filter = paint->getImageFilter();
- SkIPoint pos = { x - iter.getX(), y - iter.getY() };
+ // TODO(michaelludwig) - Devices aren't created with complex coordinate systems yet,
+ // so it should always be possible to use the relative origin. Once drawDevice() and
+ // drawSpecial() take an SkMatrix, this can switch to getRelativeTransform() instead.
+ SkIPoint pos = srcDev->getOrigin() - dstDev->getOrigin();
if (filter || clipImage) {
sk_sp<SkSpecialImage> specialImage = srcDev->snapSpecial();
if (specialImage) {
@@ -2344,7 +2354,7 @@
// We use clipRegion because it is already defined to operate in dev-space
// (i.e. ignores the ctm). However, it is going to first translate by -origin,
// but we don't want that, so we undo that before calling in.
- SkRegion rgn(bounds.makeOffset(dev->fOrigin));
+ SkRegion rgn(bounds.makeOffset(dev->getOrigin()));
dev->clipRegion(rgn, SkClipOp::kIntersect);
dev->drawPaint(draw.paint());
dev->restore(fMCRec->fMatrix);
@@ -3097,6 +3107,11 @@
void SkCanvas::LayerIter::next() {
fDone = !fImpl->next();
+ if (!fDone) {
+ // Cache the device origin. LayerIter is only used in Android, which doesn't use image
+ // filters, so its devices will always be able to report the origin exactly.
+ fDeviceOrigin = fImpl->fDevice->getOrigin();
+ }
}
SkBaseDevice* SkCanvas::LayerIter::device() const {
@@ -3119,8 +3134,8 @@
return fImpl->fDevice->getGlobalBounds();
}
-int SkCanvas::LayerIter::x() const { return fImpl->getX(); }
-int SkCanvas::LayerIter::y() const { return fImpl->getY(); }
+int SkCanvas::LayerIter::x() const { return fDeviceOrigin.fX; }
+int SkCanvas::LayerIter::y() const { return fDeviceOrigin.fY; }
///////////////////////////////////////////////////////////////////////////////
@@ -3201,17 +3216,12 @@
if (fAllocator && fMCRec->fTopLayer->fDevice) {
const auto& dev = fMCRec->fTopLayer->fDevice;
SkRasterHandleAllocator::Handle handle = dev->getRasterHandle();
- SkIPoint origin = dev->getOrigin();
- SkMatrix ctm = this->getTotalMatrix();
- ctm.preTranslate(SkIntToScalar(-origin.x()), SkIntToScalar(-origin.y()));
-
- SkIRect clip = fMCRec->fRasterClip.getBounds();
- clip.offset(-origin.x(), -origin.y());
+ SkIRect clip = dev->devClipBounds();
if (!clip.intersect({0, 0, dev->width(), dev->height()})) {
clip.setEmpty();
}
- fAllocator->updateHandle(handle, ctm, clip);
+ fAllocator->updateHandle(handle, dev->localToDevice(), clip);
return handle;
}
return nullptr;
diff --git a/src/core/SkClipStackDevice.cpp b/src/core/SkClipStackDevice.cpp
index d783b09..ab2fcd8 100644
--- a/src/core/SkClipStackDevice.cpp
+++ b/src/core/SkClipStackDevice.cpp
@@ -9,7 +9,7 @@
#include "src/core/SkDraw.h"
#include "src/core/SkRasterClip.h"
-SkIRect SkClipStackDevice::devClipBounds() const {
+SkIRect SkClipStackDevice::onDevClipBounds() const {
SkIRect r = fClipStack.bounds(this->imageInfo().bounds()).roundOut();
if (!r.isEmpty()) {
SkASSERT(this->imageInfo().bounds().contains(r));
diff --git a/src/core/SkClipStackDevice.h b/src/core/SkClipStackDevice.h
index 30abfd8..e7fb96a 100644
--- a/src/core/SkClipStackDevice.h
+++ b/src/core/SkClipStackDevice.h
@@ -21,8 +21,6 @@
SkClipStack& cs() { return fClipStack; }
const SkClipStack& cs() const { return fClipStack; }
- SkIRect devClipBounds() const;
-
protected:
void onSave() override;
void onRestore() override;
@@ -35,6 +33,7 @@
bool onClipIsWideOpen() const override;
void onAsRgnClip(SkRegion*) const override;
ClipType onGetClipType() const override;
+ SkIRect onDevClipBounds() const override;
private:
enum {
diff --git a/src/core/SkDevice.cpp b/src/core/SkDevice.cpp
index ab94f87..fa59895 100644
--- a/src/core/SkDevice.cpp
+++ b/src/core/SkDevice.cpp
@@ -33,28 +33,63 @@
#include "src/utils/SkPatchUtils.h"
SkBaseDevice::SkBaseDevice(const SkImageInfo& info, const SkSurfaceProps& surfaceProps)
- : fInfo(info)
- , fSurfaceProps(surfaceProps)
-{
- fOrigin = {0, 0};
+ : fInfo(info)
+ , fSurfaceProps(surfaceProps) {
+ fDeviceToGlobal.reset();
+ fGlobalToDevice.reset();
fLocalToDevice.reset();
}
-void SkBaseDevice::setOrigin(const SkMatrix& globalCTM, int x, int y) {
- fOrigin.set(x, y);
- fLocalToDevice = globalCTM;
+void SkBaseDevice::setDeviceCoordinateSystem(const SkMatrix& deviceToGlobal,
+ const SkMatrix& localToDevice,
+ int bufferOriginX,
+ int bufferOriginY) {
+ fDeviceToGlobal = deviceToGlobal;
+ fDeviceToGlobal.normalizePerspective();
+ SkAssertResult(deviceToGlobal.invert(&fGlobalToDevice));
+
+ fLocalToDevice = localToDevice;
fLocalToDevice.normalizePerspective();
- fLocalToDevice.postTranslate(SkIntToScalar(-x), SkIntToScalar(-y));
+ if (bufferOriginX | bufferOriginY) {
+ fDeviceToGlobal.preTranslate(bufferOriginX, bufferOriginY);
+ fGlobalToDevice.postTranslate(-bufferOriginX, -bufferOriginY);
+ fLocalToDevice.postTranslate(-bufferOriginX, -bufferOriginY);
+ }
}
void SkBaseDevice::setGlobalCTM(const SkCanvasMatrix& ctm) {
fLocalToDevice = ctm;
fLocalToDevice.normalizePerspective();
- if (fOrigin.fX | fOrigin.fY) {
- fLocalToDevice.postTranslate(-SkIntToScalar(fOrigin.fX), -SkIntToScalar(fOrigin.fY));
+ if (!fGlobalToDevice.isIdentity()) {
+ // Map from the global CTM state to this device's coordinate system.
+ fLocalToDevice.postConcat(fGlobalToDevice);
}
}
+bool SkBaseDevice::isPixelAlignedToGlobal() const {
+ return fDeviceToGlobal.isTranslate() &&
+ SkScalarIsInt(fDeviceToGlobal.getTranslateX()) &&
+ SkScalarIsInt(fDeviceToGlobal.getTranslateY());
+}
+
+SkIPoint SkBaseDevice::getOrigin() const {
+ // getOrigin() is deprecated, the old origin has been moved into the fDeviceToGlobal matrix.
+ // This extracts the origin from the matrix, but asserts that a more complicated coordinate
+ // space hasn't been set of the device. This function can be removed once existing use cases
+ // have been updated to use the device-to-global matrix instead or have themselves been removed
+ // (e.g. Android's device-space clip regions are going away, and are not compatible with the
+ // generalized device coordinate system).
+ SkASSERT(this->isPixelAlignedToGlobal());
+ return SkIPoint::Make(SkScalarFloorToInt(fDeviceToGlobal.getTranslateX()),
+ SkScalarFloorToInt(fDeviceToGlobal.getTranslateY()));
+}
+
+SkMatrix SkBaseDevice::getRelativeTransform(const SkBaseDevice& inputDevice) const {
+ // To get the transform from the input's space to this space, transform from the input space to
+ // the global space, and then from the global space back to this space.
+ return SkMatrix::Concat(fGlobalToDevice, inputDevice.fDeviceToGlobal);
+}
+
SkPixelGeometry SkBaseDevice::CreateInfo::AdjustGeometry(TileUsage tileUsage, SkPixelGeometry geo) {
switch (tileUsage) {
case kPossible_TileUsage:
diff --git a/src/core/SkDevice.h b/src/core/SkDevice.h
index 6bc625d..07b4b61 100644
--- a/src/core/SkDevice.h
+++ b/src/core/SkDevice.h
@@ -50,8 +50,9 @@
*/
void getGlobalBounds(SkIRect* bounds) const {
SkASSERT(bounds);
- const SkIPoint& origin = this->getOrigin();
- bounds->setXYWH(origin.x(), origin.y(), this->width(), this->height());
+ SkRect localBounds = SkRect::MakeIWH(this->width(), this->height());
+ fDeviceToGlobal.mapRect(&localBounds);
+ *bounds = localBounds.roundOut();
}
SkIRect getGlobalBounds() const {
@@ -60,6 +61,14 @@
return bounds;
}
+ /**
+ * Returns the bounding box of the current clip, in this device's
+ * coordinate space. No pixels outside of these bounds will be touched by
+ * draws unless the clip is further modified (at which point this will
+ * return the updated bounds).
+ */
+ SkIRect devClipBounds() const { return this->onDevClipBounds(); }
+
int width() const {
return this->imageInfo().width();
}
@@ -92,10 +101,34 @@
bool peekPixels(SkPixmap*);
/**
- * Return the device's origin: its offset in device coordinates from
- * the default origin in its canvas' matrix/clip
+ * Return the device's coordinate space transform: this maps from the device's coordinate space
+ * into the global canvas' space (or root device space). This includes the translation
+ * necessary to account for the device's origin.
*/
- const SkIPoint& getOrigin() const { return fOrigin; }
+ const SkMatrix& deviceToGlobal() const { return fDeviceToGlobal; }
+ /**
+ * Return the inverse of getDeviceToGlobal(), mapping from the global canvas' space (or root
+ * device space) into this device's coordinate space.
+ */
+ const SkMatrix& globalToDevice() const { return fGlobalToDevice; }
+ /**
+ * DEPRECATED: This asserts that 'getDeviceToGlobal' is a translation matrix with integer
+ * components. In the future some SkDevices will have more complex device-to-global transforms,
+ * so getDeviceToGlobal() or getRelativeTransform() should be used instead.
+ */
+ SkIPoint getOrigin() const;
+ /**
+ * Returns true when this device's pixel grid is axis aligned with the global coordinate space,
+ * and any relative translation between the two spaces is in integer pixel units.
+ */
+ bool isPixelAlignedToGlobal() const;
+ /**
+ * Get the transformation from the input device's to this device's coordinate space. This
+ * transform can be used to draw the input device into this device, such that once this device
+ * is drawn to the root device, the net effect will have the input device's content drawn
+ * transformed by the global CTM.
+ */
+ SkMatrix getRelativeTransform(const SkBaseDevice&) const;
virtual void* getRasterHandle() const { return nullptr; }
@@ -163,6 +196,10 @@
};
virtual ClipType onGetClipType() const = 0;
+ // This should strive to be as tight as possible, ideally not just mapping
+ // the global clip bounds by fToGlobal^-1.
+ virtual SkIRect onDevClipBounds() const = 0;
+
/** These are called inside the per-device-layer loop for each draw call.
When these are called, we have already applied any saveLayer operations,
and are handling any looping from the paint.
@@ -364,8 +401,21 @@
*/
virtual GrRenderTargetContext* accessRenderTargetContext() { return nullptr; }
- // just called by SkCanvas when built as a layer
- void setOrigin(const SkMatrix& ctm, int x, int y);
+ // Configure the device's coordinate spaces, specifying both how its device image maps back to
+ // the global space (via 'deviceToGlobal') and the initial CTM of the device (via
+ // 'localToDevice', i.e. what geometry drawn into this device will be transformed with).
+ //
+ // (bufferOriginX, bufferOriginY) defines where the (0,0) pixel the device's backing buffer
+ // is anchored in the device space. The final device-to-global matrix stored by the SkDevice
+ // will include a pre-translation by T(deviceOriginX, deviceOriginY), and the final
+ // local-to-device matrix will have a post-translation of T(-deviceOriginX, -deviceOriginY).
+ void setDeviceCoordinateSystem(const SkMatrix& deviceToGlobal, const SkMatrix& localToDevice,
+ int bufferOriginX, int bufferOriginY);
+ // Convenience to configure the device to be axis-aligned with the root canvas, but with a
+ // unique origin.
+ void setOrigin(const SkMatrix& globalCTM, int x, int y) {
+ this->setDeviceCoordinateSystem(SkMatrix::I(), globalCTM, x, y);
+ }
/** Causes any deferred drawing to the device to be completed.
*/
@@ -379,9 +429,14 @@
*const_cast<SkImageInfo*>(&fInfo) = fInfo.makeWH(w, h);
}
- SkIPoint fOrigin;
const SkImageInfo fInfo;
const SkSurfaceProps fSurfaceProps;
+ // fDeviceToGlobal and fGlobalToDevice are inverses of each other; there are never that many
+ // SkDevices, so pay the memory cost to avoid recalculating the inverse.
+ SkMatrix fDeviceToGlobal;
+ SkMatrix fGlobalToDevice;
+ // This is the device CTM, not the global CTM. This transform maps from local space to the
+ // device's coordinate space; fDeviceToGlobal * fLocalToDevice will match the canvas' CTM.
SkMatrix fLocalToDevice;
typedef SkRefCnt INHERITED;
@@ -424,6 +479,9 @@
ClipType onGetClipType() const override {
return ClipType::kRect;
}
+ SkIRect onDevClipBounds() const override {
+ return SkIRect::MakeWH(this->width(), this->height());
+ }
void drawPaint(const SkPaint& paint) override {}
void drawPoints(SkCanvas::PointMode, size_t, const SkPoint[], const SkPaint&) override {}
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index 880dd43..3ed86ab 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -302,8 +302,10 @@
if (!value) {
return;
}
- const SkMatrix& pageXform = fDocument->currentPageTransform();
- SkPoint deviceOffset = {(float)this->getOrigin().x(), (float)this->getOrigin().y()};
+ // Annotations are specified in absolute coordinates, so the page xform maps from device space
+ // to the global space, and applies the document transform.
+ SkMatrix pageXform = this->deviceToGlobal();
+ pageXform.postConcat(fDocument->currentPageTransform());
if (rect.isEmpty()) {
if (!strcmp(key, SkPDFGetNodeIdKey())) {
int nodeID;
@@ -313,7 +315,7 @@
return;
}
if (!strcmp(SkAnnotationKeys::Define_Named_Dest_Key(), key)) {
- SkPoint p = deviceOffset + this->localToDevice().mapXY(rect.x(), rect.y());
+ SkPoint p = this->localToDevice().mapXY(rect.x(), rect.y());
pageXform.mapPoints(&p, 1);
auto pg = fDocument->currentPage();
fDocument->fNamedDestinations.push_back(SkPDFNamedDestination{sk_ref_sp(value), p, pg});
@@ -327,8 +329,7 @@
SkClipStack_AsPath(this->cs(), &clip);
Op(clip, path, kIntersect_SkPathOp, &path);
// PDF wants a rectangle only.
- SkRect transformedRect =
- pageXform.mapRect(path.getBounds().makeOffset(deviceOffset.x(), deviceOffset.y()));
+ SkRect transformedRect = pageXform.mapRect(path.getBounds());
if (transformedRect.isEmpty()) {
return;
}