1438456 - Add GLLibraryEGL::CreateDisplay()

Status: RESOLVED FIXED

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

Description

[Sotaro Ikeda [:sotaro]]

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.

Comment 1

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Add capability to update EGLDisplay to GLLibraryEGL

Comment 2

[Sotaro Ikeda [:sotaro]]

:jgilbert, review ping.

Comment 3

[Sotaro Ikeda [:sotaro]]

: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?

Comment 4

[Kelsey Gilbert [: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)

Comment 5

[Sotaro Ikeda [:sotaro]]

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 6

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec03a445c9941b153618549dab47042a7f12a8ce

Comment 7

[Kelsey Gilbert [: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;
> +

We never want to recreate. We need to do a full-teardown each time.

Comment 8

[Sotaro Ikeda [:sotaro]]

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

Comment 9

[Sotaro Ikeda [:sotaro]]

attachment 8951173 [details] [diff] [review] added GLLibraryEGL::RecreateDisplay() to handle device reset without full-teardon of GPU process.

Comment 10

[Kelsey Gilbert [:jgilbert]]

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.

Comment 11

[Sotaro Ikeda [:sotaro]]

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

Comment 12

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Add GLLibraryEGL::CreateDisplay()

Comment 13

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8964480 [details] [diff] [review]
patch - Add GLLibraryEGL::CreateDisplay()

:jgilbert, can you review again?

Comment 14

[Sotaro Ikeda [:sotaro]]

attachment 8964480 [details] [diff] [review] applied your comment.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/86b13fe023ca
Add GLLibraryEGL::CreateDisplay() r=jgilbert

Comment 16

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/86b13fe023ca