1556156 - White background during image loading

Status: NEW

(Core :: Graphics: WebRender, defect, P1)

Description

[Dominic Szablewski]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:67.0) Gecko/20100101 Firefox/67.0

Steps to reproduce:

Loading a non-progressive JPEG image

Actual results:

Firefox 67 now shows a white background in the yet-unloaded region of a (non-progressive) JPEG image. Earlier versions of Firefox didn't show anything in the unloaded region.

Expected results:

The background of the yet-unloaded image region should be transparent. Safari, Chrome and earlier Firefox version have this behavior.

Test case with screenshots:
https://phoboslab.org/files/bugs/firefox-image-loading/

Comment 1

[Trevor Rowbotham [:rowbot]]

Works as expected with webrender disabled in the latest nightly.

Last good: 2017-11-23
First bad: 2017-11-24
Push log: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bdb2387396b4a74dfefb7c983733eed3625e906a&tochange=e8ce4865e42125850e6371eba3c5e71cf5ab6d29

Comment 2

[Pascal Chevrel:pascalc]

Marking as wontfix for 67 as it doesn't seem like a major issue that would require taking the risk of an uplift from mozilla-central to mozilla-release.

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

I'm pretty sure this is a regression from bug 1183378. Fixing it may be somewhat difficult, depending on the approach. We start JPEGs in BGRX, so that's why an incomplete JPEG is white. We would either need to defer drawing background images until the image is complete (I thought there was some logic surrounding this but...) or start JPEGs in BGRA and upgrade to BGRX once decoding has finished.

Comment 4

[Liz Henry (:lizzard) (relman/hg->git project)]

Too late for a fix in 68 but we could still take a patch in 69.

Comment 5

[Liz Henry (:lizzard) (relman/hg->git project)]

Happy to take a patch for 70 but since this is triaged and set to P3 priority I'm setting it as fix-optional.
That will remove the bug from weekly regression triage.

Comment 6

[Zdenek Jindra]

Version 88 and they still didn't fix it? Please do, it's an annoying bug. How do you want to compete with Chrome? I'd fix it myself if I could.

Comment 7

[Milan Laslop]

The problem does not happen when using

<svg viewBox="0 0 width height">
  <image x="0" y="0" width="width" height="height" href="src"></image>
</svg>

instead of plain

<img width="width" height="height" src="src" />

I only accidentally found this workaround when trying to solve something else.

My browser info:

96.0.3 (64-bit)
Linux

Comment 8

[cloakengaged]

"Trevor Rowbotham [:rowbot] : "Works as expected with webrender disabled.""

---

Can we expect a fix for this bug before ESR 102 lands and forces Webrender to everyone?

This one is especially annoying when using pop-up image enlargers like Imagus, Mouseover popup image viewers etc.

Turns entire screen to white while loading images and ruins the dark mode experience.

see an example here : https://www.reddit.com/r/imagus/comments/sp58gm/anyone_knows_how_to_change_this_white_color_to/

Comment 9

[Belius]

103.0 and this bug is glaringly obvious and looks outright horrible. Chrome, Opera, Edge behave as expected, just Firefox doesn't?

Comment 10

[styfle]

This seems to affect jpeg but not avif images. Perhaps its the progressive loading?

Comment 11

[beyoyo7374]

Still having this issue as of Firefox 116. Huge bummer for darkmode, forces me to use chrome

Comment 13

[Timothy Nikkel (:tnikkel)]

Bug 1231622 made this bug apply to css images (backgrounds) too.

Comment 14

[Bob Hood [:bhood]]

After having my memory refreshed, it was determined by the Graphics team that this issue remains valid, and is actually currently impacting users employing "dark mode." As this has the potential to affect many users, and could cause them to switch browsers, an S2 rating remains valid.

Comment 15

[Bob Hood [:bhood]]

Assigning to Tim for tracking purposes.

Comment 16

[Bob Hood [:bhood]]

Tim, do you think this is imageLib instead of WebRender?

Comment 17

[Timothy Nikkel (:tnikkel)]

It's kind of crosses the boundary between the two.

Comment 18

[Timothy Nikkel (:tnikkel)]

Attachment: WIP: Bug 1556156. Send an available rect with images to webrender.

This is intended to convey the part of the image that is decoded so that only that part is drawn because the region that is not decoded yet will contain opaque white (for opaque images).

Comment 19

[Timothy Nikkel (:tnikkel)]

Does this patch provide the decoded rect for webrender to use in a suitable way?

Comment 20

[Glenn Watson [:gw] | PTO until 6th Jan]

That seems like it should provided the needed information.

Comment 21

[Timothy Nikkel (:tnikkel)]

Next step here is for someone to write the webrender half of this which uses the information on which part of the surface represents the partially decoded image so that only that part of the surface is drawn. Similar to what imgFrame::SurfaceForDrawing does in the aDoPartialDecode case

https://searchfox.org/mozilla-central/rev/0b765f1050f3cb0674ab266e7692e6dad9d249a1/image/imgFrame.cpp#409

Comment 22

[Timothy Nikkel (:tnikkel)]

I'll unassign so that this is open for someone to come in for the webrender part.

Comment 23

[Kelsey Gilbert [:jgilbert]]

Waiting on webrender cycles.

Comment 24

[Bob Hood [:bhood]]

Flagging this report as stalled until we have the cycles to work it. Please remove the keyword when actively addressing it.

Comment 25

[Kagami Rosylight [:saschanaz] (they/them)]

(In reply to Timothy Nikkel (:tnikkel) from comment #21)

Next step here is for someone to write the webrender half of this which uses the information on which part of the surface represents the partially decoded image so that only that part of the surface is drawn. Similar to what imgFrame::SurfaceForDrawing does in the aDoPartialDecode case

https://searchfox.org/mozilla-central/rev/0b765f1050f3cb0674ab266e7692e6dad9d249a1/image/imgFrame.cpp#409

This sounds ideal, but what if we just send transparent pixels? Would it be easier as it sounds or would it be actually hard too?

Comment 26

[Timothy Nikkel (:tnikkel)]

We can't send transparent pixels because we have opaque images which we put in opaque surfaces which have to contain opaque pixels because skia needs that for some reason.

Comment 27

[Kagami Rosylight [:saschanaz] (they/them)]

Hmm. How much perf increase do we get by using opaque surfaces? Maybe a bit more extra memcopies? Would it be terrible to use non-opaque ones?

Comment 28

[Glenn Watson [:gw] | PTO until 6th Jan]

If rasterizing images on the GPU, a very significant performance increase. A reasonable rule of thumb (depends on lots of factors, of course) on older Intel integrated GPUs (typically our low end) is that rasterizing with blend enabled is ~an order of magnitude slower than the cost of rasterizing opaque pixels.