Implement support for using GL ES 3.0 with command buffer

Implement support for using GL ES 3.0 with command buffer

Adds a new 'api': --config gpu(api=commandbuffer3) for dm/nanobench.

BUG=skia:4943
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1684413003

Committed: https://skia.googlesource.com/skia/+/45c2c8166bbd84a87e29fdd344b39e36e8a28a3f

[Kimmo Kinnunen, 2016-02-12 08:10:28]

Description was changed from

==========
Implement support for using GL ES 3.0 with command buffer

Adds a new 'api': --config gpu(api=commandbuffer3) for dm/nanobench.

BUG=skia:4943
==========

to

==========
Implement support for using GL ES 3.0 with command buffer

Adds a new 'api': --config gpu(api=commandbuffer3) for dm/nanobench.

BUG=skia:4943
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[Kimmo Kinnunen, 2016-02-12 08:12:19]

kkinnunen@nvidia.com changed reviewers:
+ bsalomon@google.com

[Kimmo Kinnunen, 2016-02-12 08:12:20]

Would this complicate things too much?

[bsalomon, 2016-02-12 14:03:27]

Seems like a good idea to me.

https://codereview.chromium.org/1684413003/diff/1/include/gpu/gl/command_buff...
File include/gpu/gl/command_buffer/SkCommandBufferGLContext.h (right):

https://codereview.chromium.org/1684413003/diff/1/include/gpu/gl/command_buff...
include/gpu/gl/command_buffer/SkCommandBufferGLContext.h:53: enum ContextVersion
{
Can you either add suffixes (_ContextVersion) or make this an enum class?

https://codereview.chromium.org/1684413003/diff/1/src/gpu/GrContextFactory.h
File src/gpu/GrContextFactory.h (right):

https://codereview.chromium.org/1684413003/diff/1/src/gpu/GrContextFactory.h#...
src/gpu/GrContextFactory.h:37: kCommandBuffer_GLContextType, //! Chromium
command buffer OpenGL ES context.
Can we rename this CommandBufferES2?

https://codereview.chromium.org/1684413003/diff/1/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

https://codereview.chromium.org/1684413003/diff/1/src/gpu/gl/GrGLCaps.cpp#new...
src/gpu/gl/GrGLCaps.cpp:117: kChromium_GrGLDriver != ctxInfo.driver()) ||
Could we consolidate all the workarounds for an incomplete implementation in one
spot at the bottom? Or have a single loud bool set above bool
HACKS_FOR_INCOMPLETE_ES3_CMD_BUFFER? Just don't want these to get lost over
time.

https://codereview.chromium.org/1684413003/diff/1/src/gpu/gl/GrGLInterface.cpp
File src/gpu/gl/GrGLInterface.cpp (right):

https://codereview.chromium.org/1684413003/diff/1/src/gpu/gl/GrGLInterface.cp...
src/gpu/gl/GrGLInterface.cpp:220: GrGLDriver glDriver = kUnknown_GrGLDriver;
Similar request to above, instead of storing a driver var, can we have a bool
that indicates these are workarounds for command buffer issues.

e.g.

if (0 == strcmp(renderer, "Chromium")) {
   ALLOW_MISSING_FUNCTIONS_FOR_INCOMPLETE_CMD_BUFFER_ES3 = true;
}

[Kimmo Kinnunen, 2016-02-15 08:10:14]

https://codereview.chromium.org/1684413003/diff/1/include/gpu/gl/command_buff...
File include/gpu/gl/command_buffer/SkCommandBufferGLContext.h (right):

https://codereview.chromium.org/1684413003/diff/1/include/gpu/gl/command_buff...
include/gpu/gl/command_buffer/SkCommandBufferGLContext.h:53: enum ContextVersion
{
On 2016/02/12 14:03:27, bsalomon wrote:
> Can you either add suffixes (_ContextVersion) or make this an enum class?

Done.

https://codereview.chromium.org/1684413003/diff/1/src/gpu/GrContextFactory.h
File src/gpu/GrContextFactory.h (right):

https://codereview.chromium.org/1684413003/diff/1/src/gpu/GrContextFactory.h#...
src/gpu/GrContextFactory.h:37: kCommandBuffer_GLContextType, //! Chromium
command buffer OpenGL ES context.
On 2016/02/12 14:03:27, bsalomon wrote:
> Can we rename this CommandBufferES2?

Done.

https://codereview.chromium.org/1684413003/diff/1/src/gpu/gl/GrGLCaps.cpp
File src/gpu/gl/GrGLCaps.cpp (right):

https://codereview.chromium.org/1684413003/diff/1/src/gpu/gl/GrGLCaps.cpp#new...
src/gpu/gl/GrGLCaps.cpp:117: kChromium_GrGLDriver != ctxInfo.driver()) ||
On 2016/02/12 14:03:27, bsalomon wrote:
> Could we consolidate all the workarounds for an incomplete implementation in
one
> spot at the bottom? Or have a single loud bool set above bool
> HACKS_FOR_INCOMPLETE_ES3_CMD_BUFFER? Just don't want these to get lost over
> time.

