Closed
Bug 1453953
Opened 7 years ago
Closed 7 years ago
regression: Zoom level indicator is ugly in its fade-in animation (choose the right animation scale for blob images)
Categories
(Core :: Graphics: WebRender, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox-esr60 | --- | unaffected |
firefox59 | --- | unaffected |
firefox60 | --- | unaffected |
firefox61 | --- | disabled |
firefox62 | --- | disabled |
firefox63 | --- | verified |
firefox64 | --- | verified |
People
(Reporter: jan, Assigned: Gankra)
References
Details
(Keywords: nightly-community, regression)
Attachments
(1 file, 1 obsolete file)
2.65 MB,
video/mp4
|
Details |
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
Comment 1•7 years ago
|
||
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.
Reporter | ||
Updated•7 years ago
|
Summary: regression: Zoom level indicator is ugly in its fading-in animation → regression: Zoom level indicator is ugly in its fade-in animation
Updated•7 years ago
|
Blocks: stage-wr-nightly
Priority: -- → P1
Updated•7 years ago
|
Assignee: nobody → jmuizelaar
Reporter | ||
Comment 3•7 years ago
|
||
Reporter | ||
Comment 4•7 years ago
|
||
Could this be fixed as part of bug 1397874?
status-firefox62:
--- → disabled
OS: Linux → All
Updated•7 years ago
|
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)
Updated•7 years ago
|
Assignee: jmuizelaar → a.beingessner
Assignee | ||
Comment 5•7 years ago
|
||
MozReview-Commit-ID: 57L6KiYawqA
Comment 6•7 years ago
|
||
Do we know why the fallback was ending up so terrible?
Flags: needinfo?(a.beingessner)
Assignee | ||
Comment 7•7 years ago
|
||
Assignee | ||
Comment 8•7 years ago
|
||
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)
Flags: needinfo?(a.beingessner)
Assignee | ||
Comment 9•7 years ago
|
||
Comment 10•7 years ago
|
||
Comment on attachment 9003329 [details]
Bug 1453953 - natively implement xul textboxes. r?mstange
Markus Stange [:mstange] has approved the revision.
Attachment #9003329 -
Flags: review+
Assignee | ||
Comment 11•7 years ago
|
||
note to sheriffs: there's multiple commits on phabricator but I only see the first in bugzilla?
Keywords: checkin-needed
Comment 12•7 years ago
|
||
(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?
Flags: needinfo?(aryx.bugmail)
![]() |
||
Comment 13•7 years ago
|
||
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?
Flags: needinfo?(aryx.bugmail)
Comment 14•7 years ago
|
||
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.
Comment 15•7 years ago
|
||
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
Flags: needinfo?(a.beingessner)
Updated•7 years ago
|
Keywords: checkin-needed
Comment 16•7 years ago
|
||
Pushed by jmuizelaar@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd278a6e5c73
natively implement xul textboxes. r=mstange
Updated•7 years ago
|
Priority: P2 → P1
Comment 17•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Updated•7 years ago
|
Flags: qe-verify+
Reporter | ||
Comment 18•7 years ago
|
||
Verified fixed in Nightly 63 x64 20180831100058 de_DE @ Debian Testing. Thank you!
Status: RESOLVED → VERIFIED
Flags: needinfo?(a.beingessner)
Updated•7 years ago
|
status-firefox-esr60:
--- → unaffected
Comment 20•7 years ago
|
||
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
Comment 21•7 years ago
|
||
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.
Updated•6 years ago
|
Attachment #9003329 -
Attachment is obsolete: true
You need to log in
before you can comment on or make changes to this bug.
Description
•