about:newtab with its noisy background image combined with a gradient, and its thumbnails implemented as background images is quite slow on my Retina MBP. Opening a new tab has a noticeable delay. Closing new tabs and switching to them shows the same issues. On a retina screen, the tabopen animation is completely skipped when the browser is maximized. The animation improves a little for smaller windows. All these symptoms are not present on my other, less powerful machines with lower resolutions. CC'ing roc to see if there's anything we can do about this.

Removing the noisy background image and using <img> elements for the thumbnails yields a huge improvement, especially for maximized windows. We unfortunately lose the background-size:cover functionality as that's not available for images.

I wonder if this is something we could do in case fixing background image painting is too hard. We could hook this up to something like -moz-image-sizing:cover/contain. This seems to work fine but it should probably automatically set a clipping rect to not draw outside of the image frame.

Here's a profile of opening a single about:newtab page, that has been preloaded in the background: http://people.mozilla.com/~bgirard/cleopatra/#report=fee988ad2fea957e7f55947d4797cbe773e09317

We've actually got the optimization mentioned in bug 761009 implemented now, but it only takes effect under some conditions. In this case we're skipping it because the backgrounds are being clipped to the padding box rather than the border box. I can't think of any reason why we couldn't fix this case though. We also require that the scaled image exactly covers the border area. This is because we haven't implemented support for tiling, or preventing sampling outside the intended area when drawing part of an image. The latter is the case we're hitting here, background-size:cover is causing the bottom 15 pixels of the image to be cut off. I'm not sure how easy it is to handle this correctly when drawing with the GPU.

I guess we're scaling the background image and/or the thumbnails. Can we avoid that by providing the background image at higher resolution (via a media query say) so it doesn't need to be scaled? That would look better too. Same goes for the thumbnails. We should be taking the snapshots at 2x resolution so they don't need to be scaled and they look better.

I'm not sure what you mean sorry. I think the problem is that the thumbnail isn't the same aspect ratio as the div we're trying to use it as a background for. We scale the thumbnail to the same width as the div, but preserving aspect ratio. This leaves us with the thumbnail being 15pixels taller than the div (at least with my setup), and we fallback to the CPU to do the clipping.

I think with "contain" and "cover" we can assume no atlasing/spriting and allow ourselves to sample from anywhere in the image. That should fix this.

