Revert of command_buffer_gles2: Implement EGL default Display as a global object

Revert of command_buffer_gles2: Implement EGL default Display as a global object (patchset #16 id:300001 of https://codereview.chromium.org/1714883002/ )

Reason for revert:
Broke GPU build on Linux, more info in http://crbug.com/581634#c18.

Original issue's description:
> command_buffer_gles2: Implement EGL default Display as a global object
>
> Make EGL Display a global. eglGetDisplay(EGL_DEFAULT_DISPLAY) will
> return the same object.
>
> Store EGL state in a new thread-local object (ThreadState). One
> display, multiple surfaces, multiple contexts.
>
> Fix the currently implemented entry points to return specified errors
> and behave as specified. Notable changes:
> * eglGetDisplay() does not create new displays for each invocation
> * eglTerminate() invalidates the created egl objects but leaves the
>   current context intact as specified
> * eglMakeCurrent() is implemented more correctly with refcount semantics
> * eglQueryStrings() works without display for some names
> * eglReleaseThread() is implemeted
> * Config are now separate for pbuffers and windows, so that contexts
>   can be created separate from surfaces
> * Context can be made current with different surfaces
> * Catches errors related to context being set current to multiple
>   threads
> * Tries to be thread-safe
>
> In the implementation, the Display contains a lock ensuring all thread
> accesses are consistent. All functions using or changing thread-local
> state have explicit egl::ThreadState parameter.
>
> BUG=581634
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
>
> Committed: https://crrev.com/de9aa3be41bca094454babbd62907815fad7f5c2
> Cr-Commit-Position: refs/heads/master@{#377535}
>
> Committed: https://crrev.com/7b01a0867be0895009b120f37fcd4166a84f404b
> Cr-Commit-Position: refs/heads/master@{#386613}

TBR=piman@chromium.org,thakis@chromium.org,kkinnunen@nvidia.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=581634
Committed: https://crrev.com/938d4b784441bf7a31bce765cca7dd964b0098ec
Cr-Commit-Position: refs/heads/master@{#386626}

[vabr (Chromium), 2016-04-12 08:34:21]

Created Revert of command_buffer_gles2: Implement EGL default Display as a
global object

[commit-bot: I haz the power, 2016-04-12 08:34:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1878133003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1878133003/1

[vabr (Chromium), 2016-04-12 08:34:48]

The CQ bit was unchecked by vabr@chromium.org

[vabr (Chromium), 2016-04-12 08:35:10]

On 2016/04/12 08:34:48, vabr (Chromium) wrote:
> The CQ bit was unchecked by mailto:vabr@chromium.org

Let me run the trybots to make sure this won't break anything more.

[commit-bot: I haz the power, 2016-04-12 08:35:25]

Committed patchset #1 (id:1)

[vabr (Chromium), 2016-04-12 08:35:27]

Description was changed from

==========
Revert of command_buffer_gles2: Implement EGL default Display as a global object
(patchset #16 id:300001 of https://codereview.chromium.org/1714883002/ )

Reason for revert:
Broke GPU build on Linux, more info in http://crbug.com/581634#c18.

Original issue's description:
> command_buffer_gles2: Implement EGL default Display as a global object
> 
> Make EGL Display a global. eglGetDisplay(EGL_DEFAULT_DISPLAY) will
> return the same object.
> 
> Store EGL state in a new thread-local object (ThreadState). One
> display, multiple surfaces, multiple contexts.
> 
> Fix the currently implemented entry points to return specified errors
> and behave as specified. Notable changes:
> * eglGetDisplay() does not create new displays for each invocation
> * eglTerminate() invalidates the created egl objects but leaves the
>   current context intact as specified
> * eglMakeCurrent() is implemented more correctly with refcount semantics
> * eglQueryStrings() works without display for some names
> * eglReleaseThread() is implemeted
> * Config are now separate for pbuffers and windows, so that contexts
>   can be created separate from surfaces
> * Context can be made current with different surfaces
> * Catches errors related to context being set current to multiple
>   threads
> * Tries to be thread-safe
> 
> In the implementation, the Display contains a lock ensuring all thread
> accesses are consistent. All functions using or changing thread-local
> state have explicit egl::ThreadState parameter.
> 
> BUG=581634
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
> 
> Committed: https://crrev.com/de9aa3be41bca094454babbd62907815fad7f5c2
> Cr-Commit-Position: refs/heads/master@{#377535}
> 
> Committed: https://crrev.com/7b01a0867be0895009b120f37fcd4166a84f404b
> Cr-Commit-Position: refs/heads/master@{#386613}

TBR=piman@chromium.org,thakis@chromium.org,kkinnunen@nvidia.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=581634
==========

to

==========
Revert of command_buffer_gles2: Implement EGL default Display as a global object
(patchset #16 id:300001 of https://codereview.chromium.org/1714883002/ )

Reason for revert:
Broke GPU build on Linux, more info in http://crbug.com/581634#c18.

Original issue's description:
> command_buffer_gles2: Implement EGL default Display as a global object
> 
> Make EGL Display a global. eglGetDisplay(EGL_DEFAULT_DISPLAY) will
> return the same object.
> 
> Store EGL state in a new thread-local object (ThreadState). One
> display, multiple surfaces, multiple contexts.
> 
> Fix the currently implemented entry points to return specified errors
> and behave as specified. Notable changes:
> * eglGetDisplay() does not create new displays for each invocation
> * eglTerminate() invalidates the created egl objects but leaves the
>   current context intact as specified
> * eglMakeCurrent() is implemented more correctly with refcount semantics
> * eglQueryStrings() works without display for some names
> * eglReleaseThread() is implemeted
> * Config are now separate for pbuffers and windows, so that contexts
>   can be created separate from surfaces
> * Context can be made current with different surfaces
> * Catches errors related to context being set current to multiple
>   threads
> * Tries to be thread-safe
> 
> In the implementation, the Display contains a lock ensuring all thread
> accesses are consistent. All functions using or changing thread-local
> state have explicit egl::ThreadState parameter.
> 
> BUG=581634
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
> 
> Committed: https://crrev.com/de9aa3be41bca094454babbd62907815fad7f5c2
> Cr-Commit-Position: refs/heads/master@{#377535}
> 
> Committed: https://crrev.com/7b01a0867be0895009b120f37fcd4166a84f404b
> Cr-Commit-Position: refs/heads/master@{#386613}

TBR=piman@chromium.org,thakis@chromium.org,kkinnunen@nvidia.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
BUG=581634
==========

[vabr (Chromium), 2016-04-12 08:35:53]

On 2016/04/12 08:35:27, vabr (Chromium) wrote:
> Description was changed from
> 
> ==========
> Revert of command_buffer_gles2: Implement EGL default Display as a global
object
> (patchset #16 id:300001 of https://codereview.chromium.org/1714883002/ )
> 
> Reason for revert:
> Broke GPU build on Linux, more info in http://crbug.com/581634#c18.
> 
> Original issue's description:
> > command_buffer_gles2: Implement EGL default Display as a global object
> > 
> > Make EGL Display a global. eglGetDisplay(EGL_DEFAULT_DISPLAY) will
> > return the same object.
> > 
> > Store EGL state in a new thread-local object (ThreadState). One
> > display, multiple surfaces, multiple contexts.
> > 
> > Fix the currently implemented entry points to return specified errors
> > and behave as specified. Notable changes:
> > * eglGetDisplay() does not create new displays for each invocation
> > * eglTerminate() invalidates the created egl objects but leaves the
> >   current context intact as specified
> > * eglMakeCurrent() is implemented more correctly with refcount semantics
> > * eglQueryStrings() works without display for some names
> > * eglReleaseThread() is implemeted
> > * Config are now separate for pbuffers and windows, so that contexts
> >   can be created separate from surfaces
> > * Context can be made current with different surfaces
> > * Catches errors related to context being set current to multiple
> >   threads
> > * Tries to be thread-safe
> > 
> > In the implementation, the Display contains a lock ensuring all thread
> > accesses are consistent. All functions using or changing thread-local
> > state have explicit egl::ThreadState parameter.
> > 
> > BUG=581634
> > CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
> > 
> > Committed: https://crrev.com/de9aa3be41bca094454babbd62907815fad7f5c2
> > Cr-Commit-Position: refs/heads/master@{#377535}
> > 
> > Committed: https://crrev.com/7b01a0867be0895009b120f37fcd4166a84f404b
> > Cr-Commit-Position: refs/heads/master@{#386613}
> 
> mailto:TBR=piman@chromium.org,thakis@chromium.org,kkinnunen@nvidia.com
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=581634
> ==========
> 
> to
> 
> ==========
> Revert of command_buffer_gles2: Implement EGL default Display as a global
object
> (patchset #16 id:300001 of https://codereview.chromium.org/1714883002/ )
> 
> Reason for revert:
> Broke GPU build on Linux, more info in http://crbug.com/581634#c18.
> 
> Original issue's description:
> > command_buffer_gles2: Implement EGL default Display as a global object
> > 
> > Make EGL Display a global. eglGetDisplay(EGL_DEFAULT_DISPLAY) will
> > return the same object.
> > 
> > Store EGL state in a new thread-local object (ThreadState). One
> > display, multiple surfaces, multiple contexts.
> > 
> > Fix the currently implemented entry points to return specified errors
> > and behave as specified. Notable changes:
> > * eglGetDisplay() does not create new displays for each invocation
> > * eglTerminate() invalidates the created egl objects but leaves the
> >   current context intact as specified
> > * eglMakeCurrent() is implemented more correctly with refcount semantics
> > * eglQueryStrings() works without display for some names
> > * eglReleaseThread() is implemeted
> > * Config are now separate for pbuffers and windows, so that contexts
> >   can be created separate from surfaces
> > * Context can be made current with different surfaces
> > * Catches errors related to context being set current to multiple
> >   threads
> > * Tries to be thread-safe
> > 
> > In the implementation, the Display contains a lock ensuring all thread
> > accesses are consistent. All functions using or changing thread-local
> > state have explicit egl::ThreadState parameter.
> > 
> > BUG=581634
> > CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
> > 
> > Committed: https://crrev.com/de9aa3be41bca094454babbd62907815fad7f5c2
> > Cr-Commit-Position: refs/heads/master@{#377535}
> > 
> > Committed: https://crrev.com/7b01a0867be0895009b120f37fcd4166a84f404b
> > Cr-Commit-Position: refs/heads/master@{#386613}
> 
> mailto:TBR=piman@chromium.org,thakis@chromium.org,kkinnunen@nvidia.com
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> BUG=581634
> ==========

Hmm, too late, I'll check the tree then.

[commit-bot: I haz the power, 2016-04-12 08:36:49]

Description was changed from

==========
Revert of command_buffer_gles2: Implement EGL default Display as a global object
(patchset #16 id:300001 of https://codereview.chromium.org/1714883002/ )

Reason for revert:
Broke GPU build on Linux, more info in http://crbug.com/581634#c18.

Original issue's description:
> command_buffer_gles2: Implement EGL default Display as a global object
> 
> Make EGL Display a global. eglGetDisplay(EGL_DEFAULT_DISPLAY) will
> return the same object.
> 
> Store EGL state in a new thread-local object (ThreadState). One
> display, multiple surfaces, multiple contexts.
> 
> Fix the currently implemented entry points to return specified errors
> and behave as specified. Notable changes:
> * eglGetDisplay() does not create new displays for each invocation
> * eglTerminate() invalidates the created egl objects but leaves the
>   current context intact as specified
> * eglMakeCurrent() is implemented more correctly with refcount semantics
> * eglQueryStrings() works without display for some names
> * eglReleaseThread() is implemeted
> * Config are now separate for pbuffers and windows, so that contexts
>   can be created separate from surfaces
> * Context can be made current with different surfaces
> * Catches errors related to context being set current to multiple
>   threads
> * Tries to be thread-safe
> 
> In the implementation, the Display contains a lock ensuring all thread
> accesses are consistent. All functions using or changing thread-local
> state have explicit egl::ThreadState parameter.
> 
> BUG=581634
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
> 
> Committed: https://crrev.com/de9aa3be41bca094454babbd62907815fad7f5c2
> Cr-Commit-Position: refs/heads/master@{#377535}
> 
> Committed: https://crrev.com/7b01a0867be0895009b120f37fcd4166a84f404b
> Cr-Commit-Position: refs/heads/master@{#386613}

TBR=piman@chromium.org,thakis@chromium.org,kkinnunen@nvidia.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
BUG=581634
==========

to

==========
Revert of command_buffer_gles2: Implement EGL default Display as a global object
(patchset #16 id:300001 of https://codereview.chromium.org/1714883002/ )

Reason for revert:
Broke GPU build on Linux, more info in http://crbug.com/581634#c18.

Original issue's description:
> command_buffer_gles2: Implement EGL default Display as a global object
>
> Make EGL Display a global. eglGetDisplay(EGL_DEFAULT_DISPLAY) will
> return the same object.
>
> Store EGL state in a new thread-local object (ThreadState). One
> display, multiple surfaces, multiple contexts.
>
> Fix the currently implemented entry points to return specified errors
> and behave as specified. Notable changes:
> * eglGetDisplay() does not create new displays for each invocation
> * eglTerminate() invalidates the created egl objects but leaves the
>   current context intact as specified
> * eglMakeCurrent() is implemented more correctly with refcount semantics
> * eglQueryStrings() works without display for some names
> * eglReleaseThread() is implemeted
> * Config are now separate for pbuffers and windows, so that contexts
>   can be created separate from surfaces
> * Context can be made current with different surfaces
> * Catches errors related to context being set current to multiple
>   threads
> * Tries to be thread-safe
>
> In the implementation, the Display contains a lock ensuring all thread
> accesses are consistent. All functions using or changing thread-local
> state have explicit egl::ThreadState parameter.
>
> BUG=581634
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
>
> Committed: https://crrev.com/de9aa3be41bca094454babbd62907815fad7f5c2
> Cr-Commit-Position: refs/heads/master@{#377535}
>
> Committed: https://crrev.com/7b01a0867be0895009b120f37fcd4166a84f404b
> Cr-Commit-Position: refs/heads/master@{#386613}

TBR=piman@chromium.org,thakis@chromium.org,kkinnunen@nvidia.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=581634

Committed: https://crrev.com/938d4b784441bf7a31bce765cca7dd964b0098ec
Cr-Commit-Position: refs/heads/master@{#386626}
==========

[commit-bot: I haz the power, 2016-04-12 08:36:50]

Patchset 1 (id:??) landed as
https://crrev.com/938d4b784441bf7a31bce765cca7dd964b0098ec
Cr-Commit-Position: refs/heads/master@{#386626}

[vabr (Chromium), 2016-04-12 09:24:34]

> Hmm, too late, I'll check the tree then.

The tree looks fine, and the GPU build failures disappeared after the revert.

[Kimmo Kinnunen, 2016-04-12 11:38:51]

On 2016/04/12 09:24:34, vabr (Chromium) wrote:
> > Hmm, too late, I'll check the tree then.
> 
> The tree looks fine, and the GPU build failures disappeared after the revert.

Is it ok to re-try cq'ing the patch? I'll add the nognchecks to try to
circumvent the problem.
(I can not build the component that fails, it needs internal sources)

[vabr (Chromium), 2016-04-12 11:57:01]

On 2016/04/12 11:38:51, Kimmo Kinnunen wrote:
> On 2016/04/12 09:24:34, vabr (Chromium) wrote:
> > > Hmm, too late, I'll check the tree then.
> > 
> > The tree looks fine, and the GPU build failures disappeared after the
revert.
> 
> Is it ok to re-try cq'ing the patch? I'll add the nognchecks to try to
> circumvent the problem.
> (I can not build the component that fails, it needs internal sources)

Please coordinate with the sheriff
(https://build.chromium.org/p/chromium/console tells me kjellander@chromium.org
is one of them) if you think you risk breaking the bots again.

Thanks!
Vaclav

[vabr (Chromium), 2016-04-12 11:58:46]

On 2016/04/12 11:57:01, vabr (Chromium) wrote:
> On 2016/04/12 11:38:51, Kimmo Kinnunen wrote:
> > On 2016/04/12 09:24:34, vabr (Chromium) wrote:
> > > > Hmm, too late, I'll check the tree then.
> > > 
> > > The tree looks fine, and the GPU build failures disappeared after the
> revert.
> > 
> > Is it ok to re-try cq'ing the patch? I'll add the nognchecks to try to
> > circumvent the problem.
> > (I can not build the component that fails, it needs internal sources)
> 
> Please coordinate with the sheriff
> (https://build.chromium.org/p/chromium/console tells me
mailto:kjellander@chromium.org
> is one of them) if you think you risk breaking the bots again.
> 
> Thanks!
> Vaclav

Having read your message once more: masking GN failures instead of fixing them
sounds like a really bad idea. If you cannot reproduce the issue, please find
somebody who works on a similar area and can do the build for you, do not commit
code with wrong build dependencies.

[Kimmo Kinnunen, 2016-04-12 12:18:21]

On 2016/04/12 11:58:46, vabr (Chromium) wrote:
> Having read your message once more: masking GN failures instead of fixing them
> sounds like a really bad idea. If you cannot reproduce the issue, please find
> somebody who works on a similar area and can do the build for you, do not
commit
> code with wrong build dependencies.

It's not wrong build dependency. It's a special case that already exist and not
invented by me.

[vabr (Chromium), 2016-04-12 12:33:59]

On 2016/04/12 12:18:21, Kimmo Kinnunen wrote:
> On 2016/04/12 11:58:46, vabr (Chromium) wrote:
> > Having read your message once more: masking GN failures instead of fixing
them
> > sounds like a really bad idea. If you cannot reproduce the issue, please
find
> > somebody who works on a similar area and can do the build for you, do not
> commit
> > code with wrong build dependencies.
> 
> It's not wrong build dependency. It's a special case that already exist and
not
> invented by me.

Fair enough, let this be discussed with GPU OWNERS on the original CL
(https://codereview.chromium.org/1714883002/).