Convert Ozone GBM to use new surface API.

Convert Ozone GBM to use new surface API.

The Ozone GBM platform now directly creates GLSurfaces instead of
creating SurfaceOzoneEGL. The base classes for surfaceless and
framebuffer backed GLSurfaces is moved into //ui/ozone/platform/drm.
The old GbmSurfaceless class is replaced entirely despite retaining the
same name. A new GbmSurface class is added for the framebuffer backed
surface.

Add a new version of ChooseEGLConfig() to egl_util.cc/h that doesn't
rely on any callbacks now that deps problem as been fixed. The old
version will be deleted when this refactor is done as it will be unused.

BUG=611142
Committed: https://crrev.com/b3d66d0ac16f7569abe9be477e5ac58e50ca2362
Cr-Commit-Position: refs/heads/master@{#407574}

[kylechar, 2016-07-21 17:17:32]

kylechar@chromium.org changed reviewers:
+ dnicoara@chromium.org

[dnicoara, 2016-07-21 17:41:37]

dnicoara@chromium.org changed reviewers:
+ spang@chromium.org

[dnicoara, 2016-07-21 17:41:38]

+ spang@ for the common Ozone nits and for the naming bickeshed.

lgtm otherwise

https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
File ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.cc (right):

https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.cc:80: return
gfx::SwapResult::SWAP_FAILED;
nit: Add a NOTREACHED() just in case.

https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
File ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.h (right):

https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.h:25: class
GLSurfaceGbmSurfaceless : public gl::SurfacelessEGL {
nit: I'm thinking we could simplify the naming a bit; how about GbmSurfaceless?

Alternatively, if you like the longer form, perhaps move the "Gbm" to be a
prefix for consistency?

[spang, 2016-07-21 17:53:03]

On 2016/07/21 17:41:38, dnicoara wrote:
> + spang@ for the common Ozone nits and for the naming bickeshed.
> 
> lgtm otherwise
> 
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> File ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.cc (right):
> 
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.cc:80: return
> gfx::SwapResult::SWAP_FAILED;
> nit: Add a NOTREACHED() just in case.
> 
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> File ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.h (right):
> 
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.h:25: class
> GLSurfaceGbmSurfaceless : public gl::SurfacelessEGL {
> nit: I'm thinking we could simplify the naming a bit; how about
GbmSurfaceless?
> 
> Alternatively, if you like the longer form, perhaps move the "Gbm" to be a
> prefix for consistency?

Maybe

GLSurfaceGbmSurfaceless - > GbmSurfaceless (IsSurfaceless() - > true)
GLSurfaceGbmBufferedSurfaceless -> GbmSurface  (IsSurfaceless() - > false)

The whole "surfaceless" name is kind of misleading so more radical renaming
might be helpful at some point (I dunno, "overlay surface" ?).

[kylechar, 2016-07-21 18:04:23]

On 2016/07/21 17:53:03, spang wrote:
> On 2016/07/21 17:41:38, dnicoara wrote:
> > + spang@ for the common Ozone nits and for the naming bickeshed.
> > 
> > lgtm otherwise
> > 
> >
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> > File ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.cc (right):
> > 
> >
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> > ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.cc:80: return
> > gfx::SwapResult::SWAP_FAILED;
> > nit: Add a NOTREACHED() just in case.
> > 
> >
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> > File ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.h (right):
> > 
> >
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> > ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.h:25: class
> > GLSurfaceGbmSurfaceless : public gl::SurfacelessEGL {
> > nit: I'm thinking we could simplify the naming a bit; how about
> GbmSurfaceless?
> > 
> > Alternatively, if you like the longer form, perhaps move the "Gbm" to be a
> > prefix for consistency?
> 
> Maybe
> 
> GLSurfaceGbmSurfaceless - > GbmSurfaceless (IsSurfaceless() - > true)
> GLSurfaceGbmBufferedSurfaceless -> GbmSurface  (IsSurfaceless() - > false)
> 
> The whole "surfaceless" name is kind of misleading so more radical renaming
> might be helpful at some point (I dunno, "overlay surface" ?).

I'd be totally okay with GbmSurfaceless and GbmSurface. Although I agree,
surfaceless is a pretty confusing term. Could do something like:

GLSurfaceGbmSurfaceless - > GbmOverlaySurface
GLSurfaceGbmBufferedSurfaceless -> Gbm???OverlaySurface (maybe
GbmBufferedOverlaySurface or GbmFbOverlaySurface?)

[dnicoara, 2016-07-21 18:08:21]

On 2016/07/21 18:04:23, kylechar wrote:
> On 2016/07/21 17:53:03, spang wrote:
> > On 2016/07/21 17:41:38, dnicoara wrote:
> > > + spang@ for the common Ozone nits and for the naming bickeshed.
> > > 
> > > lgtm otherwise
> > > 
> > >
> >
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> > > File ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> > > ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.cc:80: return
> > > gfx::SwapResult::SWAP_FAILED;
> > > nit: Add a NOTREACHED() just in case.
> > > 
> > >
> >
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> > > File ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> > > ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.h:25: class
> > > GLSurfaceGbmSurfaceless : public gl::SurfacelessEGL {
> > > nit: I'm thinking we could simplify the naming a bit; how about
> > GbmSurfaceless?
> > > 
> > > Alternatively, if you like the longer form, perhaps move the "Gbm" to be a
> > > prefix for consistency?
> > 
> > Maybe
> > 
> > GLSurfaceGbmSurfaceless - > GbmSurfaceless (IsSurfaceless() - > true)
> > GLSurfaceGbmBufferedSurfaceless -> GbmSurface  (IsSurfaceless() - > false)
> > 
> > The whole "surfaceless" name is kind of misleading so more radical renaming
> > might be helpful at some point (I dunno, "overlay surface" ?).
> 
> I'd be totally okay with GbmSurfaceless and GbmSurface. Although I agree,
> surfaceless is a pretty confusing term. Could do something like:
> 
> GLSurfaceGbmSurfaceless - > GbmOverlaySurface
> GLSurfaceGbmBufferedSurfaceless -> Gbm???OverlaySurface (maybe
> GbmBufferedOverlaySurface or GbmFbOverlaySurface?)

I'd rather stay away from "Overlay" since it has a different meaning. (Note that
you can schedule overlay planes on a GLSurface)

GbmSurface and GbmSurfaceless are pretty simple and concrete. I don't think we
need to be more explicit.

[kylechar, 2016-07-21 18:34:08]

Changed the names to GbmSurfaceless and GbmSurface.

https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
File ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.cc (right):

https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.cc:80: return
gfx::SwapResult::SWAP_FAILED;
On 2016/07/21 17:41:38, dnicoara wrote:
> nit: Add a NOTREACHED() just in case.

Done.

https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
File ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.h (right):

https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.h:25: class
GLSurfaceGbmSurfaceless : public gl::SurfacelessEGL {
On 2016/07/21 17:41:38, dnicoara wrote:
> nit: I'm thinking we could simplify the naming a bit; how about
GbmSurfaceless?
> 
> Alternatively, if you like the longer form, perhaps move the "Gbm" to be a
> prefix for consistency?

Done.

[spang, 2016-07-21 18:42:47]

On 2016/07/21 18:08:21, dnicoara wrote:
> On 2016/07/21 18:04:23, kylechar wrote:
> > On 2016/07/21 17:53:03, spang wrote:
> > > On 2016/07/21 17:41:38, dnicoara wrote:
> > > > + spang@ for the common Ozone nits and for the naming bickeshed.
> > > > 
> > > > lgtm otherwise
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> > > > File ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.cc (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> > > > ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.cc:80: return
> > > > gfx::SwapResult::SWAP_FAILED;
> > > > nit: Add a NOTREACHED() just in case.
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> > > > File ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.h (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2165303002/diff/20001/ui/ozone/platform/drm/g...
> > > > ui/ozone/platform/drm/gpu/gl_surface_gbm_surfaceless.h:25: class
> > > > GLSurfaceGbmSurfaceless : public gl::SurfacelessEGL {
> > > > nit: I'm thinking we could simplify the naming a bit; how about
> > > GbmSurfaceless?
> > > > 
> > > > Alternatively, if you like the longer form, perhaps move the "Gbm" to be
a
> > > > prefix for consistency?
> > > 
> > > Maybe
> > > 
> > > GLSurfaceGbmSurfaceless - > GbmSurfaceless (IsSurfaceless() - > true)
> > > GLSurfaceGbmBufferedSurfaceless -> GbmSurface  (IsSurfaceless() - > false)
> > > 
> > > The whole "surfaceless" name is kind of misleading so more radical
renaming
> > > might be helpful at some point (I dunno, "overlay surface" ?).
> > 
> > I'd be totally okay with GbmSurfaceless and GbmSurface. Although I agree,
> > surfaceless is a pretty confusing term. Could do something like:
> > 
> > GLSurfaceGbmSurfaceless - > GbmOverlaySurface
> > GLSurfaceGbmBufferedSurfaceless -> Gbm???OverlaySurface (maybe
> > GbmBufferedOverlaySurface or GbmFbOverlaySurface?)
> 
> I'd rather stay away from "Overlay" since it has a different meaning. (Note
that
> you can schedule overlay planes on a GLSurface)

I'd say overlay has exactly the right meaning. For an "overlay" surface you must
use ScheduleOverlayPlane(). For a non-overlay surface you get traditional
SwapBuffers().

> 
> GbmSurface and GbmSurfaceless are pretty simple and concrete. I don't think we
> need to be more explicit.

[spang, 2016-07-21 19:05:28]

lgtm

[dnicoara, 2016-07-21 19:05:47]

lgtm

[kylechar, 2016-07-25 17:50:50]

Description was changed from

==========
Convert Ozone DRM to use new surface API.

The Ozone DRM platform now directly creates GLSurfaces instead of
creating SurfaceOzoneEGL. The base classes for surfaceless and buffered
surfaceless GLSurfaces is moved as only DRM uses them. The logic from
GbmSurfaceless is merged into the new GLSurface implementations.

Add a new version of ChooseEGLConfig() into egl_util.cc/h that doesn't
rely on any callbacks now that deps problem as been fixed. The old
version will be deleted when this refactor is done as it will be unused.

BUG=611142
==========

to

==========
Convert Ozone GBM to use new surface API.

The Ozone GBM platform now directly creates GLSurfaces instead of
creating SurfaceOzoneEGL. The base classes for surfaceless and
framebuffer backed GLSurfaces is moved into //ui/ozone/platform/drm.
The old GbmSurfaceless class is replaced entirely despite retaining the
same name. A new GbmSurface class is added for the framebuffer backed
surface.

Add a new version of ChooseEGLConfig() to egl_util.cc/h that doesn't
rely on any callbacks now that deps problem as been fixed. The old
version will be deleted when this refactor is done as it will be unused.

BUG=611142
==========

[kylechar, 2016-07-25 17:51:01]

Description was changed from

==========
Convert Ozone GBM to use new surface API.

The Ozone GBM platform now directly creates GLSurfaces instead of
creating SurfaceOzoneEGL. The base classes for surfaceless and
framebuffer backed GLSurfaces is moved into //ui/ozone/platform/drm.
The old GbmSurfaceless class is replaced entirely despite retaining the
same name. A new GbmSurface class is added for the framebuffer backed
surface.

Add a new version of ChooseEGLConfig() to egl_util.cc/h that doesn't
rely on any callbacks now that deps problem as been fixed. The old
version will be deleted when this refactor is done as it will be unused.

BUG=611142
==========

to

==========
Convert Ozone GBM to use new surface API.

The Ozone GBM platform now directly creates GLSurfaces instead of
creating SurfaceOzoneEGL. The base classes for surfaceless and
framebuffer backed GLSurfaces is moved into //ui/ozone/platform/drm.
The old GbmSurfaceless class is replaced entirely despite retaining the
same name. A new GbmSurface class is added for the framebuffer backed
surface.

Add a new version of ChooseEGLConfig() to egl_util.cc/h that doesn't
rely on any callbacks now that deps problem as been fixed. The old
version will be deleted when this refactor is done as it will be unused.

BUG=611142
==========

[kylechar, 2016-07-25 17:58:59]

The CQ bit was checked by kylechar@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-25 17:59:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-25 18:04:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-25 18:04:47]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[kylechar, 2016-07-25 20:00:13]

The CQ bit was checked by kylechar@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-25 20:00:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-25 21:04:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-25 21:04:07]

Dry run: This issue passed the CQ dry run.

[kylechar, 2016-07-25 21:05:27]

The CQ bit was checked by kylechar@chromium.org

[kylechar, 2016-07-25 21:05:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
dnicoara@chromium.org, spang@chromium.org
Link to the patchset: https://codereview.chromium.org/2165303002/#ps100001
(title: "Add missing dep.")

[commit-bot: I haz the power, 2016-07-25 21:06:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-25 21:16:25]

Description was changed from

==========
Convert Ozone GBM to use new surface API.

The Ozone GBM platform now directly creates GLSurfaces instead of
creating SurfaceOzoneEGL. The base classes for surfaceless and
framebuffer backed GLSurfaces is moved into //ui/ozone/platform/drm.
The old GbmSurfaceless class is replaced entirely despite retaining the
same name. A new GbmSurface class is added for the framebuffer backed
surface.

Add a new version of ChooseEGLConfig() to egl_util.cc/h that doesn't
rely on any callbacks now that deps problem as been fixed. The old
version will be deleted when this refactor is done as it will be unused.

BUG=611142
==========

to

==========
Convert Ozone GBM to use new surface API.

The Ozone GBM platform now directly creates GLSurfaces instead of
creating SurfaceOzoneEGL. The base classes for surfaceless and
framebuffer backed GLSurfaces is moved into //ui/ozone/platform/drm.
The old GbmSurfaceless class is replaced entirely despite retaining the
same name. A new GbmSurface class is added for the framebuffer backed
surface.

Add a new version of ChooseEGLConfig() to egl_util.cc/h that doesn't
rely on any callbacks now that deps problem as been fixed. The old
version will be deleted when this refactor is done as it will be unused.

BUG=611142
==========

[commit-bot: I haz the power, 2016-07-25 21:16:26]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-07-25 21:19:56]

Description was changed from

==========
Convert Ozone GBM to use new surface API.

The Ozone GBM platform now directly creates GLSurfaces instead of
creating SurfaceOzoneEGL. The base classes for surfaceless and
framebuffer backed GLSurfaces is moved into //ui/ozone/platform/drm.
The old GbmSurfaceless class is replaced entirely despite retaining the
same name. A new GbmSurface class is added for the framebuffer backed
surface.

Add a new version of ChooseEGLConfig() to egl_util.cc/h that doesn't
rely on any callbacks now that deps problem as been fixed. The old
version will be deleted when this refactor is done as it will be unused.

BUG=611142
==========

to

==========
Convert Ozone GBM to use new surface API.

The Ozone GBM platform now directly creates GLSurfaces instead of
creating SurfaceOzoneEGL. The base classes for surfaceless and
framebuffer backed GLSurfaces is moved into //ui/ozone/platform/drm.
The old GbmSurfaceless class is replaced entirely despite retaining the
same name. A new GbmSurface class is added for the framebuffer backed
surface.

Add a new version of ChooseEGLConfig() to egl_util.cc/h that doesn't
rely on any callbacks now that deps problem as been fixed. The old
version will be deleted when this refactor is done as it will be unused.

BUG=611142

Committed: https://crrev.com/b3d66d0ac16f7569abe9be477e5ac58e50ca2362
Cr-Commit-Position: refs/heads/master@{#407574}
==========

[commit-bot: I haz the power, 2016-07-25 21:19:57]

Patchset 6 (id:??) landed as
https://crrev.com/b3d66d0ac16f7569abe9be477e5ac58e50ca2362
Cr-Commit-Position: refs/heads/master@{#407574}