1589955 - Javascript image change cause stretching glitch before CSS is honored

Status: VERIFIED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect, P2)

Description

[Vincent Deconinck]

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

Steps to reproduce:

Open sample page at http://vdeconinck.synology.me/ffx_bug/ and wait for the slideshow to switch images.

Actual results:

There is a visible glitch when an image is first shown : it is resized to fit the frame of the previous image (even if it has a different aspect ratio) before CSS is honored and image is shown with its original image aspect ratio.

Expected results:

New image should immediately be displayed with the CSS applied.
NB: Chrome behaviour is ok.

Comment 1

[Vincent Deconinck]

Note : the symptoms are similar to bug 953364 but this one is obviously not fixed. Feel free to mark this as a dupe and reopen the original one if the root cause is identical...

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Thanks for filing!

I wonder if it's a regression...

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 4

[Timothy Nikkel (:tnikkel)]

Sorry bugbug, we don't know yet.

Comment 5

[Mats Palmgren (inactive)]

Attachment: Testcase #1

Here's a somewhat reduced testcase based on the Reporter's testcase.

Comment 6

[Mats Palmgren (inactive)]

Fwiw, the flicker also occurs with display: flex instead of display: grid.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

I see it also with display: block, I'm moderately sure this shouldn't be grid/flex-related.

So I think I know what happens, and I'm ~sure this was regressed by bug 1395964 (though it'd be good to confirm).

What is going on is, I think:

• src changes.
• We get enough metadata about the new image to figure out the right aspect-ratio / size of the image, but haven't decoded it yet.
• We do layout with the new image's dimensions.
• We go to paint, try to decode the image, but it's not fully ready yet, so in order to prevent flashing, we paint the previous image.
• Normally this is fine since the images have similar aspect ratios, but in this case it's bad.

As far as I understand, other browsers sync-decode by default.

So, options in this case, I think, could be something like, if we have both mImage and mPrevImage, and their ratios are quite different (how much, though?), and the image is sized intrinsically (the frame size is dependent on the image intrinsic size / ratio), either:

• Sync-decode, to prevent flashing, or...
• Don't paint mPrevImage (we'd flash white).

Tim, thougts / better ideas? Maybe I'm completely off? :)

Comment 8

[Alice0775 White]

I confirmed the regression range on Windows10 using the Testcase #1
mozregression:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=94682e23ba118dcfab3843a3280618b2e1686ecb&tochange=fdc7b66d76a4ded62d0292bef4de0ffb090d9587

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Thanks so much Alice :)

Comment 10

[Timothy Nikkel (:tnikkel)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

So, options in this case, I think, could be something like, if we have both mImage and mPrevImage, and their ratios are quite different (how much, though?), and the image is sized intrinsically (the frame size is dependent on the image intrinsic size / ratio), either:

• Sync-decode, to prevent flashing, or...
• Don't paint mPrevImage (we'd flash white).

Tim, thougts / better ideas? Maybe I'm completely off? :)

Thanks for this explanation.

I would be fine with sync-decoding if the image size or ratio changed at all (even by a small amount) and the frame size was dependent on that.

Comment 11

[Daniel Bodea [:danibodea]]

I confirm the regression range found in comment 8. Also, I've set the flags and platform.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Alright, will give that a shot, when I get to it.

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1589955 - Sync-decode images when intrinsic ratio between the old and new source changes.

This seems to fix it for me, and it's pretty straight-forward, but I think
we'd still paint the old image if the image is huge and it's loading, which
may be counter-productive. Maybe we should guard the whole "paint mOldImage"
with and if (!oldImageIsDifferent), wdyt?

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

Change the status for beta to have the same as nightly and release.
For more information, please visit auto_nag documentation.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/37538ab48bab
Sync-decode images when intrinsic ratio between the old and new source changes. r=tnikkel

Comment 16

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/37538ab48bab

Comment 17

[Pascal Chevrel:pascalc]

Regression in Firefox 54 and not a P1, this can ride the 72 train, wontfix 71.

Comment 18

[Daniel Bodea [:danibodea]]

I have verified this fix on Nightly v73.0a1 and Beta v72.0b67 on Windows 10 and Mac OS 10.13.6 after reproducing it in Firefox v71.0.