Closed Bug 1438456 Opened 2 years ago Closed 2 years ago

Add GLLibraryEGL::CreateDisplay()

Categories

(Core :: Graphics: WebRender, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: sotaro, Assigned: sotaro)

Details

Attachments

(1 file, 1 obsolete file)

When GLLibraryEGL is used for ANGLE WebRender use case, it needs to handle a use case of updating D3d11Device when device reset happens. When the device reset happens, gecko creates new CompositorDevice. With the new CompositorDevice, EGLDisplay needs to be recreated.

It is necessary for Bug 1364504.
Blocks: 1364504
Assignee: nobody → sotaro.ikeda.g
Attachment #8951173 - Flags: review?(jgilbert)
:jgilbert, review ping.
Flags: needinfo?(jgilbert)
Flags: needinfo?(jgilbert)
:jgilbert, what is a better way to update EGLDisplay? Is attachment 8951173 [details] [diff] [review] OK? Or is there another better way to handle it?
Flags: needinfo?(jgilbert)
Comment on attachment 8951173 [details] [diff] [review]
patch - Add capability to update EGLDisplay to GLLibraryEGL

Review of attachment 8951173 [details] [diff] [review]:
-----------------------------------------------------------------

This should do for now.
We really need to make this non-static.

::: gfx/gl/GLLibraryEGL.cpp
@@ +683,5 @@
> +
> +EGLDisplay
> +GLLibraryEGL::CreateDisplay(bool forceAccel, const nsCOMPtr<nsIGfxInfo>& gfxInfo, nsACString* const out_failureId)
> +{
> +    EGLDisplay chosenDisplay = nullptr;

MOZ_ASSERT(!mInitialized)
Attachment #8951173 - Flags: review?(jgilbert) → review+
Flags: needinfo?(jgilbert)
Comment on attachment 8951173 [details] [diff] [review]
patch - Add capability to update EGLDisplay to GLLibraryEGL

Review of attachment 8951173 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/gl/GLLibraryEGL.cpp
@@ +684,5 @@
> +EGLDisplay
> +GLLibraryEGL::CreateDisplay(bool forceAccel, const nsCOMPtr<nsIGfxInfo>& gfxInfo, nsACString* const out_failureId)
> +{
> +    EGLDisplay chosenDisplay = nullptr;
> +

When we want to re-create EGLDisplay for updating default EGLDisplay, mInitialized is true.
Comment on attachment 8951173 [details] [diff] [review]
patch - Add capability to update EGLDisplay to GLLibraryEGL

Review of attachment 8951173 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/gl/GLLibraryEGL.cpp
@@ +684,5 @@
> +EGLDisplay
> +GLLibraryEGL::CreateDisplay(bool forceAccel, const nsCOMPtr<nsIGfxInfo>& gfxInfo, nsACString* const out_failureId)
> +{
> +    EGLDisplay chosenDisplay = nullptr;
> +

We never want to recreate. We need to do a full-teardown each time.
Attachment #8951173 - Flags: review+ → review-
(In reply to Jeff Gilbert [:jgilbert] from comment #7)
> > +
> 
> We never want to recreate. We need to do a full-teardown each time.

:jgilbert, what does the full-teardown mean? Full-teardon of GPU process?
Flags: needinfo?(jgilbert)
attachment 8951173 [details] [diff] [review] added GLLibraryEGL::RecreateDisplay() to handle device reset without full-teardon of GPU process.
We shouldn't move between two active-looking displays. We should completely reset to our starting state before trying to bring up a new display. This keeps the number of possible valid states and state transitions to a minimum.
Flags: needinfo?(jgilbert)
Thanks for the reply. I am going to reset GPU process to handle device reset for now. And this bug is changed just to add GLLibraryEGL::CreateDisplay().
Summary: Add capability to update EGLDisplay to GLLibraryEGL → Add GLLibraryEGL::CreateDisplay()
No longer blocks: 1364504, stage-wr-trains
Attachment #8951173 - Attachment is obsolete: true
Comment on attachment 8964480 [details] [diff] [review]
patch - Add GLLibraryEGL::CreateDisplay()

:jgilbert, can you review again?
Attachment #8964480 - Flags: review?(jgilbert)
attachment 8964480 [details] [diff] [review] applied your comment.
Attachment #8964480 - Flags: review?(jgilbert) → review+
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/86b13fe023ca
Add GLLibraryEGL::CreateDisplay() r=jgilbert
https://hg.mozilla.org/mozilla-central/rev/86b13fe023ca
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.