Closed Bug 1453953 Opened 3 years ago Closed 3 years ago
regression: Zoom level indicator is ugly in its fade-in animation (choose the right animation scale for blob images)
mozregression --good 2018-04-01 --bad 2018-04-10 --pref gfx.webrender.all:true startup.homepage_welcome_url:'https://www.reddit.com/r/firefox/' > 11:32.04 INFO: Last good revision: 48e344feb212edf648879380b877151fb4c29ec6 > 11:32.04 INFO: First bad revision: 0c721699dba5fdcf8d1fafe9aeb845532349bb59 > 11:32.04 INFO: Pushlog: > https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=48e344feb212edf648879380b877151fb4c29ec6&tochange=0c721699dba5fdcf8d1fafe9aeb845532349bb59 > 0c721699dba5 Kartikaya Gupta — Bug 1419851 - Handle OMTA throttling for webrender. r=jrmuizel
This text is using XUL text which is still rendered using fallback. So when we rasterize the fallback image, we're probably using a bad scale / resolution. If there's an animation, FrameLayerBuilder chooses a scale based on the animation by calling ComputeDesiredDisplaySizeForAnimation(aContainerFrame). We should probably do something similar in WebRenderCommandsBuilder.
Summary: regression: Zoom level indicator is ugly in its fading-in animation → regression: Zoom level indicator is ugly in its fade-in animation
3 years ago
Priority: -- → P1
XUL scaling doesn't sound like it should be P1
Priority: P1 → P2
Summary: regression: Zoom level indicator is ugly in its fade-in animation → regression: Zoom level indicator is ugly in its fade-in animation (choose the right animation scale for blob images)
Do we know why the fallback was ending up so terrible?
No I hadn't figured it out, this just seemed like the better solution to the exact issue (and we haven't seen this anywhere else)
Comment on attachment 9003329 [details] Bug 1453953 - natively implement xul textboxes. r?mstange Markus Stange [:mstange] has approved the revision.
Attachment #9003329 - Flags: review+
note to sheriffs: there's multiple commits on phabricator but I only see the first in bugzilla?
(In reply to Alexis Beingessner [:Gankro] from comment #11) > note to sheriffs: there's multiple commits on phabricator but I only see the > first in bugzilla? That phabricator patch has included all commits, that;s why you see only one here. Aryx, is that correct?
No. https://phabricator.services.mozilla.com/p/Gankro/ mentions only https://phabricator.services.mozilla.com/D4010 for this bug but also for bug 1412179. Are the other patches on phabricator?
No, what Andreea said is correct: One phabricator "revision" can encompass multiple commits. You can see the list of commits if you go to https://phabricator.services.mozilla.com/D4010 , scroll down to "Revision Contents", and click the "Commits" tab. So if you land this phabricator revision, all your changes for this bug will be included. But I think they'll be squashed down to a single mercurial changeset. Alexis, if you want to create distinct phabricator revisions for the individual commits, you'll either need to instruct arcanist to submit them one by one, or you need to use moz-phab (from https://github.com/mozilla-conduit/review ) to submit multiple revisions with a single command.
Backed out changeset 641eedc51cc0 (bug 1453953)for reftest failures on tests/layout/reftests/text-shadow/basic.xul Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/4ac24eebd69c1cfb79e889bdac3785870bbceeab Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=641eedc51cc0c78dff8d00788f1ea72dc967eae8 Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=195936698&repo=mozilla-inbound&lineNumber=25861 Log snippet: 13:19:23 INFO - REFTEST TEST-START | file:///Users/cltbld/tasks/task_1535215997/build/tests/reftest/tests/layout/reftests/text-shadow/basic.xul == file:///Users/cltbld/tasks/task_1535215997/build/tests/reftest/tests/layout/reftests/text-shadow/basic-ref.xul 13:19:23 INFO - REFTEST TEST-LOAD | file:///Users/cltbld/tasks/task_1535215997/build/tests/reftest/tests/layout/reftests/text-shadow/basic.xul | 1 / 32 (3%) 13:19:23 INFO - REFTEST TEST-LOAD | file:///Users/cltbld/tasks/task_1535215997/build/tests/reftest/tests/layout/reftests/text-shadow/basic-ref.xul | 1 / 32 (3%) 13:19:23 INFO - REFTEST INFO | REFTEST fuzzy test (1, 20) <= (0, 0) <= (1, 20) 13:19:23 INFO - REFTEST TEST-UNEXPECTED-PASS | file:///Users/cltbld/tasks/task_1535215997/build/tests/reftest/tests/layout/reftests/text-shadow/basic.xul == file:///Users/cltbld/tasks/task_1535215997/build/tests/reftest/tests/layout/reftests/text-shadow/basic-ref.xul | image comparison, max difference: 0, number of differing pixels: 0
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/dd278a6e5c73 natively implement xul textboxes. r=mstange
Verified fixed in Nightly 63 x64 20180831100058 de_DE @ Debian Testing. Thank you!
Status: RESOLVED → VERIFIED
Either this bug or bug 1452426 brought this slight perf improvement: == Change summary for alert #15535 (as of Thu, 30 Aug 2018 20:55:55 GMT) == Improvements: 3% displaylist_mutate windows10-64-qr opt e10s stylo 7,643.19 -> 7,418.22 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=15535
I can confirm that this issue is verified fixed using Firefox 64.0a1 (BuildId:20180906232139) and Firefox 63.0b3 (BuildId:20180904170936) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 16.04 64bit.
You need to log in before you can comment on or make changes to this bug.