1645762 - mask images should not be considered invalid when drawn at a non-ideal size

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Donnie Ray Crisp]

Attachment: mask-cover.zip

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/83.0.4103.97 Safari/537.36

Steps to reproduce:

i have a grid / flex container and child divs with flexible widths. each div has a background-image and a mask-image property with background- and mask-size set to cover or contain. when the page is loaded i see the correctly masked images.

i attached a demo as a zip.

Actual results:

when resizing the browser the mask-images randomly disappear and reappear. sometimes all of them sometimes just a few. if i shift + reload the page on a disappeared state, all the images reaapear correctly.

the property mask-size: cover or contain is causing the problem.

Expected results:

the mask-images shouldn´t be disappearing.

Comment 1

[Marcela (Please NI request to Brindusa Tot)]

Hi Donnie Ray Crisp!
I was not able to reproduce this issue on latest Nightly version 79.0a1 (2020-06-18)(64-bit) on macOS 10.15. After downloading the zip file attached and loaded the page, I resized the browser and the mask-images were shown correctly.
Could you please try to reproduce it on latest Nightly and tell us your input? You can download here: https://nightly.mozilla.org/

Comment 2

[Donnie Ray Crisp]

Attachment: MacBook Information

Comment 3

[Donnie Ray Crisp]

Hello Marcela,
i attached a screenshot of my MacBook version / information. Maybe this helps to pinpoint the issue.

I am still able to reproduce the issue with the latest Firefox nightly build. Here is a screenrecording: https://drive.google.com/file/d/1nY1qyOVYDkyTlCzBNPAxk59VsZo7f9pQ/view?usp=sharing

The problem seemed to be fixed on a colleagues older MacBook with the nightly build. But not on mine. Sorry.

Comment 4

[Marcela (Please NI request to Brindusa Tot)]

I was able to reproduce this issue on latest Nightly version 79.0a1 (2020-06-25)(64-bit), Beta 78.0b9, and Release 77.0.1 on macOS 10.15 and Windows 10 using file attached. The background-image is loaded but mask-image is not showing anytime. On Chrome background and image are showing correctly.
I'll change flags accordingly.

Regression range:
build_date: 2019-07-05 22:12:56.147000
build_url: https://queue.taskcluster.net/v1/task/ZOJONXVkQI-WbeFnO1DSfw/runs/0/artifacts/public%2Fbuild%2Ftarget.zip
changeset: b50ec6b229e5aa8c882aa65a265d27866a3ad042
pushlog_url: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4d37f42cecafac5c14d3dbc20a9204643b5cc3ea&tochange=b50ec6b229e5aa8c882aa65a265d27866a3ad042

Comment 5

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

The severity field is not set for this bug.
:nordzilla, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 6

[Cameron McCormack (:heycam)]

The commits in the regression range in comment 4 don't seem to be relevant.

I can reproduce this in current Nightly on Linux, but only with WebRender disabled.

Interestingly, in Firefox 56 and earlier, I get a flickering when narrowing the window, but in 57 and later, the foxes just disappear entirely below a certain window width.

Comment 7

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

The severity field is not set for this bug.
:heycam, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 8

[Cameron McCormack (:heycam)]

The problem is that when we render the mask images at certain sizes, the image draw result is ImgDrawResult::WRONG_SIZE, which PaintMaskSurface in SVGIntegrationUtils.cpp treats as "the mask is invalid".

Comment 9

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a2193ac88979da3ff134b4f1f6f569d88a9f2c2e

Comment 10

[Cameron McCormack (:heycam)]

Attachment: Bug 1645762 - Don't treat image masks as invalid if painted at an unideal size.

Comment 11

[Cameron McCormack (:heycam)]

Timothy, the attached patch fixes the test case for me, but I am wondering whether we should be doing anything to respond to the WRONG_SIZE result by scheduling a new decode at the requested size. (I'm not familiar with how WRONG_SIZE is normally handled.)

Comment 12

[Timothy Nikkel (:tnikkel)]

(In reply to Cameron McCormack (:heycam) from comment #11)

Timothy, the attached patch fixes the test case for me, but I am wondering whether we should be doing anything to respond to the WRONG_SIZE result by scheduling a new decode at the requested size. (I'm not familiar with how WRONG_SIZE is normally handled.)

Whatever path was used to draw the image (Draw, GetFrame etc) should have requested a decode (modulo some cases where we don't allow further differently sized decodes because we have too many or other unusual cases we might have). When that decode completes it should send an invalidation, which would cause whatever is drawing the image to repaint which should pick up the correctly sized image. That's how it is supposed to work, it's possible that doesn't work for some reason on a specific path.

Comment 13

[Cameron McCormack (:heycam)]

Is it correct then to draw the mask image when it's WRONG_SIZE, or should we only fix whatever is causing the decode / invalidation not to happen?

Comment 14

[Timothy Nikkel (:tnikkel)]

Okay, so the problem is persistent. It does seem like we should fix the invalidation issue as well.

Comment 15

[Cameron McCormack (:heycam)]

It looks like for background images, unlike content images, we never try to compute an ideal size to decode at. Does that match your understanding? Mask images are treated the same as background images in this regard. If that's right, then there is nothing else to do in this patch -- we do kick off a decode and invalidate the element once the mask image has finished decoding at its natural size.

Comment 16

[Cameron McCormack (:heycam)]

Attachment: Bug 1645762 - Don't treat image masks as invalid if painted at an unideal size.

Comment 17

[Timothy Nikkel (:tnikkel)]

When RasterImage::Draw is called with a size that should trigger a decode at that size. Content images and background images should both go through nsLayoutUtils::DrawImageInternal which computes a size for that parameter. Content images do try to be smart and get the proper sized decode started before the draw call, that part does differ from background images.

Comment 18

[Cameron McCormack (:heycam)]

Understood. Tracing through, I find that we do indeed trigger a decode at the requested size, and correctly invalidate the frame when it's finished. It's only when the surface cache decides not to store decoded frames at more sizes that we start to return WRONG_SIZE and not invalidate. So I think everything is OK.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6707f5a64dac
Don't treat image masks as invalid if painted at an unideal size. r=tnikkel

Comment 20

[Timothy Nikkel (:tnikkel)]

Thanks!

Comment 21

[Timothy Nikkel (:tnikkel)]

I realized that we need this same fix in another place, I filed bug 1653905 and I'll upload a patch.

Comment 22

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/6707f5a64dac

Comment 23

[Cristian Fogel [:cfogel]]

In an attempt to verify the issue, running into an issue:
No images 79.0a1 (2020-06-15) or 80.0b6 with either the macOS or Win10 device.

@Cameron, am I missing something or is this a separate issue?

Comment 24

[Cameron McCormack (:heycam)]

Not sure about the missing images. If there are still problems, please file a new bug. Thank you!