1016434 - Blurry loading throbbers after Bug 759252

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Tim Nguyen :ntim]

Attachment: Urgh

As you see in the screenshot, it looks terribly blurry :(

Comment 1

[Arcturus66]

I can confirm this behaviour on Firefox 32 beta 1.

OS : Windows 8.1 64 bits

Comment 2

[Loic]

Attachment: blurry-icon.jpg

Regression range:
good=2014-05-22
bad=2014-05-23
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b40296602083&tochange=e9b2b72f4e6c

Comment 3

[Loic]

[Tracking Requested - why for this release]:

Comment 4

[Lawrence Mandel [:lmandel] (use needinfo)]

Philipp - Can you please review and comment on the urgency of fixing this from a UX perspective?

Comment 5

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Looks like something went wrong with the compression of that graphic…?
If it's just the asset and we can uplift that to beta, we should definitely do that.

Stephen, could you have a look at this?

Comment 6

[Lawrence Mandel [:lmandel] (use needinfo)]

Thanks Philipp. Tracking for 32. If possible, it is preferable to get this resolved this week.

Comment 7

[Stephen Horlander [:shorlander]]

The images look fine.

Probably has something to do with using CSS animations to spin the image: bug 759252.

Looks like it is making it jaggy and creating artifacts. It might also be better or worse depending on hardware/GPU.

Comment 8

[Tim Nguyen :ntim]

Wr could back out bug 759252

Comment 9

[Tim Nguyen :ntim]

(In reply to Tim Nguyen [:ntim] from comment #8)
> We could back out bug 759252

At least for beta and aurora until we find a fix

Comment 10

[Lawrence Mandel [:lmandel] (use needinfo)]

Jared - You fixed bug 759252. Do you have an idea of what's gone wrong to cause the behaviour noted in this bug? What do you think of backing out bug 759252 on 32 to resolved this issue?

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

I'm not sure what is causing it, but we may need to wait for bug 980770 to get fixed before shipping this.

Lawrence, can you follow up with people to find out what the expected timeline is for bug 980770?

Comment 12

[Lawrence Mandel [:lmandel] (use needinfo)]

Jared - I'll follow up on bug 980770. However, unless we want to ship blurry throbbers in 32, we need another solution next week for beta. Do you have a recommendation? Should we consider a backout of bug 759252?

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Backout of bug 759252

Approval Request Comment
[Feature/regressing bug #]: bug 759252 introduced new throbbers but some people see them as blurry
[User impact if declined]: see above
[Describe test coverage new/current, TBPL]: no test coverage, visual differences
[Risks and why]: none expected, near-simple backout (https://hg.mozilla.org/releases/mozilla-beta/rev/1979a1b89b46 landed on top of it, but that didn't change much)
[String/UUID change made/needed]: none

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8473867 [details] [diff] [review]
Backout of bug 759252

Approval Request Comment: See above comment.

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Jared, are you planning to commit that in m-c too for 34? Thanks

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #15)
> Jared, are you planning to commit that in m-c too for 34? Thanks

I'd like to wait to hear the response that Lawrence got for bug 980770 before determining if we should back out from m-c.

Comment 17

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8473867 [details] [diff] [review]
Backout of bug 759252

Approved for Aurora and Beta.

Looking at the dependencies for bug 980770, I see 4 bugs that haven't had any recent action. 
Jet - When we plan to resolve bug 980770 will have an impact on the branches on which we land this backout. Do you have a target release for OMT animations?

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/de49ce4ac5f5
https://hg.mozilla.org/releases/mozilla-beta/rev/3741e9a5c6ca

Comment 19

[Jet Villegas (inactive)]

(In reply to Lawrence Mandel [:lmandel] from comment #17)

> Jet - When we plan to resolve bug 980770 will have an impact on the branches
> on which we land this backout. Do you have a target release for OMT
> animations?

I'm not sure why this bug is blocked on OMT Animations. How bitmaps are rasterized during animation shouldn't be affected. That is, how they are painted should be the same as it is today. You can try this out by setting layers.offmainthreadcomposition.async-animations=true in about:config

Comment 20

[Dão Gottwald [:dao]]

(In reply to Jet Villegas (:jet) from comment #19)
> (In reply to Lawrence Mandel [:lmandel] from comment #17)
> 
> > Jet - When we plan to resolve bug 980770 will have an impact on the branches
> > on which we land this backout. Do you have a target release for OMT
> > animations?
> 
> I'm not sure why this bug is blocked on OMT Animations.

Probably because the "blurriness" is minor compared to the performance & smoothness win promised by off-main-thread animations.

Comment 21

[Ricardo]

Two ideas I'm not sure were tested:

1. The static image 'chrome://browser/skin/tabbrowser/loading.png'
in 32b7 (prior to backout) is saved with 96 dpi,
in 32b8 and previous is not dpi-fixed (typically used in web).

2. Using 'image-rendering' CSS property might improve or make the blurriness worse.
Even though I don't reproduce the 'blurriness' in the tab's loading icon that caused this backout ('Urgh' attachment above), when I tried 'image-rendering: -moz-crisp-edges' in a test page rendering the same icon caused a similar effect.

Comment 22

[Jared Wein [:jaws] (please needinfo? me)]

Stephen, could you attach a static image of the throbber at 96dpi?

Comment 23

[Stephen Horlander [:shorlander]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #22)
> Stephen, could you attach a static image of the throbber at 96dpi?

I am not sure why this would matter. A 16 x 16 pixel image is 16 x 16 pixels no matter what the DPI is.

Comment 24

[Ricardo]

loading_animated-noDPI-31.0.png: http://goo.gl/9pmQgL
loading_static-96dpi-32.0b7.png: http://goo.gl/EWFMAB
loading_static-resaved-noDPI.png: http://goo.gl/FRQ6Jc
loading-icon.zip (all 3 files): http://goo.gl/NLrXbt

Comment 25

[Jared Wein [:jaws] (please needinfo? me)]

Stephen, what do you think about comment #24?

Comment 26

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #15)
> Jared, are you planning to commit that in m-c too for 34? Thanks

Let's keep this on m-c for the time being so that people can report when this bug no longer occurs. I'm also waiting to see what dbaron comments about based on his self-appointed needinfo.

Clearing needinfo from shorlander since we discussed over IRC that the icon likely isn't the cause of the blurriness. Ricardo, can you please file a new bug to make sure that we are using a high quality icon here?

Comment 27

[Jared Wein [:jaws] (please needinfo? me)]

Per comment #26.

Comment 28

[Dão Gottwald [:dao]]

Why are you closing this? Are you going to file a new bug, since this is still unaddressed on mozilla-central? Even if we want to leave it unaddressed for the time being, we should keep track of it as a bug.

Comment 29

[Jared Wein [:jaws] (please needinfo? me)]

Yeah, keeping it open sounds reasonable.

I'll unassign myself from it though since I'm not working on it.

Comment 30

[Dão Gottwald [:dao]]

backout landed on nightly based on the discussion in bug 1059557.

https://hg.mozilla.org/integration/fx-team/rev/4af29b25a7bf

Comment 31

[Dão Gottwald [:dao]]

Comment on attachment 8473867 [details] [diff] [review]
Backout of bug 759252

see comment 13

Requesting approval again for the current aurora (Firefox 34).

Comment 32

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4af29b25a7bf

Comment 33

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to Dão Gottwald [:dao] from comment #31)
> Comment on attachment 8473867 [details] [diff] [review]
> Backout of bug 759252
> 
> see comment 13
> 
> Requesting approval again for the current aurora (Firefox 34).

This is just a request to backout the change on 34, where we had left it enabled for further testing, right?

Comment 34

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

I was able to reproduce the original issue on Win 7 x64 on Nightly from 2014-05-23 and latest Aurora. The issue looks just like depicted in https://bug1016434.bugzilla.mozilla.org/attachment.cgi?id=8466936, and is even more visible while the throbber is animated.

Verified that the issue no longer shows on Win 7 x64 with:
- Firefox 32.0.3 - BuildID=20140923175406
- Firefox 33 RC - BuildID=20141007073543
- Firefox 35 Nightly - BuildID=20141008030202

While the fix is easy to spot and verify on Windows, I could not see anything wrong with latest Aurora on Ubuntu 13.04 x64 or Mac OS X 10.9.5, so I cannot confirm the fix there, but then again I haven't seen anything reported for these Operating Systems.

I'm marking this as verified for now.

Comment 35

[Dão Gottwald [:dao]]

(In reply to Lawrence Mandel [:lmandel] from comment #33)
> (In reply to Dão Gottwald [:dao] from comment #31)
> > Comment on attachment 8473867 [details] [diff] [review]
> > Backout of bug 759252
> > 
> > see comment 13
> > 
> > Requesting approval again for the current aurora (Firefox 34).
> 
> This is just a request to backout the change on 34, where we had left it
> enabled for further testing, right?

We had it enabled while waiting for bug 980770, which still isn't ready.

Comment 36

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8473867 [details] [diff] [review]
Backout of bug 759252

Let's back this out of 34. Aurora+

Comment 37

[Dão Gottwald [:dao]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8811060bf3fe

Comment 38

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Verified on Win 7 x64, in today's Firefox 34 (BuildID=20141009004002) and the issue now no longer shows in Aurora.