User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:42.0) Gecko/20100101 Firefox/42.0 Build ID: 20150721150952 Steps to reproduce: 1. I configured my Nightly to use GTK3, following this page : http://glandium.org/blog/?p=3555 2. I tried to select and drag&drop two paragraphs of a web page Actual results: Trying to drag&drop cause Firefox to freeze for about 5 seconds. After this delay, I can drag correctly. The bigger the amount of text I try to drag, the longer Firefox freezes. Expected results: I should be able to move the selected text immediately, instead of waiting up to five seconds.

I can't seem to reproduce with GTK 3.14 on Debian Jessie. Do you mind providing your GTK version and distro (as well as any other information that might be relevant)? Thanks!

(In reply to Andrew Comminos [:acomminos] from comment #1) > I can't seem to reproduce with GTK 3.14 on Debian Jessie. Do you mind > providing your GTK version and distro (as well as any other information that > might be relevant)? Thanks! I'm on Ubuntu 15.04, with Unity 7.3.2. My libgtk package is in version 3.14.13. The "About Nightly" window says I have the version 42.0a1 (2015-07-21). Also, I don't have a graphic card, only Intel's chipset. My "details" screen in the settings says "Intel Haswell Mobile".

Oh, and I just saw it seems to be particularly visible in the "Reader View". If I select several paragraphs in this view, the freeze can take about 5 seconds.

This looks to be caused by a slower drawing path being used for DND on GTK3. I'll take this.

Looks like the use of GtkOffscreenWindow is causing us to reinitialize nsScreenManagerGtk every frame due to a ConfigureNotify being sent to the root window, which doesn't help.

This patch implements DND alpha pixmap drawing using cairo surfaces on GTK3, included support for device scale.

This patch uses monitors-changed to signal monitor size changes to nsScreenManagerGtk instead of listening to the root window's ConfigureNotify, preventing GtkOffscreenWindow from generating screen manager initialization requests. Added bonus of handling monitor change events on non-X11 targets. Thanks!

Comment on attachment 8638974 [details] [diff] [review] Use monitors-changed signal to update screen manager on GTK. (In reply to Andrew Comminos [:acomminos] from comment #5) > Looks like the use of GtkOffscreenWindow is causing us to reinitialize > nsScreenManagerGtk every frame due to a ConfigureNotify being sent to the > root window, which doesn't help. Do you know why/how GtkOffscreenWindow triggers native ConfigureNotify events on the root window? Regardless, this change is good, thanks.

Comment on attachment 8638973 [details] [diff] [review] Draw drag and drop alpha pixmap correctly on GTK3. Please add a build time check that system cairo is being used here. Something like this for one of the cairo functions invoked https://hg.mozilla.org/mozilla-central/annotate/2ddec2dedced/widget/gtk/nsWindow.cpp#l1948 >- // TODO GTK3 >+ cairo_surface_t *surf = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, >+ dragRect.width, >+ dragRect.height); This should be an x11 pixmap surface. It will be used to draw to the drag icon window. It makes more sense to upload once instead of each time this is drawn, but that may not be a major issue if the window is not actually redrawn, but just recomposited when moved. More importantly aSurface may be a pixmap, so drawing to an image surface would need to read back from the pixmap. Readback is the number 1 performance problem to avoid when drawing. However, what is in this patch should at least give nicer results, even if there may not be an improvement in performance, so this can land with just a TODO comment indicating that an X11 Pixmap should be used instead of an image surface to avoid readback.

(In reply to Karl Tomlinson (ni?:karlt) from comment #8) > Do you know why/how GtkOffscreenWindow triggers native ConfigureNotify events > on the root window? This appears to be caused by the root window having the SubstructureNotify event mask set, causing events to be received from the offscreen window (which is a direct child of the root). (In reply to Karl Tomlinson (ni?:karlt) from comment #9) > This should be an x11 pixmap surface. > > It will be used to draw to the drag icon window. It makes more sense to > upload once instead of each time this is drawn, but that may not be a major > issue if the window is not actually redrawn, but just recomposited when > moved. > > More importantly aSurface may be a pixmap, so drawing to an image surface > would need to read back from the pixmap. Readback is the number 1 > performance > problem to avoid when drawing. > > However, what is in this patch should at least give nicer results, even if > there may not be an improvement in performance, so this can land with just a > TODO comment indicating that an X11 Pixmap should be used instead of an image > surface to avoid readback. Sounds good. I suppose we'd want something similar to what is done in DrawThemeWithCairo in nsNativeThemeGtk, where we attempt to borrow the underlying X drawable or in-memory image. Perhaps it would be worth implementing a shim around tree cairo for mapping to system cairo surfaces. I'll flag these patches for checkin (with the TODO) and look into that.

Updated with tree cairo check and TODO.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bae15f584f6a

the 2nd patch failed to apply: renamed 1186661 -> Bug-1186661---Draw-drag-and-drop-alpha-pixmap-corr.patch applying Bug-1186661---Draw-drag-and-drop-alpha-pixmap-corr.patch patching file widget/gtk/nsDragService.cpp Hunk #4 FAILED at 450 1 out of 4 hunks FAILED -- saving rejects to file widget/gtk/nsDragService.cpp.rej patch failed, unable to continue (try -v) patch failed, rejects left in working directory errors during apply, please fix and refresh Bug-1186661---Draw-drag-and-drop-alpha-pixmap-corr.patch could you take a look ? Thanks!

Rebased off inbound, should work now. Didn't receive any merge conflicts though.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e3d7d24ebf13 https://hg.mozilla.org/integration/mozilla-inbound/rev/8a8ffb4d31b5

(In reply to Andrew Comminos [:acomminos] from comment #10) > This appears to be caused by the root window having the SubstructureNotify > event mask set, causing events to be received from the offscreen window > (which is a direct child of the root). Thanks. Seams less than ideal if each frame creates a new window, but the fix is good anyway, thanks. > I suppose we'd want something similar to what is done in > DrawThemeWithCairo in nsNativeThemeGtk, where we attempt to borrow the > underlying X drawable or in-memory image. Perhaps it would be worth > implementing a shim around tree cairo for mapping to system cairo surfaces. > I'll flag these patches for checkin (with the TODO) and look into that. It's different from nsNativeThemeGtk because DrawTarget (not system cairo) is doing the drawing and the destination surface properties are known. It can be like the code in the patch here if there is a CreateDrawTargetForPixmap kind of function.

https://hg.mozilla.org/mozilla-central/rev/e3d7d24ebf13 https://hg.mozilla.org/mozilla-central/rev/8a8ffb4d31b5

(In reply to Karl Tomlinson (ni?:karlt) from comment #16) > (In reply to Andrew Comminos [:acomminos] from comment #10) > > This appears to be caused by the root window having the SubstructureNotify > > event mask set, causing events to be received from the offscreen window > > (which is a direct child of the root). > > Thanks. Seams less than ideal if each frame creates a new window, but the > fix is good anyway, thanks. It's not creating new windows thankfully, the child window (the dragged pixmap) is just having its coordinate updates sent to the root as it's being dragged around as ConfigureNotify events. So it's not every frame (my mistake). > > > I suppose we'd want something similar to what is done in > > DrawThemeWithCairo in nsNativeThemeGtk, where we attempt to borrow the > > underlying X drawable or in-memory image. Perhaps it would be worth > > implementing a shim around tree cairo for mapping to system cairo surfaces. > > I'll flag these patches for checkin (with the TODO) and look into that. > > It's different from nsNativeThemeGtk because DrawTarget (not system cairo) > is doing the drawing and the destination surface properties are known. > > It can be like the code in the patch here if there is a > CreateDrawTargetForPixmap > kind of function. Right, thanks. We can't really do the same thing as we need the lifetime of the surface to be managed by GTK.