Closed Bug 1453953 Opened 2 years ago Closed Last year

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)

x86_64
All
defect

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: darkspirit, Assigned: Gankra)

References

Details

(Keywords: nightly-community, regression)

Attachments

(1 file, 1 obsolete file)

Attached video 2018-04-13_15-50-42.mp4
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
Assignee: nobody → jmuizelaar
XUL scaling doesn't sound like it should be P1
Priority: P1 → P2
Could this be fixed as part of bug 1397874?
OS: Linux → All
Blocks: 1337552
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)
Assignee: jmuizelaar → a.beingessner
MozReview-Commit-ID: 57L6KiYawqA
Do we know why the fallback was ending up so terrible?
Flags: needinfo?(a.beingessner)
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)
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?
Keywords: checkin-needed
(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)
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)
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
Flags: needinfo?(a.beingessner)
Pushed by jmuizelaar@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd278a6e5c73
natively implement xul textboxes. r=mstange
Priority: P2 → P1
https://hg.mozilla.org/mozilla-central/rev/dd278a6e5c73
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Verified fixed in Nightly 63 x64 20180831100058 de_DE @ Debian Testing. Thank you!
Status: RESOLVED → VERIFIED
Flags: needinfo?(a.beingessner)
Duplicate of this bug: 1413271
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.
Flags: qe-verify+
Attachment #9003329 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.