1482403 - DigitalOcean map doesn't paint properly

Status: VERIFIED FIXED

(Core :: Web Painting, defect, P1)

Description

[Jeff Muizelaar [:jrmuizel]]

Sometimes some of the blured shapes seem clipped. I suspect this is a regression from bug 1480620 or one of the other blob changes.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

The map I mean is at the bottom of https://www.digitalocean.com/

Comment 2

[Darkspirit]

Greenland: It also seems to happen with Basic compositing, but not with Chrome Dev (HW accel on/off). I suspect layout.display-list.retain.

Comment 3

[Darkspirit]

Attachment: 2018-08-28_22-59-33.mp4

Comment 4

[Darkspirit]

mozregression --good 2018-07-01 --bad 2018-08-10 --pref gfx.webrender.all:true -a https://www.digitalocean.com/
> 7:35.14 INFO: Last good revision: 03ecdc85a12613b5fb79805c32a9896395351415
> 7:35.14 INFO: First bad revision: ae6e737276854a376177f2a64c15e9172e01ab8a
> 7:35.14 INFO: Pushlog:
> https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=03ecdc85a12613b5fb79805c32a9896395351415&tochange=ae6e737276854a376177f2a64c15e9172e01ab8a

> ae6e73727685	Matt Woodrow — Bug 1475413 - Prefer using old items for uninvalidated frames during display list merging. r=miko

Also happens without WebRender and can be fixed with layout.display-list.retain;false.

Comment 5

[Pascal Chevrel:pascalc]

This regression affects 63 and has no owner nor priority set, could this bug be prioritized? Thanks

Comment 6

[Matt Woodrow (:mattwoodrow)]

Any chance you can take a look at this Miko?

The fix is either to add a new type to ShouldUseNewItem, or to mark the frame as modified when the property changes.

Should be able to look at the display list when it's broken and try spot which item is rendering wrong.

Comment 7

[Miko Mynttinen]

Attachment: testcase.zip

Minified testcase

Comment 8

[Miko Mynttinen]

Attachment: Bug 1482403 - Always create nsDisplayOpacity for filters and masks when there is visual opacity

Comment 9

[Miko Mynttinen]

Previously we did not always create nsDisplayOpacity, when creating nsDisplayFilter or nsDisplayMask for frames with opacity. Because this decision was made during DL building, and could change without frame invalidation, the retained display list merging would sometimes get confused when the container item type changed.
This patch always wraps frames with visual opacity in nsDisplayOpacity, unless the frame is an SVG frame that supports optimizing stroke/fill opacity, or if the frame is an SVGImage frame.
For most cases, the performance penalty should be minimal. The common case is that nsDisplayOpacity passes opacity handling to nsDisplayMasksAndClipPaths or nsDisplayFilters though opacity flattening mechanics.

Comment 10

[Pascal Chevrel:pascalc]

Miko, do you have a status update on this issue? thanks

Comment 11

[Miko Mynttinen]

(In reply to Pascal Chevrel:pascalc from comment #10)
> Miko, do you have a status update on this issue? thanks

The patch is ready and waiting for a review.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1feb8b3f06e7
Always create nsDisplayOpacity for filters and masks when there is visual opacity r=jwatt

Comment 13

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/1feb8b3f06e7

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Is it possible to write a test for this?

Comment 15

[Miko Mynttinen]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #14)
> Is it possible to write a test for this?

The original "simplified" testcase was very complex (due to 300KB of minified JS applying unpredictable style changes), but I will look into it.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Too late for 63.

Comment 18

[Emilio Cobos Álvarez [:emilio]]

Bug 1503601 has a much simpler test-case that was fixed by this patch, any chance we could land something based on that?

Comment 19

[Miko Mynttinen]

Thank you Emilio, I used that testcase to create a reftest.

Comment 20

[Emilio Cobos Álvarez [:emilio]]

[Tracking Requested - why for this release]: We're getting quite a few dupes of this bug, would be nice if it could be fixed if there's any 63 ridealong.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5690f5bfcd08
Add reftest r=me

Comment 22

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/5690f5bfcd08

Comment 23

[Pascal Chevrel:pascalc]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #20)
> [Tracking Requested - why for this release]: We're getting quite a few dupes
> of this bug, would be nice if it could be fixed if there's any 63 ridealong.

This has only recently landed and was not uplifted to beta yet so that will probably not make it into the dot release for Desktop planned for this week. I am tracking it though in case we have another dot release before we ship 64.

Comment 25

[Pascal Chevrel:pascalc]

Wontfix for 63 as we are unlikely to have another dot release before 64 ships 3 weeks from now.

Comment 26

[Daniel Cicas [:dcicas], Release QA]

I reproduced this issue using Fx 63.0a1, build ID: 20180810220115, on Windows 10 x64 and Ubuntu 18.04 x64.

I can confirm this issue is fixed, I verified using Fx 65.0b9, build ID: 20190107180200, on Windows 10 x64 and Ubuntu 18.04 LTS.