Closed
Bug 1071769
Opened 10 years ago
Closed 9 years ago
Use DrawTargetTiled on B2G
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
RESOLVED
FIXED
mozilla42
People
(Reporter: nical, Assigned: nical)
References
Details
Attachments
(5 files)
1.27 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
914 bytes,
patch
|
bas.schouten
:
review+
|
Details | Diff | Splinter Review |
4.76 KB,
patch
|
bas.schouten
:
review+
|
Details | Diff | Splinter Review |
1.27 KB,
patch
|
bas.schouten
:
review+
|
Details | Diff | Splinter Review |
5.41 KB,
patch
|
bas.schouten
:
review+
|
Details | Diff | Splinter Review |
On Mac we observed that DrawTargetTiled yields better performance than the single paint buffer method (which we use on B2G and Fennec). The same is probably true for B2G and Fennec since they use software content rendering. Let's find out.
Assignee | ||
Comment 1•10 years ago
|
||
try push https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e4d918901231
Comment 2•10 years ago
|
||
There are three test failures. These two can be fuzzed: layout/reftests/forms/input/range/stepDown-unthemed.html layout/reftests/forms/input/range/stepDown.html And this one looks very bad: layout/reftests/position-dynamic-changes/horizontal/leftA-widthN-rightA.html?padding_abspos It looks like your push didn't contain the patch from bug 1063046. Can you fire off another one from a more recent mozilla-central revision? Hopefully bug 1063046 will have fixed the latter test failure.
Assignee | ||
Comment 3•10 years ago
|
||
new try push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f4a6279a1456
Assignee | ||
Comment 4•10 years ago
|
||
With a bit of fuzzing this try push shows up green on b2g https://tbpl.mozilla.org/?tree=Try&rev=b36bd826a511 (still need a bit of work on android, though)
Comment 5•10 years ago
|
||
Exciting stuff. \o/
Assignee | ||
Comment 6•10 years ago
|
||
Assignee: nobody → nical.bugzilla
Attachment #8502488 -
Flags: review?(roc)
Attachment #8502488 -
Flags: review?(roc) → review+
Assignee | ||
Comment 7•10 years ago
|
||
Attachment #8503040 -
Flags: review?(bas)
Updated•10 years ago
|
Attachment #8503040 -
Flags: review?(bas) → review+
Assignee | ||
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ac5fed37830 https://hg.mozilla.org/integration/mozilla-inbound/rev/c61c91ea40fe
https://hg.mozilla.org/mozilla-central/rev/8ac5fed37830 https://hg.mozilla.org/mozilla-central/rev/c61c91ea40fe
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 10•10 years ago
|
||
Backed out for causing bug 1081349 and a crap-ton of dupes. https://hg.mozilla.org/integration/mozilla-inbound/rev/1c77f152f280 https://hg.mozilla.org/releases/mozilla-aurora/rev/c79ad6f6a4bb
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla35 → ---
Assignee | ||
Comment 12•9 years ago
|
||
for a tile to be a placeholder, it would have to have both its front and back buffer to be null, but we'd only discard one of the two, making a placeholder look like it wasn't one.
Attachment #8538489 -
Flags: review?(bas)
Assignee | ||
Comment 13•9 years ago
|
||
Reftest analyser for the issue: http://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=http://ftp.mozilla.org/pub/mozilla.org/b2g/try-builds/nsilva@mozilla.com-bfd3368b6675/try-emulator/try_ubuntu64_vm-b2g-emulator_test-reftest-11-bm116-tests1-linux64-build513.txt.gz&only_show_unexpected=1
Attachment #8538490 -
Flags: review?(bas)
Assignee | ||
Comment 14•9 years ago
|
||
try push https://treeherder.mozilla.org/#/jobs?repo=try&revision=11e12fe9cea8
Comment 15•9 years ago
|
||
Comment on attachment 8538489 [details] [diff] [review] Fix DrawTargetTiled Review of attachment 8538489 [details] [diff] [review]: ----------------------------------------------------------------- ::: modules/libpref/init/all.js @@ +3902,5 @@ > pref("layers.draw-tile-borders", false); > pref("layers.draw-bigimage-borders", false); > pref("layers.frame-counter", false); > pref("layers.enable-tiles", false); > +pref("layers.tiled-drawtarget.enabled", true); I expected you didn't mean to have this hunk in this patch.
Attachment #8538489 -
Flags: review?(bas) → review+
Updated•9 years ago
|
Attachment #8538490 -
Flags: review?(bas) → review+
Assignee | ||
Comment 16•9 years ago
|
||
(In reply to Bas Schouten (:bas.schouten) from comment #15) > Comment on attachment 8538489 [details] [diff] [review] > Fix DrawTargetTiled > > Review of attachment 8538489 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: modules/libpref/init/all.js > @@ +3902,5 @@ > > pref("layers.draw-tile-borders", false); > > pref("layers.draw-bigimage-borders", false); > > pref("layers.frame-counter", false); > > pref("layers.enable-tiles", false); > > +pref("layers.tiled-drawtarget.enabled", true); > > I expected you didn't mean to have this hunk in this patch. oops, indeed i didn't.
Assignee | ||
Comment 17•9 years ago
|
||
Unfortunately there are some new regressions in TiledDrawTarget on B2G (with and without my patches), as shown in this try push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=5b31edbc8ce3 A few days ago R11 and R14 were the only two issues (see https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=99955634391f ).
Assignee | ||
Comment 18•9 years ago
|
||
This was a mistake from Bug 1108164. There is no more code paths where we are supposed to pass aCanRerasterizeValidRegion=true now that per-tile drawing is gone, so let's remove it. This should fix the issue (waiting for try results)
Attachment #8540164 -
Flags: review?(bas)
Updated•9 years ago
|
Attachment #8540164 -
Flags: review?(bas) → review+
Assignee | ||
Comment 19•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/002a5ed085bd https://hg.mozilla.org/integration/mozilla-inbound/rev/6cf334e6acf8 https://hg.mozilla.org/integration/mozilla-inbound/rev/f3249b2ec440 https://hg.mozilla.org/integration/mozilla-inbound/rev/342ab0e1b537
Comment 20•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/002a5ed085bd https://hg.mozilla.org/mozilla-central/rev/6cf334e6acf8 https://hg.mozilla.org/mozilla-central/rev/f3249b2ec440 https://hg.mozilla.org/mozilla-central/rev/342ab0e1b537
Status: REOPENED → RESOLVED
Closed: 10 years ago → 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment 21•9 years ago
|
||
Backed out the enabling chunk in https://hg.mozilla.org/integration/mozilla-inbound/rev/3a86099219cb - still getting reftest-14 and -15 failures in position-dynamic-changes/, around 10% of the runs.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Summary: Use DrawTagetTiled on B2G → Use DrawTargetTiled on B2G
Assignee | ||
Comment 23•9 years ago
|
||
Comment on attachment 8540164 [details] [diff] [review] We can never rerasterize the valid region Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: black artifact described in bug 1079996 [Describe test coverage new/current, TBPL]: has been baking on nightly for a week or so. [Risks and why]: low, the affected code path is plain wrong without the patch. [String/UUID change made/needed]:
Attachment #8540164 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Attachment #8540164 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 24•9 years ago
|
||
Comment on attachment 8540164 [details] [diff] [review] We can never rerasterize the valid region Review of attachment 8540164 [details] [diff] [review]: ----------------------------------------------------------------- Nevermind, the bug that caused the regression which this fixes hasn't made it to beta.
Attachment #8540164 -
Flags: approval-mozilla-aurora+
Are all the remaining pieces of this bug B2G only? This comment https://bugzilla.mozilla.org/show_bug.cgi?id=1079996#c15 is suggesting that we're expecting some goodness to come to other platforms, so should we change the summary?
Assignee | ||
Comment 26•9 years ago
|
||
(In reply to Milan Sreckovic [:milan] from comment #25) > Are all the remaining pieces of this bug B2G only? This comment > https://bugzilla.mozilla.org/show_bug.cgi?id=1079996#c15 is suggesting that > we're expecting some goodness to come to other platforms, so should we > change the summary? This tracks the work that's needed to get DrawTargetTiled on b2g specifically, but along the way I have found some issues that, once fixed, can make things better for other platforms (Mac is the only one shipping DrawTargetTiled for now, but It may get enabled on android eventually too, and I think that the plan is to get windows xp tiling directly through DrawTargetTiled). Bug 1071772 tracks DrawTaretTiled on android.
Right - I was interested in us mentioning this helping a Mac bug 1064236 and bug 1079996.
Updated•9 years ago
|
status-firefox37:
--- → ?
Comment 28•9 years ago
|
||
(In reply to Nicolas Silva [:nical] from comment #26) > This tracks the work that's needed to get DrawTargetTiled on b2g > specifically, but along the way I have found some issues that, once fixed, > can make things better for other platforms (Mac is the only one shipping > DrawTargetTiled for now, but It may get enabled on android eventually too, > and I think that the plan is to get windows xp tiling directly through > DrawTargetTiled). This description makes it sound to me like we do not need to take a fix in 37. As such, I'm not tracking this bug (was suggested in bug 1079996 that referenced this one).
status-firefox38:
--- → affected
https://hg.mozilla.org/mozilla-central/rev/90b07ab2ae5b
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: mozilla37 → mozilla42
You need to log in
before you can comment on or make changes to this bug.
Description
•