1436409 - Remove nsDisplayLayerEventRegions and related code

Status: RESOLVED FIXED

(Core :: Web Painting, enhancement, P3)

Description

[Miko Mynttinen]

Follow-up to bug 1434243. Using nsDisplayCompositorHitTestInfo display items instead of nsDisplayLayerEventRegions seems to be both faster and simpler solution.

Since they both implement the same functionality, we should get rid of nsDisplayLayerEventRegions completely.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Fx 60, which shipped the simple-event-regions codepath, has been in the wild now for a month with no (to my knowledge) issues reported. I think we should now dump the nsDisplayLayerEventRegions codepaths as it's a lot of code complexity that can be eliminated.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

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

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1436409 - Remove gecko/APZ hit-test codepath from WebRenderCommandBuilder.

This removes the gfx.webrender.hit-test pref, assumes a value of true
everywhere it is used, and deletes all the resulting dead code.

Some gtests were setting this pref to false, and they are now updated to
set gfxVars::UseWebRender to false instead, which has the desired effect
of using the non-WR hit-testing codepath in APZ. (The data needed for
this codepath is set up by the gtests themselves).

Review commit: https://reviewboard.mozilla.org/r/250454/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250454/

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1436409 - Remove the layout.event-regions.enabled pref and bake it in as false everywhere.

This pref was used to enable the building of nsDisplayLayerEventRegions
items without APZ, so that we could test it in isolation. However, we no
longer need to do so, and these display items are going to be deleted
anyway, so we can remove this pref.

Review commit: https://reviewboard.mozilla.org/r/250456/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250456/

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1436409 - Remove the simple-event-region-items pref and bake it in as true.

Review commit: https://reviewboard.mozilla.org/r/250458/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250458/

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1436409 - Remove the nsDisplayLayerEventRegions display item.

Review commit: https://reviewboard.mozilla.org/r/250460/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250460/

Comment 7

[Botond Ballo [:botond]]

Comment on attachment 8984586 [details]
Bug 1436409 - Remove gecko/APZ hit-test codepath from WebRenderCommandBuilder.

https://reviewboard.mozilla.org/r/250454/#review256762

I didn't look very closely at the changes to WebRenderCommandBuilder.

Comment 8

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8984589 [details]
Bug 1436409 - Remove the nsDisplayLayerEventRegions display item.

https://reviewboard.mozilla.org/r/250460/#review256790

Comment 9

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8984587 [details]
Bug 1436409 - Remove the layout.event-regions.enabled pref and bake it in as false everywhere.

https://reviewboard.mozilla.org/r/250456/#review256792

Comment 10

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8984589 [details]
Bug 1436409 - Remove the nsDisplayLayerEventRegions display item.

https://reviewboard.mozilla.org/r/250460/#review256800

Comment 11

[Miko Mynttinen]

Comment on attachment 8984588 [details]
Bug 1436409 - Remove the simple-event-region-items pref and bake it in as true.

https://reviewboard.mozilla.org/r/250458/#review256820

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Timothy, did you mean to r- the one patch?

Comment 13

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8984587 [details]
Bug 1436409 - Remove the layout.event-regions.enabled pref and bake it in as false everywhere.

https://reviewboard.mozilla.org/r/250456/#review256794

Comment 14

[Timothy Nikkel (:tnikkel)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #12)
> Timothy, did you mean to r- the one patch?

No, mozreview must have messed up because I remember accidentally hitting r- and then changing it to r+ before pressing publish but mozreview must have only saved the r- part.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3f96430462f8
Remove gecko/APZ hit-test codepath from WebRenderCommandBuilder. r=botond
https://hg.mozilla.org/integration/autoland/rev/53572697be32
Remove the layout.event-regions.enabled pref and bake it in as false everywhere. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/b5327ed6e61a
Remove the simple-event-region-items pref and bake it in as true. r=miko
https://hg.mozilla.org/integration/autoland/rev/995feb0ecd7f
Remove the nsDisplayLayerEventRegions display item. r=mattwoodrow,tnikkel

Comment 16

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/3f96430462f8
https://hg.mozilla.org/mozilla-central/rev/53572697be32
https://hg.mozilla.org/mozilla-central/rev/b5327ed6e61a
https://hg.mozilla.org/mozilla-central/rev/995feb0ecd7f