Instead of inverting 'deviceToGlobal', we pass the invert to the
function. 0.1% of all cpu time (M1 Mac) for Motionmark1.2 is spent
doing this inversion.
We can remove a matrix inversion in setDeviceCoordinateSystem by
taking advantage of the fact that we can easily compute the
inverted matrix from values available on hand. The code previously
computed:
fGlobalToDevice = (priorDeviceToGlobal * newLayerMappingLayerToDevice)'
We have access to the following values:
newLayerMappingLayerToDevice' = newLayerMappingDeviceToLayer
priorDeviceToGlobal' = priorGlobalToDevice
With the matrix property (A * B)' = B' * A', we can calculate:
fGlobalToDevice = newLayerMappingDeviceToLayer * priorGlobalToDevice
Change-Id: I39656f244fa5f907536d09d69f585f09f156f133
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527505
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
SkCanvas and SkDevice were using SkM44 and its definition of invert(),
but it was slightly more generous than SkMatrix::invert() so the fuzzer
caught a case where the layer's SkDevice had a valid transform but then
converting it to a SkMatrix in skif::Mapping was no longer invertible.
This modifies it so that skif::Mapping no longer tries to invert the
matrices. In almost all cases, the inverse of the layer-to-device
matrix can be constructed directly from a matrix multiply (that's what
device->getRelativeTransform() does). When the matrices are
ill-conditioned the constructed inverse may be inaccurate (hence why
SkMatrix::invert reports false), but in practice this happens for
ridiculously large transforms and the error isn't significant compared
to the precision range of the matrices anyways.
Other cases explicitly want to use the identity matrix for the layer
to device matrix, so I added a helper in the few places that would have
had to pass SkMatrix::I() twice instead.
The last case is drawImage() that creates its own skif::Mapping, now it
just calculates the inverse that skif::Mapping() would have done and if
it fails it drops the draw since it means the canvas matrix is bad.
Bug: chromium:1276525
Change-Id: Ib516bb2fac19d5e7397bd27d80f8e3932b25b2e2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/509396
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Very originally, skif::Mapping::DecomposeCTM() and
SkBaseDevice::setDeviceCoordinateSystem assumed that if the canvas
matrix was invertible, then any scale decomposition would produce a
valid device coordinate system. This proved not to be true and fuzzers
quickly caught it, but I had attempted to address it by forcing
SkCanvas to do extra work so that the above two functions remained
unchanged.
However, it's become apparent that even making the assumption that the
product of two invertible matrices remains invertible does not always
hold true in the wonderful world of floating point math.
Instead, this rewrites DecomposeCTM and setDeviceCoordinateSystem to
return bools, allowing them to fail. This cleans up some of the earlier
checks that SkCanvas makes while computing the skif::Mapping, and it
also ensures that once we fold in the prior device's transform, the
net layer->global transform remains valid. If any of this fails, it
just gets rid of the new device and sets the clip to empty, basically
preventing drawing until the invalid layer has been restored.
Bug: chromium:1239968, chromium:1240685
Change-Id: Ib9ce8f95859e726a9eacf1154f6eef8dd3995500
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442017
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This reverts commit 8f924ac0ce.
Reason for revert: suppressions landed for fuchsia images to rebaseline
Original change's description:
> Revert "Add new virts, hide old ones"
>
> This reverts commit c56e2e5aa6.
>
> Reason for revert: suspected of breaking chrome roll
>
> Original change's description:
> > Add new virts, hide old ones
> >
> > Add virtuals for the draw methods that now take sampling/filtermode.
> >
> > drawImage
> > drawImageRect
> > drawImageLattice
> > drawAtlas
> >
> > Add a flag that can remove the older virtuals, once each client has
> > stopped overriding them. In that situation, the older public methods
> > will simplify extract the sampling from the paint, and call the new
> > public methods.
> >
> > Bug: skia:11105, skia:7650
> > Change-Id: I8b0029727295caa983e8148fc743a55cfbecd043
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347022
> > Commit-Queue: Mike Reed <reed@google.com>
> > Reviewed-by: Florin Malita <fmalita@chromium.org>
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
>
> TBR=bsalomon@google.com,fmalita@chromium.org,reed@google.com
>
> Change-Id: I0a90952c11a180d918126ea06a630f4a0bf9b49b
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:11105
> Bug: skia:7650
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348194
> Reviewed-by: Derek Sollenberger <djsollen@google.com>
> Commit-Queue: Derek Sollenberger <djsollen@google.com>
TBR=djsollen@google.com,bsalomon@google.com,fmalita@chromium.org,reed@google.com
# Not skipping CQ checks because this is a reland.
Bug: skia:11105
Bug: skia:7650
Change-Id: Ia2b4537a2d330460b7554278d2c05075cf27162a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348876
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
This reverts commit c56e2e5aa6.
Reason for revert: suspected of breaking chrome roll
Original change's description:
> Add new virts, hide old ones
>
> Add virtuals for the draw methods that now take sampling/filtermode.
>
> drawImage
> drawImageRect
> drawImageLattice
> drawAtlas
>
> Add a flag that can remove the older virtuals, once each client has
> stopped overriding them. In that situation, the older public methods
> will simplify extract the sampling from the paint, and call the new
> public methods.
>
> Bug: skia:11105, skia:7650
> Change-Id: I8b0029727295caa983e8148fc743a55cfbecd043
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347022
> Commit-Queue: Mike Reed <reed@google.com>
> Reviewed-by: Florin Malita <fmalita@chromium.org>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,fmalita@chromium.org,reed@google.com
Change-Id: I0a90952c11a180d918126ea06a630f4a0bf9b49b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11105
Bug: skia:7650
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348194
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
Add virtuals for the draw methods that now take sampling/filtermode.
drawImage
drawImageRect
drawImageLattice
drawAtlas
Add a flag that can remove the older virtuals, once each client has
stopped overriding them. In that situation, the older public methods
will simplify extract the sampling from the paint, and call the new
public methods.
Bug: skia:11105, skia:7650
Change-Id: I8b0029727295caa983e8148fc743a55cfbecd043
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347022
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Brian Salomon <bsalomon@google.com>
... and lots and lots of IWYU
Change-Id: Ie5157dcdd2e6d29b95c71b39153278ab48ef4eb3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346778
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
I experimented with passing in a rectangle and computing a geometric
mean of the scale factors for the 4 corners and center. The downside
to that approach is that callers either know the parameter-space bounds
or the device-space bounds. In the latter case, we have to map by the
inverse CTM, which opens up a can of worms. In practice it seemed using
just the center point worked out just as well.
This also updates the sample to draw the axes in the layer space instead
of parameter space. I found this helps display the scale effects of the
parameter-to-layer matrix better.
Bug: skia:9074
Change-Id: I855c85cdbe1072c451aa3a0601571f2e137e5203
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/327624
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Bug: skia:9074
Change-Id: Icdac64276e0a403950fd990a619d523b0ee784de
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/326942
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
For clarity, this renames skif::Mapping::Make to skif::Mapping::DecomposeCTM
to really emphasize when it should be used over just a constructor.
Renames SampleBackdropBounds to SampleFilterBounds since the operations and
coordinate spaces that it visualizes are the same for regular or backdrop
filtering (we just swap the notion of src and dst devices, really).
Technically, this is not quite true yet since regular filtering modifies the
DAG with a matrix transform right now, but that's going away soon (tm).
The "new" SampleFilterBounds example is updated to use the new bounds APIs
that use the coord-space safe types in the skif namespace. It also visualizes
a filter, and simplifies some of the bounds being drawn and reported to
only those most interesting.
The SampleImageFilterDAG has been updated to match the soon-to-be state
of how SkCanvas processes regular image filters for a saveLayer. All
implicit matrix-transform node code is removed. The bounds calculation
code is similarly updated to use the new 'skif' types and functions. To
simplify the visualization of each node, the 'isolated' versions of the
bounds were dropped.
Bug: skia:9282, skia:9283
Change-Id: If2ad2c302e5165ae009bba93cea52bf0566a543a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/326718
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
