Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1264133003:
SkCanvas::onDrawPicture() quick-reject (Closed)
DescriptionSkCanvas::onDrawPicture() quick-reject
R=reed@google.com,mtklein@google.com
Committed: https://skia.googlesource.com/skia/+/d028180f0310cf2b23f9744256a41697b0683e67
Patch Set 1 #Patch Set 2 : don't ignore matrix #Patch Set 3 : matrix-mapped bounds #Messages
Total messages: 25 (6 generated)
I noticed SkCanvas::onDrawPicture doesn't quick-reject like the other base primitive overrides even though the cull rect seems readily available. I guess most of the time an RTree will take care of culling, but IIRC slimming paint records at least some pictures without a BBH. Does this look like something we'd want for completeness?
The CQ bit was checked by fmalita@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264133003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264133003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm. pray it doesn't break layout tests, or have you tested that? (I am sherriff, and our rolls have been happy today, so feel free to cowboy this one.)
On 2015/08/03 22:08:40, mtklein wrote: > lgtm. pray it doesn't break layout tests, or have you tested that? (I am > sherriff, and our rolls have been happy today, so feel free to cowboy this one.) Layout tests look good locally, let's give it a try.
The CQ bit was checked by fmalita@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264133003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264133003/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://skia.googlesource.com/skia/+/48ed62b29d45e42b971aac8858da06781c93e6d7
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/1273433002/ by mtklein@google.com. The reason for reverting is: webkit_unit_tests :(.
Message was sent while issue was closed.
On 2015/08/04 01:08:13, mtklein wrote: > A revert of this CL (patchset #3 id:40001) has been created in > https://codereview.chromium.org/1273433002/ by mailto:mtklein@google.com. > > The reason for reverting is: webkit_unit_tests :(. Here are the failing rolls: https://codereview.chromium.org/1266053004 https://codereview.chromium.org/1260213005
Message was sent while issue was closed.
On 2015/08/04 01:17:23, mtklein wrote: > On 2015/08/04 01:08:13, mtklein wrote: > > A revert of this CL (patchset #3 id:40001) has been created in > > https://codereview.chromium.org/1273433002/ by mailto:mtklein@google.com. > > > > The reason for reverting is: webkit_unit_tests :(. > > Here are the failing rolls: > https://codereview.chromium.org/1266053004 > https://codereview.chromium.org/1260213005 webkit_unit_tests fix landed (https://codereview.chromium.org/1303453003/), let's try this again.
The CQ bit was checked by fmalita@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264133003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264133003/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://skia.googlesource.com/skia/+/d3d07245e29504dbffa0083e84ace5bab85853d4
Message was sent while issue was closed.
mtklein@chromium.org changed reviewers: + mtklein@chromium.org
Message was sent while issue was closed.
Nice, looks like 5-7% overall win on .skps: https://perf.skia.org/#3937
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/1301973002/ by herb@google.com. The reason for reverting is: Seems to be breaking some blink tests. https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/....
Message was sent while issue was closed.
On 2015/08/19 14:20:42, mtklein_C wrote: > Nice, looks like 5-7% overall win on .skps: https://perf.skia.org/#3937 Heh, just in time for the second revert :) (this time some FEImage layout test failures - https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...)
Message was sent while issue was closed.
On 2015/08/19 14:49:12, f(malita) wrote: > On 2015/08/19 14:20:42, mtklein_C wrote: > > Nice, looks like 5-7% overall win on .skps: https://perf.skia.org/#3937 > > Heh, just in time for the second revert :) > > (this time some FEImage layout test failures - > https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...) FEImage cull rect fix in the queue: https://codereview.chromium.org/1298283003/ I'll wait for it to roll, and then give this thing a third shove.
Message was sent while issue was closed.
On 2015/08/20 16:10:53, f(malita) wrote: > On 2015/08/19 14:49:12, f(malita) wrote: > > On 2015/08/19 14:20:42, mtklein_C wrote: > > > Nice, looks like 5-7% overall win on .skps: https://perf.skia.org/#3937 > > > > Heh, just in time for the second revert :) > > > > (this time some FEImage layout test failures - > > > https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_rel/...) > > FEImage cull rect fix in the queue: https://codereview.chromium.org/1298283003/ > > I'll wait for it to roll, and then give this thing a third shove. sweet
The CQ bit was checked by fmalita@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1264133003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1264133003/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://skia.googlesource.com/skia/+/d028180f0310cf2b23f9744256a41697b0683e67 |
