Closed Bug 1435143 Opened 2 years ago Closed 2 years ago

regression: Nextcloud menu tooltip backgrounds are broken

Categories

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

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- unaffected
firefox59 --- unaffected
firefox60 --- disabled

People

(Reporter: darkspirit, Assigned: mrobinson)

References

(Blocks 1 open bug, )

Details

(Keywords: correctness, nightly-community, regression)

Attachments

(3 files)

Attached file testcase.html
No description provided.
mozregression --good 2018-01-31 --bad 2018-02-01 --pref gfx.webrender.all:true startup.homepage_welcome_url:"https://bugzilla.mozilla.org/attachment.cgi?id=8947704"
> 4:56.61 INFO: Last good revision: c9c4506e63009d9a5f4da3ef6d5e88fa529b4f75
> 4:56.61 INFO: First bad revision: 3940b5d8f13529b17acfba25fa8e1d1d179628d0
> 4:56.62 INFO: Pushlog:
> https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c9c4506e63009d9a5f4da3ef6d5e88fa529b4f75&tochange=3940b5d8f13529b17acfba25fa8e1d1d179628d0

> 3940b5d8f135	Martin Roinson — Bug 1434288 - Update gecko for API changes in WR PR 2315. r=kats
> e99f6587c86e	Kartikaya Gupta — Bug 1434288 - Update webrender to commit e772c3cb8ea0a35e6477e9dc8dd2144e2de87b56. r=jrmuizel

-----

> WR @ b6e69a8efbcd8dc3e0c0a8a9925e6a9355635de3
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=9ea0e50f426552229ab5746ed3f5ab1c9ebf0561
mozregression --repo try --launch 9ea0e50f426552229ab5746ed3f5ab1c9ebf0561 --pref gfx.webrender.all:true startup.homepage_welcome_url:"https://bugzilla.mozilla.org/attachment.cgi?id=8947704"
-> good

> WR @ 87c6a6ba7e75d8ed0b2ddeeb5fa4e363482f7f86
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=7801945b00e7eac75e8bdf9d64869a9c284599de
mozregression --repo try --launch 7801945b00e7eac75e8bdf9d64869a9c284599de --pref gfx.webrender.all:true startup.homepage_welcome_url:"https://bugzilla.mozilla.org/attachment.cgi?id=8947704"
-> goood

> WR @ e772c3cb8ea0a35e6477e9dc8dd2144e2de87b56, with the fixes
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=62ab4844535a8ae2194ee6b7e43c1f9f05bb1511
mozregression --repo try --launch 62ab4844535a8ae2194ee6b7e43c1f9f05bb1511 --pref gfx.webrender.all:true startup.homepage_welcome_url:"https://bugzilla.mozilla.org/attachment.cgi?id=8947704"
-> bad

https://github.com/servo/webrender/compare/87c6a6ba7e75d8ed0b2ddeeb5fa4e363482f7f86...e772c3cb8ea0a35e6477e9dc8dd2144e2de87b56
Assignee: nobody → gwatson
Priority: -- → P1
I reduced this down to the following test case, via a WR recording. I then rolled local wrench/WR back past the reported regression range above, and it still didn't look correct.

Looking closer at the test case below, the green rect (color 0, 128, 0) has a clip id of 5, which is the clip-id for a bullet point (a very small radius), which seems like the display list is wrong.

Looking at the regression range above, and since rolling back WR doesn't affect the output, I think this is most likely to be a gecko-side bug related to the external scroll ID changes.

Does that sound right? If so, who is best to re-assign this to?

---
root: 
    items: 
      - 
        type: "stacking-context"
        items: 
          - 
            bounds: [0, 0, 1920, 1450]
            "clip-rect": [0, 0, 1920, 1450]
            "clip-and-scroll": 0
            "backface-visible": true
            type: clip
            id: 1
            "content-size": [1920, 1450]
          - 
            "clip-rect": [0, 0, 1920, 1450]
            "clip-and-scroll": 1
            "backface-visible": true
            type: "scroll-frame"
            id: 2
            "content-size": [1920, 1450]
            bounds: [0, 0, 1920, 1450]
          - 
            bounds: [0, 0, 1920, 1450]
            "clip-rect": [0, 0, 1920, 1450]
            "clip-and-scroll": 2
            "backface-visible": true
            type: clip
            id: 3
            "content-size": [1920, 1450]
          - 
            bounds: [12, 12, 60, 77]
            "clip-rect": [12, 12, 60, 77]
            "clip-and-scroll": [0, 1]
            "backface-visible": true
            type: rect
            color: 250.00002 164 1 1.0000
          - 
            bounds: [51, 47, 9, 9]
            "clip-rect": [51, 47, 9, 9]
            "clip-and-scroll": 0
            "backface-visible": true
            type: clip
            id: 4
            "content-size": [9, 9]
            complex: 
              - 
                rect: [50.8, 46.925, 9.2, 9.2]
                radius: [4.5, 4.5]
                "clip-mode": clip
          - 
            bounds: [51, 47, 9, 9]
            "clip-rect": [51, 47, 9, 9]
            "clip-and-scroll": 4
            "backface-visible": true
            type: rect
            color: black
          - 
            bounds: [72, 36, 0, 0]
            "clip-rect": [72, 36, 0, 0]
            "clip-and-scroll": [0, 1]
            "backface-visible": true
            type: "stacking-context"
            "scroll-policy": scrollable
            transform: [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, -25, 0, 0, 1]
            "transform-style": flat
            filters: []
            items: 
              - 
                bounds: [0, 0, 50, 29]
                "clip-rect": [0, 0, 50, 29]
                "clip-and-scroll": 1
                "backface-visible": true
                type: clip
                id: 5
                "content-size": [50, 29]
                complex: 
                  - 
                    rect: [0, 0, 50, 29]
                    radius: [1.5, 1.5]
                    "clip-mode": clip
              - 
                bounds: [0, 0, 50, 29]
                "clip-rect": [0, 0, 50, 29]
                "clip-and-scroll": [0, 5]
                "backface-visible": true
                type: rect
                color: 0 128 0 1.0000
              - 
                bounds: [-1, -1, 52, 31]
                "clip-rect": [-2, -2, 54, 33]
                "clip-and-scroll": [0, 1]
                "backface-visible": true
                glyphs: [55, 72, 86, 87]
                offsets: [0, 23, 14, 23, 28, 23, 40, 23]
                size: 18
                color: black
                font: /usr/share/fonts/truetype/dejavu/DejaVuSerif.ttf
