Missing close/maximize/minimize window buttons on MacOSX with WebRender

RESOLVED FIXED in Firefox 56

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mkohler, Assigned: mstange)

Tracking

(Blocks 1 bug)

unspecified
mozilla56
x86_64
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(3 attachments)

When enabling gfx.webrender.enabled on Mac OS X the close/maximize/minimize buttons do not appear.
Whiteboard: [gfx-note]
Whiteboard: [gfx-note] → [gfx-noted]
nsChildView::DrawWindowOverlay needs to be called after compositing.
Apparently we already try to call it: http://searchfox.org/mozilla-central/rev/1a0d9545b9805f50a70de703a3c04fc0d22e3839/gfx/webrender_bindings/RendererOGL.cpp#131

Looks like we need to debug why it's not working.
Assignee: nobody → a.beingessner
So DrawWindowOverlay doesn't do anything because it does a check for aContext.mLayerManger (which we never set up) to construct a GLManager. If it's not there, then it just bails out early.

The GLManager is required by RectTextureImage::Draw, which draws the actual buttons.

We don't want to set up an mLayerManager, because that's a LayerManagerComposite which wants to handle all of the GL context, which webrender should be the owner of.

A few options:

* Have webrender expose a version of RectTextureImage::Draw and use that
* Have gecko insert the chrome's images into webrender's display list
Assignee: a.beingessner → mstange
Summary: Missing close/maximize/minimize buttons on MacOSX → Missing close/maximize/minimize window buttons on MacOSX with WebRender
Duplicate of this bug: 1370638
Comment on attachment 8880182 [details]
Bug 1368846 - Display the window buttons when webrender is used.

https://reviewboard.mozilla.org/r/151548/#review156528
Attachment #8880182 - Flags: review?(matt.woodrow) → review+
(In reply to Alexis Beingessner [:Gankro] from comment #3)
> So DrawWindowOverlay doesn't do anything because it does a check for
> aContext.mLayerManger (which we never set up) to construct a GLManager. If
> it's not there, then it just bails out early.

Can we not use a GLManager that just wraps a GLContext directly? GLPresenter seems to do that. See for example the change below. When I run with that I get shader compilation failures but it might be a viable option.


@@ -2110,17 +2110,21 @@ nsChildView::PostRender(WidgetRenderingContext* aContext)
   mViewTearDownLock.Unlock();
 }

 void
 nsChildView::DrawWindowOverlay(WidgetRenderingContext* aContext,
                                LayoutDeviceIntRect aRect)
 {
   mozilla::UniquePtr<GLManager> manager(GLManager::CreateGLManager(aContext->mLayerManager));
+  if (!manager) {
+    manager = MakeUnique<GLPresenter>(aContext->mGL);
+  }
   if (manager) {
+    PrepareWindowEffects();
     DrawWindowOverlay(manager.get(), aRect);
   }
 }

 void
 nsChildView::DrawWindowOverlay(GLManager* aManager, LayoutDeviceIntRect aRect)
 {
   GLContext* gl = aManager->gl();
Comment on attachment 8880182 [details]
Bug 1368846 - Display the window buttons when webrender is used.

https://reviewboard.mozilla.org/r/151548/#review156790

Mirroring my question from IRC: Is the intent to later break down the image into more display items? Because it seems a little overkill to create two new nsIWidget APIs just for this. I'd rather just return the image and associated data from a single API and do the WR-specific stuff in WebRenderLayerManager. The image key can be managed entirely inside WebRenderLayerManager for example.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #7)
> (In reply to Alexis Beingessner [:Gankro] from comment #3)
> > So DrawWindowOverlay doesn't do anything because it does a check for
> > aContext.mLayerManger (which we never set up) to construct a GLManager. If
> > it's not there, then it just bails out early.
> 
> Can we not use a GLManager that just wraps a GLContext directly?

That's one of the approaches we discussed, but IIRC Dzmitry didn't want to require webrender to guarantee to be in the right state by the time DrawWindowOverlay is called.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)
> Mirroring my question from IRC: Is the intent to later break down the image
> into more display items?

The intent is to add more display items, one for each window corner, for bug 1375298.

You're right that two new nsIWidget APIs seem like overkill, but it's not really a big cost.
Comment on attachment 8880182 [details]
Bug 1368846 - Display the window buttons when webrender is used.

https://reviewboard.mozilla.org/r/151548/#review156936

OK, this is fine for now then. We can evolve it in the future as the API needs become clearer.
Attachment #8880182 - Flags: review?(bugmail) → review+
Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/96d2dfbdc8d2
Display the window buttons when webrender is used. r=kats,mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/96d2dfbdc8d2
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Depends on: 1377314
Not sure if the cause is the same but these buttons missing again on Firefox 57.0b4  

Model Name:	MacBook Pro
Model Identifier:	MacBookPro13,3
Processor Name:	Intel Core i7
Processor Speed:	2.9 GHz
Chipset Model:	AMD Radeon Pro 460
Posted image image of the issues
(In reply to adamprocter from comment #13)
> Not sure if the cause is the same but these buttons missing again on Firefox 57.0b4  

Thank you for testing! :)
A friend is also testing 57.0b4 on macOS and had this problem, too (without WebRender).
I created bug 1405111 for this, which got marked as duplicate of bug 1402501.
You need to log in before you can comment on or make changes to this bug.