745936 - Display port should not override dirtyRect during hit detection

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Wesley Johnston (:wesj)]

Put a bunch of logging in for a different bug, but noticed that the rect we use for hit testing was growing from a 1x1 rect to one the displayport size right here:

http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#2169

I think that means we're looking at way to many objects for hit testing when we have the display port turned on.

Comment 1

[Wesley Johnston (:wesj)]

Attachment: Patch

Comment 2

[Wesley Johnston (:wesj)]

Comment on attachment 615468 [details] [diff] [review]
Patch

Hmm.. seeing some issues. Maybe better to just get feedback first, since its possible I'm misinterpreting this code entirely.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 615468 [details] [diff] [review]
Patch

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

Very good catch! But you should just make usingDisplayport false when aBuilder->IsForEventDelivery()

Comment 4

[Wesley Johnston (:wesj)]

Attachment: Patch

Ahh. That explains the problems I was seeing. Thanks

Comment 5

[Wesley Johnston (:wesj)]

Comment on attachment 615502 [details] [diff] [review]
Patch

Doh. Thats not right...

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

You, you need a ! there :-)

Comment 7

[Wesley Johnston (:wesj)]

Attachment: Patch

Heh. Yep. Just fixing that.

Comment 8

[Wesley Johnston (:wesj)]

Requesting blocking

Comment 9

[Jeff Muizelaar [:jrmuizel]]

Does this mean the we're also using the display port during painting when we should be using the dirtyRect? i.e. could this also be the explanation for large displaylist computation during paint?

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We always have to recompute the display list for the whole displayport, by design.

(Maybe that can be improved, but certainly not before DLBI lands.)

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/372d32e0ea61

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/372d32e0ea61
https://hg.mozilla.org/mozilla-central/rev/1f86ad4f883d

Leaving open as the second changeset is empty - not sure what the intention was?

Comment 13

[Wesley Johnston (:wesj)]

Just didn't realize it had already landed.