Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2587433003:
remove call to deprecated preroll() -- cc manages triggering/caching decodes (Closed)
Descriptionremove call to deprecated preroll() -- cc manages triggering/caching decodes
I just now (after posting this) see https://bugs.chromium.org/p/chromium/issues/detail?id=595090
Calling preroll() is (1) deprecated, and (2) is no guarantee that the bits will hang around, since the cache is volatile. If the desire is to "force" the codec to produce a raster, then lest just ask the codec to do that (i.e. *not* be lazy).
thoughts?
BUG=
Committed: https://crrev.com/baed7149f0c86f55c32999e749386aac630745b3
Cr-Commit-Position: refs/heads/master@{#439878}
Patch Set 1 #
Messages
Total messages: 22 (12 generated)
reed@google.com changed reviewers: + fmalita@chromium.org, junov@chromium.org, zakerinasab@chromium.org
Description was changed from ========== remove call to deprecated preroll() -- cc manages triggering/caching decodes BUG= ========== to ========== remove call to deprecated preroll() -- cc manages triggering/caching decodes I just now (after posting this) see https://bugs.chromium.org/p/chromium/issues/detail?id=595090 Calling preroll() is (1) deprecated, and (2) is no guarantee that the bits will hang around, since the cache is volatile. If the desire is to "force" the codec to produce a raster, then lest just ask the codec to do that (i.e. *not* be lazy). thoughts? BUG= ==========
The CQ bit was checked by reed@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== remove call to deprecated preroll() -- cc manages triggering/caching decodes I just now (after posting this) see https://bugs.chromium.org/p/chromium/issues/detail?id=595090 Calling preroll() is (1) deprecated, and (2) is no guarantee that the bits will hang around, since the cache is volatile. If the desire is to "force" the codec to produce a raster, then lest just ask the codec to do that (i.e. *not* be lazy). thoughts? BUG= ========== to ========== remove call to deprecated preroll() -- cc manages triggering/caching decodes I just now (after posting this) see https://bugs.chromium.org/p/chromium/issues/detail?id=595090 Calling preroll() is (1) deprecated, and (2) is no guarantee that the bits will hang around, since the cache is volatile. If the desire is to "force" the codec to produce a raster, then lest just ask the codec to do that (i.e. *not* be lazy). thoughts? If we really want a raster image, would an API that forces that be better? If so, I would feel fine adding something like SkImage* SkImage::forceToRaster(); Which would return a new image that was the raster version of the original, or itself if it was already raster... BUG= ==========
Description was changed from ========== remove call to deprecated preroll() -- cc manages triggering/caching decodes I just now (after posting this) see https://bugs.chromium.org/p/chromium/issues/detail?id=595090 Calling preroll() is (1) deprecated, and (2) is no guarantee that the bits will hang around, since the cache is volatile. If the desire is to "force" the codec to produce a raster, then lest just ask the codec to do that (i.e. *not* be lazy). thoughts? If we really want a raster image, would an API that forces that be better? If so, I would feel fine adding something like SkImage* SkImage::forceToRaster(); Which would return a new image that was the raster version of the original, or itself if it was already raster... BUG= ========== to ========== remove call to deprecated preroll() -- cc manages triggering/caching decodes I just now (after posting this) see https://bugs.chromium.org/p/chromium/issues/detail?id=595090 Calling preroll() is (1) deprecated, and (2) is no guarantee that the bits will hang around, since the cache is volatile. If the desire is to "force" the codec to produce a raster, then lest just ask the codec to do that (i.e. *not* be lazy). thoughts? BUG= ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/12/16 15:55:53, reed1 wrote: lgtm. Is there something we can replace this with to avoid regressing performance?
Do we have a real world measure of the perf, or was this speculative? Certainly calling preroll was costing time itself.
On 2016/12/20 00:30:32, reed1 wrote: > Do we have a real world measure of the perf, or was this speculative? Certainly > calling preroll was costing time itself. It was speculative. The idea was to compensate for the fact that the analysis canvas (or whatever it is we use to collect images for triggering async decode) does not recurse into SkPictureImageFilter, and therefore fails to scan 2D canvas content (in display list form)
ok. I'd be happy to look at any profiles we get of this code path in the future, to optimize whatever needs to be optimized. Are you ok with landing this as is?
Yes. lgtm
FWIW, most canvas use cases re-use the same images over and they rarely scroll. So triggering async decode is not as important here as it is for long scrolling pages with lots of images.
The CQ bit was checked by reed@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1482257551447950, "parent_rev":
"b1270640d88a21e686b29c305ae64149a9bf8ab5", "commit_rev":
"579a348c2c1a59c26d898e6bbd930778bf0d3aed"}
Message was sent while issue was closed.
Description was changed from ========== remove call to deprecated preroll() -- cc manages triggering/caching decodes I just now (after posting this) see https://bugs.chromium.org/p/chromium/issues/detail?id=595090 Calling preroll() is (1) deprecated, and (2) is no guarantee that the bits will hang around, since the cache is volatile. If the desire is to "force" the codec to produce a raster, then lest just ask the codec to do that (i.e. *not* be lazy). thoughts? BUG= ========== to ========== remove call to deprecated preroll() -- cc manages triggering/caching decodes I just now (after posting this) see https://bugs.chromium.org/p/chromium/issues/detail?id=595090 Calling preroll() is (1) deprecated, and (2) is no guarantee that the bits will hang around, since the cache is volatile. If the desire is to "force" the codec to produce a raster, then lest just ask the codec to do that (i.e. *not* be lazy). thoughts? BUG= Review-Url: https://codereview.chromium.org/2587433003 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== remove call to deprecated preroll() -- cc manages triggering/caching decodes I just now (after posting this) see https://bugs.chromium.org/p/chromium/issues/detail?id=595090 Calling preroll() is (1) deprecated, and (2) is no guarantee that the bits will hang around, since the cache is volatile. If the desire is to "force" the codec to produce a raster, then lest just ask the codec to do that (i.e. *not* be lazy). thoughts? BUG= Review-Url: https://codereview.chromium.org/2587433003 ========== to ========== remove call to deprecated preroll() -- cc manages triggering/caching decodes I just now (after posting this) see https://bugs.chromium.org/p/chromium/issues/detail?id=595090 Calling preroll() is (1) deprecated, and (2) is no guarantee that the bits will hang around, since the cache is volatile. If the desire is to "force" the codec to produce a raster, then lest just ask the codec to do that (i.e. *not* be lazy). thoughts? BUG= Committed: https://crrev.com/baed7149f0c86f55c32999e749386aac630745b3 Cr-Commit-Position: refs/heads/master@{#439878} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/baed7149f0c86f55c32999e749386aac630745b3 Cr-Commit-Position: refs/heads/master@{#439878} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
