Closed Bug 1016434 Opened 5 years ago Closed 5 years ago

Blurry loading throbbers after Bug 759252

Categories

(Firefox :: Theme, defect)

defect
Not set
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 35
Iteration:
35.3
Tracking Status
firefox31 --- unaffected
firefox32 + verified
firefox33 --- verified
firefox34 --- verified
firefox35 --- verified

People

(Reporter: ntim, Assigned: jaws, NeedInfo)

References

Details

(Keywords: regression)

Attachments

(3 files)

Attached image Urgh
As you see in the screenshot, it looks terribly blurry :(
No longer blocks: 759252
Blocks: 759252
Component: Developer Tools → Theme
I can confirm this behaviour on Firefox 32 beta 1.

OS : Windows 8.1 64 bits
[Tracking Requested - why for this release]:
Philipp - Can you please review and comment on the urgency of fixing this from a UX perspective?
Flags: needinfo?(philipp)
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?
Flags: needinfo?(philipp) → needinfo?(shorlander)
Thanks Philipp. Tracking for 32. If possible, it is preferable to get this resolved this week.
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.
Flags: needinfo?(shorlander)
Wr could back out bug 759252
(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
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?
Flags: needinfo?(jaws)
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?
Flags: needinfo?(jaws) → needinfo?(lmandel)
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?
Flags: needinfo?(lmandel) → needinfo?(jaws)
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
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attachment #8473867 - Flags: approval-mozilla-beta?
Flags: needinfo?(jaws)
Comment on attachment 8473867 [details] [diff] [review]
Backout of bug 759252

Approval Request Comment: See above comment.
Attachment #8473867 - Flags: approval-mozilla-aurora?
Jared, are you planning to commit that in m-c too for 34? Thanks
Flags: needinfo?(jaws)
(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.
Flags: needinfo?(jaws)
Flags: firefox-backlog+
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?
Attachment #8473867 - Flags: approval-mozilla-beta?
Attachment #8473867 - Flags: approval-mozilla-beta+
Attachment #8473867 - Flags: approval-mozilla-aurora?
Attachment #8473867 - Flags: approval-mozilla-aurora+
Flags: needinfo?(bugs)
(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
Flags: needinfo?(bugs)
(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.
Flags: needinfo?(dbaron)
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.
Stephen, could you attach a static image of the throbber at 96dpi?
Flags: needinfo?(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.
Flags: needinfo?(shorlander)
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
Stephen, what do you think about comment #24?
Flags: needinfo?(shorlander)
(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?
Flags: needinfo?(shorlander)
Per comment #26.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Summary: Blurry loading throbbers after Bug 759252 → Blurry loading throbbers after Bug 759252 in Firefox 32 and 33
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.
Yeah, keeping it open sounds reasonable.

I'll unassign myself from it though since I'm not working on it.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Summary: Blurry loading throbbers after Bug 759252 in Firefox 32 and 33 → Blurry loading throbbers after Bug 759252
Assignee: jaws → nobody
Status: REOPENED → NEW
Flags: qe-verify?
Depends on: 1059456
Depends on: 1062857
Flags: qe-verify? → qe-verify+
backout landed on nightly based on the discussion in bug 1059557.

https://hg.mozilla.org/integration/fx-team/rev/4af29b25a7bf
Comment on attachment 8473867 [details] [diff] [review]
Backout of bug 759252

see comment 13

Requesting approval again for the current aurora (Firefox 34).
Attachment #8473867 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/4af29b25a7bf
Assignee: nobody → jaws
Status: NEW → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Iteration: --- → 35.3
Points: --- → 2
QA Contact: florin.mezei
(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?
Flags: needinfo?(dao)
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.
(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.
Flags: needinfo?(dao)
Comment on attachment 8473867 [details] [diff] [review]
Backout of bug 759252

Let's back this out of 34. Aurora+
Attachment #8473867 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified on Win 7 x64, in today's Firefox 34 (BuildID=20141009004002) and the issue now no longer shows in Aurora.
Depends on: 1080841
Depends on: 1081496
You need to log in before you can comment on or make changes to this bug.