1529237 - Use matrix instead of offset for converting between chrome and content process event coordinates in TabParent

Status: RESOLVED FIXED

Description

[Henri Sivonen (:hsivonen)]

Various TabParent::Recv* methods, such as dictionary lookup, need to transform points from the coordinate space of the out-of-process iframe process to the coordinate space of the chrome process. To avoid bouncing via all the processes associated with the ancestors of the iframe in the Web hierarhy, we should probably get this info from APZ.

Comment 1

[Henri Sivonen (:hsivonen)]

Rescoping to cover TabParent::Sent* methods, too.

Comment 2

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1529237 - Use matrix instead of offset for converting between chrome and content process event coordinates in TabParent.

Depends on D20470

Comment 3

[Henri Sivonen (:hsivonen)]

I suppose it's possible for an out-of-process to have a non-invertible matrix applied to it. Is there a good way to compute an approximate inverse that e.g. for a rectangle collapsed to a line still transforms points so that they appear on the line?

Comment 4

[Botond Ballo [:botond]]

Hmm, good question. Looking through APZ code, there do seem to be places where we invert a matrix that can include CSS transforms (and thus could potentially not be invertible), without checking for invertibility (e.g. here).

It's not clear to me whether we are missing checks for invertibility, or if there is some invariant that ensures we don't even get to that code (e.g., we don't even build a layer) if a CSS transform isn't invertible.

(In reply to Henri Sivonen (:hsivonen) from comment #3)

Is there a good way to compute an approximate inverse that e.g. for a rectangle collapsed to a line still transforms points so that they appear on the line?

Matt, do you know the answer to this?

Comment 5

[Matt Woodrow (:mattwoodrow)]

(In reply to Botond Ballo [:botond] from comment #4)

Hmm, good question. Looking through APZ code, there do seem to be places where we invert a matrix that can include CSS transforms (and thus could potentially not be invertible), without checking for invertibility (e.g. here).

It's not clear to me whether we are missing checks for invertibility, or if there is some invariant that ensures we don't even get to that code (e.g., we don't even build a layer) if a CSS transform isn't invertible.

We do try to avoid building layers for transforms with non-invertible matrices, but async animations can mean that a transform changes to be non-invertible at a later point. We should make sure we handle this on the APZ side.

(In reply to Henri Sivonen (:hsivonen) from comment #3)

Is there a good way to compute an approximate inverse that e.g. for a rectangle collapsed to a line still transforms points so that they appear on the line?

Matt, do you know the answer to this?

I'm not sure I understand the question entirely, but if a content process has a non-invertible matrix applied to it, then it won't be visible at all. No coordinates in the parent coordinate space map to anything in the content process, so we can just detect a non-invertible matrix and report that hit testing didn't find anything.

Comment 6

[Henri Sivonen (:hsivonen)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #5)

I'm not sure I understand the question entirely, but if a content process has a non-invertible matrix applied to it, then it won't be visible at all. No coordinates in the parent coordinate space map to anything in the content process, so we can just detect a non-invertible matrix and report that hit testing didn't find anything.

This isn't about hit testing. This is about content saying to chrome that something happened that requires chrome to display some UI at the right location, so the inverse matrix transforms from the content coordinate space from the chrome space. We need something more sensible than crashing if the matrix isn't invertible.

One option is that if the matrix isn't invertible, we generate a point at the top left corner of the content area.

Comment 7

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #6)

One option is that if the matrix isn't invertible, we generate a point at the top left corner of the content area.

The latest just uses the top left corner of chrome coordinate space.

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e544d9948446
Use matrix instead of offset for converting between chrome and content process event coordinates in TabParent. r=kats

Comment 9

[Andrei Ciure[:andrei_ciure]]

https://hg.mozilla.org/mozilla-central/rev/e544d9948446

Comment 10

[Botond Ballo [:botond]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #5)

(In reply to Botond Ballo [:botond] from comment #4)

Hmm, good question. Looking through APZ code, there do seem to be places where we invert a matrix that can include CSS transforms (and thus could potentially not be invertible), without checking for invertibility (e.g. here).

It's not clear to me whether we are missing checks for invertibility, or if there is some invariant that ensures we don't even get to that code (e.g., we don't even build a layer) if a CSS transform isn't invertible.

We do try to avoid building layers for transforms with non-invertible matrices, but async animations can mean that a transform changes to be non-invertible at a later point. We should make sure we handle this on the APZ side.

(Just to close the loop here: the transforms we invert in APZ don't include OMTA transforms, only CSS transforms and APZ transforms. So, if only OMTA transforms can be non-invertible, we should be fine.)