On my linux PC(Ubuntu with NVIDIA gpu), popup(gecko profiler addon) is not shown with layers.acceleration.force-enabled on linux. It seemed that correct visual was not chosen like Bug 1479181.

The problem was addressed for me with attachment 8998404 [details] [diff] [review].

Comment on attachment 8998404 [details] [diff] [review] patch - Use GLContextGLX::FindVisual() when webrender is not enabled if possible Review of attachment 8998404 [details] [diff] [review]: ----------------------------------------------------------------- Why did this work before the WebRender changes?

(In reply to Jeff Muizelaar [:jrmuizel] from comment #4) > Comment on attachment 8998404 [details] [diff] [review] > patch - Use GLContextGLX::FindVisual() when webrender is not enabled if > possible > > Review of attachment 8998404 [details] [diff] [review]: > ----------------------------------------------------------------- > > Why did this work before the WebRender changes? Sorry, I could not understand your question. Can you explain more about it? I saw this bug even before WebRender was added to m-c. This bug is basically unrelated to WebRender. GLContextGLX::FindVisual() was added by Bug 1401455. But GLContextGLX::FindVisual() is used only when WebRender is enabled. And GLContextGLX::FindVisual() had bugs(Bug 1479181 and Bug 1478454). But the bugs were addressed. GLContextGLX::FindVisual() should work as expected now. By investigating Bug 1479181, I understood that this bug could be caused by bad visual selection by gdk_screen_get_rgba_visual(). Then this bug could be addressed by using GLContextGLX::FindVisual() instead of gdk_screen_get_rgba_visual() if possible. And I confirmed that attachment 8998404 [details] [diff] [review] addressed the problem for me.

Sotaro, can you please compare the visual which is used recently and which is used with your patch? Just the output of glxinfo utility. Would be great to know the difference and the default visual does not work for you. Thanks.

*why* the default does not work for you, sorry ;-)

I got the visual info by "glxinfo -v". Before apply the patch, the popup window with transparency had the following. It is got by gdk_screen_get_rgba_visual(). The visual did not have double buffer. It might affect to gl rendering problem. Visual ID: a4 depth=32 class=TrueColor, type=(none) bufferSize=32 level=0 renderType=rgba doubleBuffer=0 stereo=0 rgba: redSize=8 greenSize=8 blueSize=8 alphaSize=8 float=N sRGB=Y auxBuffers=4 depthSize=24 stencilSize=8 accum: redSize=16 greenSize=16 blueSize=16 alphaSize=16 multiSample=16 multiSampleBuffers=1 visualCaveat=Nonconformant Opaque. With attachment 8998404 [details] [diff] [review], the popup window with transparency had the following. It has double buffer. Visual ID: 82 depth=32 class=TrueColor, type=(none) bufferSize=32 level=0 renderType=rgba doubleBuffer=1 stereo=0 rgba: redSize=8 greenSize=8 blueSize=8 alphaSize=8 float=N sRGB=Y auxBuffers=4 depthSize=0 stencilSize=0 accum: redSize=16 greenSize=16 blueSize=16 alphaSize=16 multiSample=0 multiSampleBuffers=0 visualCaveat=None Opaque.

So the problem seems similar to glxChooseVisual() problem in Bug 1479181. gdk_screen_get_rgba_visual() returns only one visual, then the visual might not full the gecko's requirements(like GL, alpha, double buffer).

(In reply to Sotaro Ikeda [:sotaro] from comment #9) > So the problem seems similar to glxChooseVisual() problem in Bug 1479181. > gdk_screen_get_rgba_visual() returns only one visual, then the visual might > not full the gecko's requirements(like GL, alpha, double buffer). I'm not sure we can use double buffered visual for GdkWindow for non-accelerated path (sw rendering) when we also disable double buffering at https://dxr.mozilla.org/mozilla-central/rev/4e56a2f51ad739ca52046723448f3129a58f1666/widget/gtk/nsWindow.cpp#4082 I'll ask Gtk guys about that.

Thanks! If non-accelerated path does not support double buffering. Fallback from Webrender to BasicCompositor also causes the problem.

(In reply to Martin Stránský [:stransky] from comment #10) > (In reply to Sotaro Ikeda [:sotaro] from comment #9) > > So the problem seems similar to glxChooseVisual() problem in Bug 1479181. > > gdk_screen_get_rgba_visual() returns only one visual, then the visual might > > not full the gecko's requirements(like GL, alpha, double buffer). > > I'm not sure we can use double buffered visual for GdkWindow for > non-accelerated path (sw rendering) when we also disable double buffering at > https://dxr.mozilla.org/mozilla-central/rev/ > 4e56a2f51ad739ca52046723448f3129a58f1666/widget/gtk/nsWindow.cpp#4082 > > I'll ask Gtk guys about that. :stransky, is there an update about it?

(In reply to Sotaro Ikeda [:sotaro PTO 31/Aug-7/Sep] from comment #12) > (In reply to Martin Stránský [:stransky] from comment #10) > > (In reply to Sotaro Ikeda [:sotaro] from comment #9) > > > So the problem seems similar to glxChooseVisual() problem in Bug 1479181. > > > gdk_screen_get_rgba_visual() returns only one visual, then the visual might > > > not full the gecko's requirements(like GL, alpha, double buffer). > > > > I'm not sure we can use double buffered visual for GdkWindow for > > non-accelerated path (sw rendering) when we also disable double buffering at > > https://dxr.mozilla.org/mozilla-central/rev/ > > 4e56a2f51ad739ca52046723448f3129a58f1666/widget/gtk/nsWindow.cpp#4082 > > > > I'll ask Gtk guys about that. > > :stransky, is there an update about it? Adam Jackson (ajax) confirmed that it's okay to use db visual here as far as we don't do gl_front rendering (which we don't do anyway). So go ahead. We may also investigate if we sill need the disabled db on our widgets.

Comment on attachment 8998404 [details] [diff] [review] patch - Use GLContextGLX::FindVisual() when webrender is not enabled if possible > mHasAlphaVisual = needsAlphaVisual; >+ isSetVisual = true; > } else { > NS_WARNING("We're missing X11 Visual for WebRender!"); Please update the comment as it would be for !WebRender too. > } >- } else >+ } > #endif // MOZ_X11 >- { >+ >+ if (!isSetVisual) { > if (needsAlphaVisual) { This can be in one expression. > GdkScreen *screen = gtk_widget_get_screen(mShell);