Flags: needinfo?(mrobinson)
Flags: needinfo?(bugmail)
(Moving comment over from the github issue to keep discussion in one place)

It was my understanding that the external scroll ID change would be a no-op from the gecko point of view (i.e. it was just shifting some code around, no functional changes intended). @mrobinson can you confirm? If so then it shouldn't have exposed any latent gecko bugs assuming that's what happened.
Flags: needinfo?(bugmail)
Yes, my change should not have changed the way that scroll ids were assigned in Gecko at all. Nevertheless, it is possible that it introduced a subtle change. I'm currently trying to track down what is going with the display lists generated here.
Flags: needinfo?(mrobinson)
Thanks, reassigning to you for now.
Assignee: gwatson → mrobinson
See Also: → 1435675
See Also: → 1435415
See Also: → 1435129
When comparing a Maybe<WrScrollId> to another WrScrollId we need to properly
handle the case where Nothing() signifies the root scroll frame. This is
equivalent to calling scrollId.valueOr(FrameMetrics::NULL_SCROLL_ID), as was
done before WrScrolLId replaced ViewId in the WebRender ScrollingLayersHelper.
We also have DisplayListBuilder::TopmostScrollId always return a value instead
of a Maybe, since an empty clip stack means that the current scroll id is that
of the root scroll frame.

Previously Nothing() was not equivalent to WrScrollId { 0 }, which caused the
ScrollingLayersHelper to fill the mClipAndScroll value and push another
set of clip and scroll nodes onto the WebRender display list builder.
Attachment #8949075 - Flags: review?(bugmail)
Debian Testing (KDE, Radeon RX480)

mozregression --repo try --launch b7745f7eb6cfa1dbdca73b77e96625e0e1d013e3 --pref gfx.webrender.all:true startup.homepage_welcome_url:"https://bugzilla.mozilla.org/attachment.cgi?id=8947704|https://www.youtube.com/user/failarmy/videos|https://www.coursera.org/?authMode=login|https://bugzilla.mozilla.org/attachment.cgi?id=8949097"
* about:support webrender is active
* bug 1435143 Nextcloud: good
* bug 1435415 failarmy: good (checked with all themes + quantum lights dynamic)
* bug 1435675 login widget: good (Other bug, not new: The website had many colorful dots for a short moment when I switched to that tab.)
* bug 1435129 tab loading: good
* bug 1436420 activity stream: good

mozregression --launch 2018-02-07 --pref gfx.webrender.all:true startup.homepage_welcome_url:"https://bugzilla.mozilla.org/attachment.cgi?id=8947704|https://www.youtube.com/user/failarmy/videos|https://www.coursera.org/?authMode=login|https://bugzilla.mozilla.org/attachment.cgi?id=8949097"
* about:support webrender is active
* bug 1435143 Nextcloud: bad
* bug 1435415 failarmy: bad
* bug 1435675 login widget: bad 
* bug 1435129 tab loading: bad
* bug 1436420 activity stream: bad
See Also: 1435415
Duplicate of this bug: 1435415
See Also: 1435129
Duplicate of this bug: 1435129
Duplicate of this bug: 1436420
See Also: 1435675
Duplicate of this bug: 1435675
Comment on attachment 8949075 [details] [diff] [review]
Properly unwrap Maybe<WrScrollId> for the root scroll frame

Review of attachment 8949075 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8949075 - Flags: review?(bugmail) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d3dbee5cf682
Properly unwrap Maybe<WrScrollId> for the root scroll frame. r=kats
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d3dbee5cf682
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Duplicate of this bug: 1436772
This being fixed with a PR - is there a Gecko side reftest that we can add that catches this problem?
comment 15 includes a reftest?
Yeah it does. So this bug is good :)
You need to log in before you can comment on or make changes to this bug.