Done.

https://codereview.chromium.org/1684413003/diff/1/src/gpu/gl/GrGLInterface.cpp
File src/gpu/gl/GrGLInterface.cpp (right):

https://codereview.chromium.org/1684413003/diff/1/src/gpu/gl/GrGLInterface.cp...
src/gpu/gl/GrGLInterface.cpp:220: GrGLDriver glDriver = kUnknown_GrGLDriver;
On 2016/02/12 14:03:27, bsalomon wrote:
> Similar request to above, instead of storing a driver var, can we have a bool
> that indicates these are workarounds for command buffer issues.
> 
> e.g.
> 
> if (0 == strcmp(renderer, "Chromium")) {
>    ALLOW_MISSING_FUNCTIONS_FOR_INCOMPLETE_CMD_BUFFER_ES3 = true;
> }

Done.

[Kimmo Kinnunen, 2016-02-15 10:07:01]

https://codereview.chromium.org/1684413003/diff/60001/src/gpu/gl/command_buff...
File src/gpu/gl/command_buffer/SkCommandBufferGLContext.cpp (right):

https://codereview.chromium.org/1684413003/diff/60001/src/gpu/gl/command_buff...
src/gpu/gl/command_buffer/SkCommandBufferGLContext.cpp:221: &numConfigs) ||
numConfigs <= 0) {
Changed this too..

[bsalomon, 2016-02-16 14:20:09]

lgtm

[Kimmo Kinnunen, 2016-02-25 09:37:36]

I changed the eglMakeCurrent, eglDestroyContext order after LGTM. This was done
in order to avoid command buffer client-side error debug print.
Taking the liberty to push the commit still.

https://codereview.chromium.org/1684413003/diff/80001/src/gpu/gl/command_buff...
File src/gpu/gl/command_buffer/SkCommandBufferGLContext.cpp (right):

https://codereview.chromium.org/1684413003/diff/80001/src/gpu/gl/command_buff...
src/gpu/gl/command_buffer/SkCommandBufferGLContext.cpp:302: fDisplay =
EGL_NO_DISPLAY;
This was changed after review.

[Kimmo Kinnunen, 2016-02-25 09:43:06]

The CQ bit was checked by kkinnunen@nvidia.com

[Kimmo Kinnunen, 2016-02-25 09:43:07]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1684413003/#ps100001
(title: "comment wording")

[commit-bot: I haz the power, 2016-02-25 09:43:16]

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

[commit-bot: I haz the power, 2016-02-25 10:03:47]

Description was changed from

==========
Implement support for using GL ES 3.0 with command buffer

Adds a new 'api': --config gpu(api=commandbuffer3) for dm/nanobench.

BUG=skia:4943
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Implement support for using GL ES 3.0 with command buffer

Adds a new 'api': --config gpu(api=commandbuffer3) for dm/nanobench.

BUG=skia:4943
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/45c2c8166bbd84a87e29fdd344b39e36e8a28a3f
==========

[commit-bot: I haz the power, 2016-02-25 10:03:47]

Committed patchset #6 (id:100001) as
https://skia.googlesource.com/skia/+/45c2c8166bbd84a87e29fdd344b39e36e8a28a3f

[Kimmo Kinnunen, 2016-02-29 06:51:29]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1744963002/ by kkinnunen@nvidia.com.

The reason for reverting is: The ES3 implementation in command_buffer_gles2 got
reverted.
To get the implementation back, further work would be needed. I'm not sure I can
do that in timely manner or at all.
Better to revert this one, for the time being.
https://bugs.chromium.org/p/chromium/issues/detail?id=581634#c14.