(In reply to Matt Woodrow (:mattwoodrow) from comment #6) > ... This leaves us with > the thumbnail being 15pixels taller than the div (at least with my setup), The size and aspect ratio of the div depend on the window size and aspect ratio.

I'm not sure if this will help performance much without more work. I've verified that we create ImageLayers but for the static new-tab page I guess they'll be in inactive opacity containers so they'll still not be stretched and composited on the GPU. For that we need something more like GPUImageScalingEnabled().

It seems like the patch does help a little bit, only. As I just found out, the biggest win comes from actually removing the background image and the linear gradient entirely. I wonder if there's something we can do about that? Is a 48x48 sprite too expensive? Should we have a bigger tile?

Aha! So it *is* the page background after all, eh?

What does the linear gradient actually do? It doesn't seem to do anything useful as far as I can tell. Have you tried providing a 2x resolution background image so on Retina displays we don't have to scale the tile?

My patch here causes a test failure in B2G R10: https://tbpl.mozilla.org/php/getParsedLog.php?id=27288561&tree=Try&full=1

Comment on attachment 797078 [details] [diff] [review] Allow ImageLayerization for images using 'contain' and 'cover' Review of attachment 797078 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/nsDisplayList.cpp @@ +1910,5 @@ > nsDisplayBackgroundImage::BuildLayer(nsDisplayListBuilder* aBuilder, > LayerManager* aManager, > const ContainerParameters& aParameters) > { > + nsRefPtr<ImageLayer> layer = aManager->CreateImageLayer(); Why this change? We don't want to create new layers every paint.

Comment on attachment 797078 [details] [diff] [review] Allow ImageLayerization for images using 'contain' and 'cover' Review of attachment 797078 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/nsDisplayList.cpp @@ +1910,5 @@ > nsDisplayBackgroundImage::BuildLayer(nsDisplayListBuilder* aBuilder, > LayerManager* aManager, > const ContainerParameters& aParameters) > { > + nsRefPtr<ImageLayer> layer = aManager->CreateImageLayer(); This change is wrong. I'll revert it.

See comment #12.

Sorry for the delay. Yes, a 2x resolution bg image helps a lot. It seems like we should still look into replacing the bg images as the newtab animation is a lot more smooth when that is done, too. Maybe attachment 797078 [details] [diff] [review] would also help as well with that? I can't tell because it unfortunately doesn't apply anymore.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #12) > What does the linear gradient actually do? It doesn't seem to do anything > useful as far as I can tell. I have no idea, I removed them and we'll see if shorlander agrees. This patch also adds a 2x resolution bg image. Stephen, can you please tell if there's a reason to keep those gradients? Also, you may want to provide a better @2x version of the bg image? I'll attach the one from the patch, too.

The bg image with 2x the resolution.

Comment on attachment 810444 [details] [diff] [review] Remove bg gradients and provide a 2x resolution bg image for hi-dpi screens Review of attachment 810444 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Tim Taubert [:ttaubert] from comment #18) > Created attachment 810444 [details] [diff] [review] > Remove bg gradients and provide a 2x resolution bg image for hi-dpi screens > > (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #12) > > What does the linear gradient actually do? It doesn't seem to do anything > > useful as far as I can tell. > > I have no idea, I removed them and we'll see if shorlander agrees. > > This patch also adds a 2x resolution bg image. > > Stephen, can you please tell if there's a reason to keep those gradients? > Also, you may want to provide a better @2x version of the bg image? I'll > attach the one from the patch, too. If we remove the gradient I would like to lighten the whole page. I think we can just remove the noise texture entirely. Although if we do it on the about:newtab page we should also do it to about:home. background-color: hsl(0,0%,95%) should be light enough. Thanks!

(In reply to Stephen Horlander [:shorlander] from comment #20) > If we remove the gradient I would like to lighten the whole page. I think we > can just remove the noise texture entirely. Although if we do it on the > about:newtab page we should also do it to about:home. > > background-color: hsl(0,0%,95%) should be light enough. Will do, thanks for the quick response!

This patch removes the noise.png background images (and gradients) from about:newtab and about:home. I move the newtab/noise.png images to the devtools folder as it turned out that devtools was referencing it.

Comment on attachment 811526 [details] [diff] [review] Remove noise backgrounds for about:newtab and about:home > #newtab-scrollbox:not([page-disabled]) { >- background-color: rgb(229,229,229); >- background-image: url(chrome://browser/skin/newtab/noise.png), >- linear-gradient(rgba(255,255,255,.5), rgba(255,255,255,.2)); >- background-attachment: fixed; >+ background-color: hsl(0,0%,95%); > } Please add color:black here, even though a custom color is used for various (hopefully all) text-related elements. Better safe than sorry.

(In reply to Dão Gottwald [:dao] from comment #23) > Please add color:black here, even though a custom color is used for various > (hopefully all) text-related elements. Better safe than sorry. Will do, thanks!

(In reply to Tim Taubert [:ttaubert] from comment #22) > ... I move the newtab/noise.png images to the > devtools folder as it turned out that devtools was referencing it. Maybe we could also do the same trick on devtools, if it improves rendering performance?

(In reply to Avi Halachmi (:avih) from comment #27) > (In reply to Tim Taubert [:ttaubert] from comment #22) > > ... I move the newtab/noise.png images to the > > devtools folder as it turned out that devtools was referencing it. > > Maybe we could also do the same trick on devtools, if it improves rendering > performance? The connect page (the page the uses this image) is disabled by default. Bug filed: bug 921835

Comment on attachment 811526 [details] [diff] [review] Remove noise backgrounds for about:newtab and about:home [Approval Request Comment] Bug caused by (feature/regressing bug #): None. User impact if declined: Shitty tabopen animation on hi-res displays. Testing completed (on m-c, etc.): Landed on m-c. Risk to taking this patch (and alternatives if risky): Low risk patch that only changes styles. String or IDL/UUID changes made by this patch: None. In terms of performance, it would be great if we could uplift this to Aurora and Beta. On Retina displays, the tabopen animation is almost back to previous performance. I don't know though if we usually uplift visual changes like this to Beta? It seems like a minor change (we'll have a lighter background on about:newtab and about:home) and the perf win is very noticeable.

Comment on attachment 811526 [details] [diff] [review] Remove noise backgrounds for about:newtab and about:home This appears to be a very minor visual change, so we'll take the perf win on branches (although this doesn't meet typical uplift criteria).

Thanks! https://hg.mozilla.org/releases/mozilla-aurora/rev/e9b9a61a3236 https://hg.mozilla.org/releases/mozilla-beta/rev/efb226e93d5e

Not sure why this bug is still open.

If attachment 797078 [details] [diff] [review] would improve things even more it would be great to have that rebased and maybe landed as well?

It caused a test failure, see comment #13. If it's not important I don't want to spend time on it right now.

Ok, it's not super important but it would be great to address this rather sooner than later. We've made a big improvement by removing the background image/gradient but the animation is still not as smooth as it can be. Filed bug 927228 so that we can fix it later.

Tim can you or anyone else with a hi-dpi screen please check that this is verified on Firefox 26 beta 1 and latest Aurora 27.0a2?

This is definitely fixed, we're doing much better now on Retina. We're still not on par with images (instead of bg images) but I'm hoping for bug 927228.

I don't have a retina display, but do have a hi-dpi Surface Pro. In full screen, New Tab Open/Close responsiveness is good with smooth automation.