1453953 - regression: Zoom level indicator is ugly in its fade-in animation (choose the right animation scale for blob images)

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect, P1)

Description

[Jan Andre Ikenmeyer [:darkspirit]]

Attachment: 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

Comment 1

[Markus Stange [:mstange]]

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.

Comment 2

[Kartikaya Gupta (email:[e-mail]) (away until Feb-2020)]

XUL scaling doesn't sound like it should be P1

Comment 3

[Jan Andre Ikenmeyer [:darkspirit]]

https://searchfox.org/mozilla-central/search?q=urlbar-zoom-button

Comment 4

[Jan Andre Ikenmeyer [:darkspirit]]

Could this be fixed as part of bug 1397874?

Comment 5

[Alexis Beingessner [:Gankra]]

Attachment: Bug 1453953 - natively implement xul textboxes. r?mstange

MozReview-Commit-ID: 57L6KiYawqA

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Do we know why the fallback was ending up so terrible?

Comment 7

[Alexis Beingessner [:Gankra]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=67cf7a496253cff6fd60893f1336438394793e26

Comment 8

[Alexis Beingessner [:Gankra]]

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 9

[Alexis Beingessner [:Gankra]]

try2 https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb81bb13f9839ec632e8966dd5faab79e1d81203

Comment 10

[Markus Stange [:mstange]]

Comment on attachment 9003329 [details]
Bug 1453953 - natively implement xul textboxes. r?mstange

Markus Stange [:mstange] has approved the revision.

Comment 11

[Alexis Beingessner [:Gankra]]

note to sheriffs: there's multiple commits on phabricator but I only see the first in bugzilla?

Comment 12

[Andreea Pavel [:apavel]]

(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?

Comment 13

[Sebastian Hengst [:aryx] (needinfo on intermittent or backout)]

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?

Comment 14

[Markus Stange [:mstange]]

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

[Raul Gurzau (:RaulG)]

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

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd278a6e5c73
natively implement xul textboxes. r=mstange

Comment 17

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/dd278a6e5c73

Comment 18

[Jan Andre Ikenmeyer [:darkspirit]]

Verified fixed in Nightly 63 x64 20180831100058 de_DE @ Debian Testing. Thank you!

Comment 20

[Ionuț Goldan [:igoldan], Performance Sheriff]

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

[Emil Ghitta, QA [:emilghitta]]

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.