Closed Bug 1635153 Opened 3 months ago Closed 1 month ago

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

Categories

(Core :: Widget, enhancement)

Desktop
Linux
enhancement

Tracking

()

RESOLVED FIXED
mozilla80
Tracking Status
firefox80 --- fixed

People

(Reporter: nical, Assigned: nical)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

Attachments

(1 file)

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.

Is caching the window screen position something feasible?

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

Flags: needinfo?(masayuki)

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?

Flags: needinfo?(masayuki) → needinfo?(bugs)

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

Blocks: wr-linux-mvp

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.

Assignee: nobody → nical.bugzilla
Status: NEW → ASSIGNED

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.

OS: Unspecified → Linux
Hardware: Unspecified → Desktop
Summary: Consider dispatching events with widget coordinates rather than screen coordinates → Reduce the cost of querying window positions to tyhe x server.
Summary: Reduce the cost of querying window positions to tyhe x server. → Reduce the cost of querying window positions to the x server.
Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b9a02e153bcd
Cache the result of gdk_window_get_origin. r=masayuki
Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80
Regressions: 1652743
Depends on: 1655924

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

Flags: needinfo?(nical.bugzilla)

(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.

Flags: needinfo?(nical.bugzilla)
You need to log in before you can comment on or make changes to this bug.