(In reply to Martin Stránský [:stransky] from comment #13) > > > > :stransky, is there an update about it? > > Adam Jackson (ajax) confirmed that it's okay to use db visual here as far as > we don't do gl_front rendering (which we don't do anyway). So go ahead. We > may also investigate if we sill need the disabled db on our widgets. Thanks!

Applied the comment.

Comment on attachment 9004740 [details] [diff] [review] patch - Use GLContextGLX::FindVisual() when webrender is not enabled if possible I think we can ship that.

Could not land the patch. I have received this error "applying patch_1481694_2 patching file widget/gtk/nsWindow.cpp Hunk #1 FAILED at 3711 1 out of 1 hunks FAILED -- saving rejects to file widget/gtk/nsWindow.cpp.rej " This is the content of the rej file: --- nsWindow.cpp +++ nsWindow.cpp @@ -3712,56 +3712,57 @@ nsWindow::Create(nsIWidget* aParent, // popup window position. GtkWindowType type = GTK_WINDOW_TOPLEVEL; if (mWindowType == eWindowType_popup) { type = (mIsX11Display && aInitData->mNoAutoHide) ? GTK_WINDOW_TOPLEVEL : GTK_WINDOW_POPUP; } mShell = gtk_window_new(type); + bool isSetVisual = false; #ifdef MOZ_X11 // Ensure gfxPlatform is initialized, since that is what initializes // gfxVars, used below. Unused << gfxPlatform::GetPlatform(); bool useWebRender = gfx::gfxVars::UseWebRender() && AllowWebRenderForThisWindow(); // If using WebRender on X11, we need to select a visual with a depth buffer, // as well as an alpha channel if transparency is requested. This must be done // before the widget is realized. - if (mIsX11Display && useWebRender) { + if (mIsX11Display) { auto display = GDK_DISPLAY_XDISPLAY(gtk_widget_get_display(mShell)); auto screen = gtk_widget_get_screen(mShell); int screenNumber = GDK_SCREEN_XNUMBER(screen); int visualId = 0; if (GLContextGLX::FindVisual(display, screenNumber, useWebRender, needsAlphaVisual, &visualId)) { // If we're using CSD, rendering will go through mContainer, but // it will inherit this visual as it is a child of mShell. gtk_widget_set_visual(mShell, gdk_x11_screen_lookup_visual(screen, visualId)); mHasAlphaVisual = needsAlphaVisual; + isSetVisual = true; } else { - NS_WARNING("We're missing X11 Visual for WebRender!"); + NS_WARNING("We're missing X11 Visual!"); } - } else + } #endif // MOZ_X11 - { - if (needsAlphaVisual) { - GdkScreen *screen = gtk_widget_get_screen(mShell); - if (gdk_screen_is_composited(screen)) { - GdkVisual *visual = gdk_screen_get_rgba_visual(screen); - if (visual) { - gtk_widget_set_visual(mShell, visual); - mHasAlphaVisual = true; - } + + if (!isSetVisual && needsAlphaVisual) { + GdkScreen *screen = gtk_widget_get_screen(mShell); + if (gdk_screen_is_composited(screen)) { + GdkVisual *visual = gdk_screen_get_rgba_visual(screen); + if (visual) { + gtk_widget_set_visual(mShell, visual); + mHasAlphaVisual = true; } } } // We only move a general managed toplevel window if someone has // actually placed the window somewhere. If no placement has taken // place, we just let the window manager Do The Right Thing. NativeResize(); :sotaro please take a look.

Rebased.

Pushed by sikeda@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ba0c3051a9ed Use GLContextGLX::FindVisual() when webrender is not enabled if possible r=stransky