1517198 - Toolbox icons are rendered after labels, with a noticeable lag

Status: NEW

(DevTools :: General, defect, P3)

Description

[Florens Verschelde :fvsch]

Attachment: devtools-toolbox-open-close.mov

I’m working on the Toolbox icons in bug 1517189 and noticing that when opening devtools the tab icons consistently show up after the tab labels. This happens in Nightly (regardless of my patch) too.

This happens both for `<img alt="" src="some-icon.svg">` and for icons defined in CSS as pseudo-elements with a background-image.

Do we know if that’s an inevitable cost for SVG, or all images, or `-moz-context-fill` maybe?

Play the attached video frame-by-frame if you want to see it clearly, but I find it noticeable at normal speed too.

Comment 1

[Florens Verschelde :fvsch]

:jdescottes mentioned bug 1472938, so I'm wondering if Brian has some information on what could be done to speed up rendering these icons.

Comment 2

[Brian Birtles (:birtles)]

You can try the approach in bug 1472938 which is to use the chrome-only getCSSImageURLs() function then preload the images using `const image = new Image(); image.src = url`. That should certainly help with the background-images.

Furthermore, for SVG images, we have an image cache for them so it should work for them too. (From memory Seth Fowler did that work for Firefox OS but unfortunately he's no longer around. CC'ing Daniel Holbert since he probably knows that area best.)

However, I suppose in your case the issue is that you need to preload even before DevTools is activated? I suspect Julian would know if there's somewhere suitable for doing that.

But to answer your other question, I don't think flicker is an inevitable cost of using SVG or -moz-context-fill since we use it a lot in Firefox front-end. Jaws probably knows the most about this area. Using React does make it a little trickier however since presumably the nodes aren't added to the DOM until the last moment.

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to Brian Birtles (:birtles) from comment #2)
> Furthermore, for SVG images, we have an image cache for them so it should
> work for them too.

Correct - this is called the "surface cache", where we cache rasterized versions of SVG images, and it should Just Work with context-fill etc (the context colors are part of the cache key that's used).

I don't think there's anything SVG-specific here, actually -- I see the same behavior with the PNG "HTTPS Everywhere" icon on devtools (for the label that appears if you install https://addons.mozilla.org/en-US/firefox/addon/https-everywhere/ ).

That icon is not SVG -- it's a PNG -- and it shows the same behavior.

So this might just be a general XUL:image behavior, perhaps? [EDIT: sorry, this isn't a XUL:image, never mind]

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: screencast with PNG https-everywhere icon included (which shows the same issue)

Comment 5

[Daniel Holbert [:dholbert]]

If you adjust the devtools/client/themes/toolbox.css rule for ".devtools-tab > img"  to have...
>    border: 2px solid purple;
...then it reveals a bit more.

This shows that these images aren't merely failing to paint in that first render -- rather, their frame is being drawn at 0 height in this initial paint. This suggests we don't know their intrinsic size, which I think means imagelib is waiting on image data.  And that might mean we're bypassing the image cache, though I'm not entirely sure.  Might want to have tnikkel or aosmond take a look to see what can be done, since they're most recently active in imagelib stuff AFAIK.

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: screencast in debug build w/ border added around img elements

Here's a screencast to illustrate my previous comment. (This is in a debug build where this issue is particularly obvious.)

Comment 7

[Florens Verschelde :fvsch]

Attachment: screencast with border around tab images and one base64 icon (console)

I’m not sure the intrinsic height thing has a big impact.

In bug 1517189 we're defining the image's width and height explicitly, so that it's always 16x16, but the icon still appears late.

For this test I’ve also added the Console icon as a base64-encoded string in https://searchfox.org/mozilla-central/source/devtools/client/definitions.js#104
Then it renders at the same time as the label.

That could be a possible optimization for the tab icons, but it seems that using context-fill will not work in that case. So we would have to inline the SVG images in that JS component and use React's dangerouslySetInnerHTML.
And maybe preloading the images would work better anyway.

Comment 8

[Daniel Holbert [:dholbert]]

(In reply to Florens Verschelde [:fvsch] from comment #7)
> I’m not sure the intrinsic height thing has a big impact.
> 
> In bug 1517189 we're defining the image's width and height explicitly, so
> that it's always 16x16, but the icon still appears late.

Sorry if I wasn't clear -- the point of my observations about the intrinsic height was simply this: when this issue happens, the problem is that **we're still waiting on the image data to load** (we don't even have the first few bytes that'd tell us the image size for the PNG-image case).   So, it's not a "painting is slow" sort of issuu -- instead, it's a "we don't even know how big the image is yet, presumably because imagelib is still waiting on the image data to arrive" sort of issue. And I think that means we're missing the imagelib cache entirely, each time devtools (re)opens and tries to display these icons.

That seems like something we should try to fix, if possible.  (Shot in the dark: maybe this is related to how HTTPS sites can't share cached images to avoid data leakage? Maybe we purge or bypass the previously-cached versions of these images because the new devtools and the old devtools aren't obviously same-origin, or something?)

> For this test I’ve also added the Console icon as a base64-encoded string [...]
> Then it renders at the same time as the label.

Yeah, it makes sense that that'd help here, if the problem is us waiting on image data to asynchronously arrive.

> but it seems that using context-fill will not work in that case

Really? I'd expect that it should work...  (But generally I agree that it's nicer if we can use actual image files for these SVGs, to avoid issues that we run into sometimes with long data-URLs and to avoid potential duplication of data, among other reasons.)

> And maybe preloading the images would work better anyway.

Preloading sounds like it could be the easiest way to address this, yeah.