1635153 - Reduce the cost of querying window positions to the x server.

Status: RESOLVED FIXED

(Core :: Widget, enhancement)

Description

[Nicolas Silva [:nical]]

Converting from widget-local to screen coordinates is very expensive on x11 because it has to issue a blocking message to the x server to query the window position, and that call can block for arbitarily long times if the compositor is busy.
Also I believe wayland expects you to only deal in window-local coordinates.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Is caching the window screen position something feasible?

Comment 2

[Nicolas Silva [:nical]]

Masayuki, I'm told you know your way around the event handling code.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Yeah, I think that nsWindow should cache the screen position and it should be updated when widget is moved. Then, WidgetEvent should store screen coordinates. I bet that our Event.screenX etc do wrong thing:
https://searchfox.org/mozilla-central/rev/dc4560dcaafd79375b9411fdbbaaebb0a59a93ac/dom/events/Event.cpp#531-561
They computes latest position of the event. But I think that they should return screen coordinates at the event is synthesized. Especially this works odd when an event is cached to a variable in JS.

(I still don't check other browsers' behavior yet, though.)

smaug: Any thoughts?

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Ah, but this bug reports only for the first thing, I found these lines:
https://searchfox.org/mozilla-central/rev/dc4560dcaafd79375b9411fdbbaaebb0a59a93ac/widget/gtk/nsWindow.cpp#2349,2353
https://searchfox.org/mozilla-central/rev/dc4560dcaafd79375b9411fdbbaaebb0a59a93ac/widget/gtk/nsWindow.cpp#3063,3072-3073

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Caching window position should be doable. Don't BrowserChilds already do that in child processes?

Comment 6

[Michel Dänzer]

Also I believe wayland expects you to only deal in window-local coordinates.

There is no global coordinate space at all in Wayland. There are only per-surface local coordinate spaces.

Comment 7

[Nicolas Silva [:nical]]

Attachment: Bug 1635153 - Cache the result of gdk_window_get_origin.

Comment 8

[Markus Stange [:mstange]]

I think it would be a good idea to rename this bug to match the patch, or to move the patch to a new bug.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b9a02e153bcd
Cache the result of gdk_window_get_origin. r=masayuki

Comment 10

[Dorel Luca [:dluca]]

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

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Should this be backed out from beta because of the regressions?

Comment 12

[Nicolas Silva [:nical]]

(In reply to Olli Pettay [:smaug] from comment #11)

Should this be backed out from beta because of the regressions?

Either that or uplifting the most recent patch of bug 1652743 which is equivalent to backing this out and another patch that landed on top of it.