1374224 - SVG toolbar icons load after the button is shown on the toolbar

Status: NEW

(Core :: SVG, defect, P3)

Description

[:Gijs (out for now; he/him)]

Attachment: icons-missing-svg-load.ogg

See video for context.

This is contributing to perceived UI messiness on startup for me (see also bug 1374216). My default nightly profile also has the library and sidebar icons in the main toolbar (which we'll eventually make the default for 57). It looks like those buttons, which aren't in the navbar by default in the browser.xul markup, when they get added, get painted with only their padding size and a 0x0 space for the icon, until the icon loads. This is especially visible right now because of bug 1372954, which causes a dark blob to show up for the sidebar button, without a loaded icon.

This shouldn't happen. We should only paint once the icon is loaded. At the point where the buttons are added, the CSS that defines all these icons is clearly loaded (because the icons of the other buttons are present). It's not clear to me why we allow painting of this incomplete UI state. This didn't use to happen with toolbar icons as pngs, perhaps because they were sprites - but AIUI the prevailing wisdom for SVG icons is that we don't want sprites, so it's not clear how to achieve the same result here.

:jwatt, do you have ideas / suggestions?

Comment 1

[Florian Quèze [:florian]]

(In reply to :Gijs from comment #0)

> It looks like those buttons, which aren't in the navbar by default in the
> browser.xul markup, when they get added, get painted with only their padding
> size and a 0x0 space for the icon, until the icon loads.

It's especially this part that contributes to perceived flickering, as it causes the rest of the toolbar content to move when we finally get the right size. If the actual icon is painted a couple frames later than the rest of the UI, it's not as bad. Of course if painting everything at once is possible without regressing first paint time, it's better.

Comment 2

[Jonathan Watt [:jwatt]]

Was this regressed by bug 1347543 or something else?

It seems like the elements that reference the SVG files should have their width defined independently of the images they reference exactly so this doesn't happen. As far as I know that's standard good practice, but maybe we stopped doing that or omitted to add code to do it when switching away from -moz-image-region (if that's what caused it).

Comment 3

[Jonathan Watt [:jwatt]]

Both RasterImage and VectorImage fire events to the main event loop to do their decoding, so switching to SVG per se shouldn't add any delay over PNG. Prior to bug 1347543 we were painting -moz-image-region regions out of one large image instead of painting multiple small images though. It may be that prior to that bug landing we had a similar delay, but because they were regions of the same physical image all the icons would essentially become available at the same time, and the "expansion" of all the icons would occur together just once, rather than incrementally as each icon becomes available.

Comment 4

[:Gijs (out for now; he/him)]

(In reply to Florian Quèze [:florian] [:flo] from comment #1)
> without regressing first paint time, it's better.

I don't think this is feasible - we're not doing enough work right now before paint, so of necessity, first paint will need to happen later in order for this to be done. I don't think that should bother us - we could decrease the first paint time a lot by just painting a lot less stuff, but that would clearly not be acceptable. Likewise, I don't think this state should be acceptable even if fixing it regresses tpaint.

Comment 5

[:Gijs (out for now; he/him)]

Attachment: Icons from mozregression profile in 2017-01-01 build

(In reply to Jonathan Watt [:jwatt] from comment #2)
> Was this regressed by bug 1347543 or something else?

The short answer is I'm not sure. I marked this as a regression because now I visually see jumping in the navbar, which is at least in part because of bug 1374216.

However, given your comment I tried to bisect. I started from Jan 1st of this year, and funnily enough, when screen recording, you can see similar issues in older builds that were still using spritesheet pngs. I'll attach two other videos to try to help clarify.

Comment 6

[:Gijs (out for now; he/him)]

Attachment: Icons from mozregression profile in 2017-03-27 build

Comment 7

[:Gijs (out for now; he/him)]

Attachment: 01-01-developer-and-pocket-missing.jpg

In the screencast from the 03-27 build, you can see that at least devtools and pocket are both still missing by the time the window has animated into position (thanks, OS X), though it's only for a fraction of a second.

In the screencast from the 01-01 build, it seems like pocket is the only problem (and it has its own CSS files and images, so that is vaguely understandable) - but if you look very closely, there is a point in the video while the window is still animating in, where you can just see the tops of the icons in the navbar, and you can see there is a gap between the character encoding and sidebar button - but no devtools icon in sight. I've attached a snapshot from the video for clarity.

Comment 8

[:Gijs (out for now; he/him)]

So the TL;DR is that it *feels* like this is now worse than before, but I can't think of a reasonable way of putting numbers / hard data to that. Inasmuch as the image loads and first paint are effectively racing, I don't know if that is even possible.


More generally, what could we do to avoid this, beyond setting the sizes of the relevant icons?