Rename ClientNativePixmapFactoryGbm to ClientNativePixmapFactoryDmabuf amd move to ui/gfx

Rename ClientNativePixmapFactoryGbm to ClientNativePixmapFactoryDmabuf amd move to ui/gfx

Also rename ClientNativePixmapGbm to ClientNativePixmapOpaque.
Also move client_native_pixmap_dmabuf.{cc,h} from ui/ozone to ui/gfx/linux.

All the touched code is not ozone specific.
This is a pre-step to enable glCreateImageCHROMIUM with linux dma buf on linux.

BUG=584248
R=reveman@chromium.org

Review-Url: https://codereview.chromium.org/2711933002
Cr-Commit-Position: refs/heads/master@{#459592}
Committed: https://chromium.googlesource.com/chromium/src/+/7a2645913c2a61f7c53d47f16879925ccae10509

[Julien Isorce, 2017-02-24 11:28:19]

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

[commit-bot: I haz the power, 2017-02-24 11:28:37]

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

[Julien Isorce, 2017-02-24 11:41:53]

Hi David,

This CL is also to allow calling
ui::ClientNativePixmapFactory::GetInstance()->ImportFromHandle from
GpuMemoryBufferImpl[-Ozone]NativePixmap::CreateFromHandle.

Please take a look. Thx.

The renaming CLs will be to rename
GpuMemoryBuffer{Factory,Impl}OzoneNativePixmap to
GpuMemoryBuffer{Factory,Impl}NativePixmap

[commit-bot: I haz the power, 2017-02-24 11:43:46]

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

[commit-bot: I haz the power, 2017-02-24 11:43: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-...)

[reveman, 2017-03-09 12:22:15]

lgtm as a next step.

Long term I'd really like to see client pixmap classes go away in favor of just
using the GMB interface directly as the only abstraction layer.
GpuMemoryBufferImplLinuxDmabuf + GLImageLinuxDmabuf

https://codereview.chromium.org/2711933002/diff/20001/ui/gfx/linux/client_nat...
File ui/gfx/linux/client_native_pixmap_dmabuf.h (right):

https://codereview.chromium.org/2711933002/diff/20001/ui/gfx/linux/client_nat...
ui/gfx/linux/client_native_pixmap_dmabuf.h:28: // Overridden from
ClientNativePixmap.
nit: s/from ClientNativePixmap./from ClientNativePixmap:/

[Julien Isorce, 2017-03-09 12:37:54]

On 2017/03/09 12:22:15, reveman wrote:
> lgtm as a next step.
> 
> Long term I'd really like to see client pixmap classes go away in favor of
just
> using the GMB interface directly as the only abstraction layer.
> GpuMemoryBufferImplLinuxDmabuf + GLImageLinuxDmabuf
> 
>
https://codereview.chromium.org/2711933002/diff/20001/ui/gfx/linux/client_nat...
> File ui/gfx/linux/client_native_pixmap_dmabuf.h (right):
> 
>
https://codereview.chromium.org/2711933002/diff/20001/ui/gfx/linux/client_nat...
> ui/gfx/linux/client_native_pixmap_dmabuf.h:28: // Overridden from
> ClientNativePixmap.
> nit: s/from ClientNativePixmap./from ClientNativePixmap:/

Hi David, thx for the review. Make sense what you suggested. Lets discuss about
it in details once this bug is done (i.e. listed CLs landed). In the meantime
and if you have time of course, please file a new bug with some details about
your suggestion. Thx again!

[spang, 2017-03-09 17:06:12]

On 2017/03/09 12:22:15, reveman wrote:
> lgtm as a next step.
> 
> Long term I'd really like to see client pixmap classes go away in favor of
just
> using the GMB interface directly as the only abstraction layer.
> GpuMemoryBufferImplLinuxDmabuf + GLImageLinuxDmabuf

This interface exists because GMBImpl abstraction is in a place that doesn't
make sense for ozone.. are you planning to move it somewhere we can use it
directly?

Otherwise I think you'll find that moving it creates either circular
dependencies or (more) abstraction leaks, neither of which are better than the
duplication which we have now.

> 
>
https://codereview.chromium.org/2711933002/diff/20001/ui/gfx/linux/client_nat...
> File ui/gfx/linux/client_native_pixmap_dmabuf.h (right):
> 
>
https://codereview.chromium.org/2711933002/diff/20001/ui/gfx/linux/client_nat...
> ui/gfx/linux/client_native_pixmap_dmabuf.h:28: // Overridden from
> ClientNativePixmap.
> nit: s/from ClientNativePixmap./from ClientNativePixmap:/

[Julien Isorce, 2017-03-14 17:52:44]

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

[commit-bot: I haz the power, 2017-03-14 17:53:33]

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, 2017-03-14 18:04:27]

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

[commit-bot: I haz the power, 2017-03-14 18:04:28]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[reveman, 2017-03-15 15:00:09]

On 2017/03/09 at 17:06:12, spang wrote:
> On 2017/03/09 12:22:15, reveman wrote:
> > lgtm as a next step.
> > 
> > Long term I'd really like to see client pixmap classes go away in favor of
just
> > using the GMB interface directly as the only abstraction layer.
> > GpuMemoryBufferImplLinuxDmabuf + GLImageLinuxDmabuf
> 
> This interface exists because GMBImpl abstraction is in a place that doesn't
make sense for ozone.. are you planning to move it somewhere we can use it
directly?
> 
> Otherwise I think you'll find that moving it creates either circular
dependencies or (more) abstraction leaks, neither of which are better than the
duplication which we have now.

I agree that circular dependencies are worse. I'm failing to see where that
would happen. I was hoping that we could just have some simply linux interface
for allocating dmabufs that would be implemented differently on linux and ozone
and then everything else GMB related lives in non-ozone specific code.

> 
> > 
> >
https://codereview.chromium.org/2711933002/diff/20001/ui/gfx/linux/client_nat...
> > File ui/gfx/linux/client_native_pixmap_dmabuf.h (right):
> > 
> >
https://codereview.chromium.org/2711933002/diff/20001/ui/gfx/linux/client_nat...
> > ui/gfx/linux/client_native_pixmap_dmabuf.h:28: // Overridden from
> > ClientNativePixmap.
> > nit: s/from ClientNativePixmap./from ClientNativePixmap:/

[spang, 2017-03-15 15:28:42]

On 2017/03/15 15:00:09, reveman wrote:
> On 2017/03/09 at 17:06:12, spang wrote:
> > On 2017/03/09 12:22:15, reveman wrote:
> > > lgtm as a next step.
> > > 
> > > Long term I'd really like to see client pixmap classes go away in favor of
> just
> > > using the GMB interface directly as the only abstraction layer.
> > > GpuMemoryBufferImplLinuxDmabuf + GLImageLinuxDmabuf
> > 
> > This interface exists because GMBImpl abstraction is in a place that doesn't
> make sense for ozone.. are you planning to move it somewhere we can use it
> directly?
> > 
> > Otherwise I think you'll find that moving it creates either circular
> dependencies or (more) abstraction leaks, neither of which are better than the
> duplication which we have now.
> 
> I agree that circular dependencies are worse. I'm failing to see where that
> would happen. I was hoping that we could just have some simply linux interface
> for allocating dmabufs that would be implemented differently on linux and
ozone
> and then everything else GMB related lives in non-ozone specific code.

If you restrict ozone to dmabuf I think you are likely right, but we
historically weren't willing to do that. If we don't make that restriction, then
there needs to be a way to choose the import, mapping, and unmapping primitives;
that's what's provided by the ClientNativePixmap interface.

For some examples, if you make a build of ozone for the Android NDK you'd need
to use gralloc in place of dmabuf. Or the chromecast guys might someday want to
do zero copy using code provided by their SoC vendor.. which probably does not
use dmabuf.

Ozone was designed for the long tail of devices, and we're not too fussed about
what kernel APIs you use to get zero copy, on principle.

> 
> > 
> > > 
> > >
>
https://codereview.chromium.org/2711933002/diff/20001/ui/gfx/linux/client_nat...
> > > File ui/gfx/linux/client_native_pixmap_dmabuf.h (right):
> > > 
> > >
>
https://codereview.chromium.org/2711933002/diff/20001/ui/gfx/linux/client_nat...
> > > ui/gfx/linux/client_native_pixmap_dmabuf.h:28: // Overridden from
> > > ClientNativePixmap.
> > > nit: s/from ClientNativePixmap./from ClientNativePixmap:/

[spang, 2017-03-15 15:40:32]

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

[spang, 2017-03-15 15:40:32]

https://codereview.chromium.org/2711933002/diff/40001/ui/gfx/BUILD.gn
File ui/gfx/BUILD.gn (right):

https://codereview.chromium.org/2711933002/diff/40001/ui/gfx/BUILD.gn#newcode17
ui/gfx/BUILD.gn:17: packages = [ "libdrm" ]
+phajdan

Is linking this unconditionally into the linux desktop build OK? Or do we need
to use dlopen?

https://codereview.chromium.org/2711933002/diff/40001/ui/gfx/BUILD.gn#newcode575
ui/gfx/BUILD.gn:575: deps += [ "//third_party/libdrm" ]
I think this conditional should be removed and just always depend on
//third_party/libdrm; there's already a conditional and pkg_config() there which
we shouldn't duplicate here.

[Julien Isorce, 2017-03-21 17:59:55]

https://codereview.chromium.org/2711933002/diff/40001/ui/gfx/BUILD.gn
File ui/gfx/BUILD.gn (right):

https://codereview.chromium.org/2711933002/diff/40001/ui/gfx/BUILD.gn#newcode575
ui/gfx/BUILD.gn:575: deps += [ "//third_party/libdrm" ]
On 2017/03/15 15:40:32, spang wrote:
> I think this conditional should be removed and just always depend on
> //third_party/libdrm; there's already a conditional and pkg_config() there
which
> we shouldn't duplicate here.

Done, thx.

[Julien Isorce, 2017-03-21 18:00:33]

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

[commit-bot: I haz the power, 2017-03-21 18:01:07]

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, 2017-03-21 20:46:49]

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

[commit-bot: I haz the power, 2017-03-21 20:46:50]

Dry run: This issue passed the CQ dry run.

[spang, 2017-03-22 01:47:05]

lgtm

[spang, 2017-03-22 01:48:18]

spang@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[spang, 2017-03-22 01:48:19]

https://codereview.chromium.org/2711933002/diff/60001/ui/gfx/BUILD.gn
File ui/gfx/BUILD.gn (right):

https://codereview.chromium.org/2711933002/diff/60001/ui/gfx/BUILD.gn#newcode5
ui/gfx/BUILD.gn:5: import("//build/config/linux/pkg_config.gni")
remove.

[Julien Isorce, 2017-03-22 12:40:16]

Thx for the review.

https://codereview.chromium.org/2711933002/diff/60001/ui/gfx/BUILD.gn
File ui/gfx/BUILD.gn (right):

https://codereview.chromium.org/2711933002/diff/60001/ui/gfx/BUILD.gn#newcode5
ui/gfx/BUILD.gn:5: import("//build/config/linux/pkg_config.gni")
On 2017/03/22 01:48:18, spang wrote:
> remove.

Done.

[Julien Isorce, 2017-03-22 12:40:29]

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

[commit-bot: I haz the power, 2017-03-22 12:40:41]

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, 2017-03-22 13:43:29]

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

[commit-bot: I haz the power, 2017-03-22 13:43:30]

Dry run: This issue passed the CQ dry run.

[Julien Isorce, 2017-03-22 13:44:46]

Description was changed from

==========
Rename ClientNativePixmapFactoryGbm to ClientNativePixmapFactoryDmabuf amd move
to ui/gfx

Also rename ClientNativePixmapGbm to ClientNativePixmapOpaque.
Also move client_native_pixmap_dmabuf.{cc,h} from ui/ozone to ui/gfx/linux.

All the touched code is not ozone specific.
This is a pre-step to enable glCreateImageCHROMIUM with linux dma buf on linux.

BUG=584248
R=reveman@chromium.org
==========

to

==========
Rename ClientNativePixmapFactoryGbm to ClientNativePixmapFactoryDmabuf amd move
to ui/gfx

Also rename ClientNativePixmapGbm to ClientNativePixmapOpaque.
Also move client_native_pixmap_dmabuf.{cc,h} from ui/ozone to ui/gfx/linux.

All the touched code is not ozone specific.
This is a pre-step to enable glCreateImageCHROMIUM with linux dma buf on linux.

BUG=584248
R=reveman@chromium.org
==========

[Julien Isorce, 2017-03-22 13:44:46]

julien.isorce@chromium.org changed reviewers:
+ sadrul@chromium.org

[Julien Isorce, 2017-03-22 13:45:18]

Hi sadrul@, please take a look, Thx.

[sadrul, 2017-03-22 16:55:38]

https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
File ui/gfx/linux/client_native_pixmap_dmabuf.h (right):

https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
ui/gfx/linux/client_native_pixmap_dmabuf.h:18: namespace ui {
I missed this in earlier reviews. Can you please change the namespace of this
(and in earlier cls) to gfx?

[Julien Isorce, 2017-03-22 17:21:18]

https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
File ui/gfx/linux/client_native_pixmap_dmabuf.h (right):

https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
ui/gfx/linux/client_native_pixmap_dmabuf.h:18: namespace ui {
On 2017/03/22 16:55:37, sadrul wrote:
> I missed this in earlier reviews. Can you please change the namespace of this
> (and in earlier cls) to gfx?

Ops I missed that sorry. I will submit a seprate CL to fix previous CLs. Thx

[Julien Isorce, 2017-03-22 18:24:11]

https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
File ui/gfx/linux/client_native_pixmap_dmabuf.h (right):

https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
ui/gfx/linux/client_native_pixmap_dmabuf.h:18: namespace ui {
On 2017/03/22 17:21:18, Julien Isorce wrote:
> On 2017/03/22 16:55:37, sadrul wrote:
> > I missed this in earlier reviews. Can you please change the namespace of
this
> > (and in earlier cls) to gfx?
> 
> Ops I missed that sorry. I will submit a seprate CL to fix previous CLs. Thx

The followning is causing me issues since it requires both --namespace to be ui
for OzonePlatform and and gfx for ClientNativePixmapFactory:

  args = [
    "--platform_list=" + rebase_path(platform_list_txt_file, root_build_dir),
    "--output_cc=" + rebase_path(constructor_list_cc_file, root_build_dir),
    "--namespace=gfx",
    "--typename=OzonePlatform",
    "--typename=ClientNativePixmapFactory",
    "--include=\"ui/gfx/client_native_pixmap_factory.h\"",
    "--include=\"ui/ozone/public/ozone_platform.h\"",
  ]

I do not know how to solve this at the moment.

[spang, 2017-03-22 22:49:36]

https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
File ui/gfx/linux/client_native_pixmap_dmabuf.h (right):

https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
ui/gfx/linux/client_native_pixmap_dmabuf.h:18: namespace ui {
On 2017/03/22 18:24:10, Julien Isorce wrote:
> On 2017/03/22 17:21:18, Julien Isorce wrote:
> > On 2017/03/22 16:55:37, sadrul wrote:
> > > I missed this in earlier reviews. Can you please change the namespace of
> this
> > > (and in earlier cls) to gfx?
> > 
> > Ops I missed that sorry. I will submit a seprate CL to fix previous CLs. Thx
> 
> The followning is causing me issues since it requires both --namespace to be
ui
> for OzonePlatform and and gfx for ClientNativePixmapFactory:
> 
>   args = [
>     "--platform_list=" + rebase_path(platform_list_txt_file, root_build_dir),
>     "--output_cc=" + rebase_path(constructor_list_cc_file, root_build_dir),
>     "--namespace=gfx",
>     "--typename=OzonePlatform",
>     "--typename=ClientNativePixmapFactory",
>     "--include=\"ui/gfx/client_native_pixmap_factory.h\"",
>     "--include=\"ui/ozone/public/ozone_platform.h\"",
>   ]
> 
> I do not know how to solve this at the moment.
> 

Try:

--namespace=ui
--typename=gfx::ClientNativePixmapFactory

[spang, 2017-03-22 22:51:45]

On 2017/03/22 22:49:36, spang wrote:
>
https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
> File ui/gfx/linux/client_native_pixmap_dmabuf.h (right):
> 
>
https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
> ui/gfx/linux/client_native_pixmap_dmabuf.h:18: namespace ui {
> On 2017/03/22 18:24:10, Julien Isorce wrote:
> > On 2017/03/22 17:21:18, Julien Isorce wrote:
> > > On 2017/03/22 16:55:37, sadrul wrote:
> > > > I missed this in earlier reviews. Can you please change the namespace of
> > this
> > > > (and in earlier cls) to gfx?
> > > 
> > > Ops I missed that sorry. I will submit a seprate CL to fix previous CLs.
Thx
> > 
> > The followning is causing me issues since it requires both --namespace to be
> ui
> > for OzonePlatform and and gfx for ClientNativePixmapFactory:
> > 
> >   args = [
> >     "--platform_list=" + rebase_path(platform_list_txt_file,
root_build_dir),
> >     "--output_cc=" + rebase_path(constructor_list_cc_file, root_build_dir),
> >     "--namespace=gfx",
> >     "--typename=OzonePlatform",
> >     "--typename=ClientNativePixmapFactory",
> >     "--include=\"ui/gfx/client_native_pixmap_factory.h\"",
> >     "--include=\"ui/ozone/public/ozone_platform.h\"",
> >   ]
> > 
> > I do not know how to solve this at the moment.
> > 
> 
> Try:
> 
> --namespace=ui
> --typename=gfx::ClientNativePixmapFactory

Actually, maybe that won't help. You might be stuck without changing the python
to strip out the namespace.

[Julien Isorce, 2017-03-22 22:52:10]

https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
File ui/gfx/linux/client_native_pixmap_dmabuf.h (right):

https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
ui/gfx/linux/client_native_pixmap_dmabuf.h:18: namespace ui {
On 2017/03/22 22:49:35, spang wrote:
> On 2017/03/22 18:24:10, Julien Isorce wrote:
> > On 2017/03/22 17:21:18, Julien Isorce wrote:
> > > On 2017/03/22 16:55:37, sadrul wrote:
> > > > I missed this in earlier reviews. Can you please change the namespace of
> > this
> > > > (and in earlier cls) to gfx?
> > > 
> > > Ops I missed that sorry. I will submit a seprate CL to fix previous CLs.
Thx
> > 
> > The followning is causing me issues since it requires both --namespace to be
> ui
> > for OzonePlatform and and gfx for ClientNativePixmapFactory:
> > 
> >   args = [
> >     "--platform_list=" + rebase_path(platform_list_txt_file,
root_build_dir),
> >     "--output_cc=" + rebase_path(constructor_list_cc_file, root_build_dir),
> >     "--namespace=gfx",
> >     "--typename=OzonePlatform",
> >     "--typename=ClientNativePixmapFactory",
> >     "--include=\"ui/gfx/client_native_pixmap_factory.h\"",
> >     "--include=\"ui/ozone/public/ozone_platform.h\"",
> >   ]
> > 
> > I do not know how to solve this at the moment.
> > 
> 
> Try:
> 
> --namespace=ui
> --typename=gfx::ClientNativePixmapFactory


Thx, well I tried earlier but I got some errors, can't remember the output
though. I will have another go.

[spang, 2017-03-22 22:55:09]

On 2017/03/22 22:52:10, Julien Isorce wrote:
>
https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
> File ui/gfx/linux/client_native_pixmap_dmabuf.h (right):
> 
>
https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
> ui/gfx/linux/client_native_pixmap_dmabuf.h:18: namespace ui {
> On 2017/03/22 22:49:35, spang wrote:
> > On 2017/03/22 18:24:10, Julien Isorce wrote:
> > > On 2017/03/22 17:21:18, Julien Isorce wrote:
> > > > On 2017/03/22 16:55:37, sadrul wrote:
> > > > > I missed this in earlier reviews. Can you please change the namespace
of
> > > this
> > > > > (and in earlier cls) to gfx?
> > > > 
> > > > Ops I missed that sorry. I will submit a seprate CL to fix previous CLs.
> Thx
> > > 
> > > The followning is causing me issues since it requires both --namespace to
be
> > ui
> > > for OzonePlatform and and gfx for ClientNativePixmapFactory:
> > > 
> > >   args = [
> > >     "--platform_list=" + rebase_path(platform_list_txt_file,
> root_build_dir),
> > >     "--output_cc=" + rebase_path(constructor_list_cc_file,
root_build_dir),
> > >     "--namespace=gfx",
> > >     "--typename=OzonePlatform",
> > >     "--typename=ClientNativePixmapFactory",
> > >     "--include=\"ui/gfx/client_native_pixmap_factory.h\"",
> > >     "--include=\"ui/ozone/public/ozone_platform.h\"",
> > >   ]
> > > 
> > > I do not know how to solve this at the moment.
> > > 
> > 
> > Try:
> > 
> > --namespace=ui
> > --typename=gfx::ClientNativePixmapFactory
> 
> 
> Thx, well I tried earlier but I got some errors, can't remember the output
> though. I will have another go.

I think that's what you actually want, but you probably need to add some code to
strip out the namespace prefix so that it doesn't try to form an identifier that
looks like "Create" + "gfx::ClientNativePixmapFactoryOzone" =
Creategfx::ClientNativePixmapFactoryOzone.

[Julien Isorce, 2017-03-22 22:58:29]

On 2017/03/22 22:55:09, spang wrote:
>
https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
> > ui/gfx/linux/client_native_pixmap_dmabuf.h:18: namespace ui {
> > On 2017/03/22 22:49:35, spang wrote:
> > > On 2017/03/22 18:24:10, Julien Isorce wrote:
> > > > On 2017/03/22 17:21:18, Julien Isorce wrote:
> > > > > On 2017/03/22 16:55:37, sadrul wrote:
> > > > > > I missed this in earlier reviews. Can you please change the
namespace
> of
> > > > this
> > > > > > (and in earlier cls) to gfx?
> > > > > 
> > > > > Ops I missed that sorry. I will submit a seprate CL to fix previous
CLs.
> > Thx
> > > > 
> > > > The followning is causing me issues since it requires both --namespace
to
> be
> > > ui
> > > > for OzonePlatform and and gfx for ClientNativePixmapFactory:
> > > > 
> > > >   args = [
> > > >     "--platform_list=" + rebase_path(platform_list_txt_file,
> > root_build_dir),
> > > >     "--output_cc=" + rebase_path(constructor_list_cc_file,
> root_build_dir),
> > > >     "--namespace=gfx",
> > > >     "--typename=OzonePlatform",
> > > >     "--typename=ClientNativePixmapFactory",
> > > >     "--include=\"ui/gfx/client_native_pixmap_factory.h\"",
> > > >     "--include=\"ui/ozone/public/ozone_platform.h\"",
> > > >   ]
> > > > 
> > > > I do not know how to solve this at the moment.
> > > > 
> > > 
> > > Try:
> > > 
> > > --namespace=ui
> > > --typename=gfx::ClientNativePixmapFactory
> > 
> > 
> > Thx, well I tried earlier but I got some errors, can't remember the output
> > though. I will have another go.
> 
> I think that's what you actually want, but you probably need to add some code
to
> strip out the namespace prefix so that it doesn't try to form an identifier
that
> looks like "Create" + "gfx::ClientNativePixmapFactoryOzone" =
> Creategfx::ClientNativePixmapFactoryOzone.

Indeed that was exactly this error! I will try your suggestion, thx.

[spang, 2017-03-23 01:11:18]

On 2017/03/22 22:58:29, Julien Isorce wrote:
> On 2017/03/22 22:55:09, spang wrote:
> >
>
https://codereview.chromium.org/2711933002/diff/80001/ui/gfx/linux/client_nat...
> > > ui/gfx/linux/client_native_pixmap_dmabuf.h:18: namespace ui {
> > > On 2017/03/22 22:49:35, spang wrote:
> > > > On 2017/03/22 18:24:10, Julien Isorce wrote:
> > > > > On 2017/03/22 17:21:18, Julien Isorce wrote:
> > > > > > On 2017/03/22 16:55:37, sadrul wrote:
> > > > > > > I missed this in earlier reviews. Can you please change the
> namespace
> > of
> > > > > this
> > > > > > > (and in earlier cls) to gfx?
> > > > > > 
> > > > > > Ops I missed that sorry. I will submit a seprate CL to fix previous
> CLs.
> > > Thx
> > > > > 
> > > > > The followning is causing me issues since it requires both --namespace
> to
> > be
> > > > ui
> > > > > for OzonePlatform and and gfx for ClientNativePixmapFactory:
> > > > > 
> > > > >   args = [
> > > > >     "--platform_list=" + rebase_path(platform_list_txt_file,
> > > root_build_dir),
> > > > >     "--output_cc=" + rebase_path(constructor_list_cc_file,
> > root_build_dir),
> > > > >     "--namespace=gfx",
> > > > >     "--typename=OzonePlatform",
> > > > >     "--typename=ClientNativePixmapFactory",
> > > > >     "--include=\"ui/gfx/client_native_pixmap_factory.h\"",
> > > > >     "--include=\"ui/ozone/public/ozone_platform.h\"",
> > > > >   ]
> > > > > 
> > > > > I do not know how to solve this at the moment.
> > > > > 
> > > > 
> > > > Try:
> > > > 
> > > > --namespace=ui
> > > > --typename=gfx::ClientNativePixmapFactory
> > > 
> > > 
> > > Thx, well I tried earlier but I got some errors, can't remember the output
> > > though. I will have another go.
> > 
> > I think that's what you actually want, but you probably need to add some
code
> to
> > strip out the namespace prefix so that it doesn't try to form an identifier
> that
> > looks like "Create" + "gfx::ClientNativePixmapFactoryOzone" =
> > Creategfx::ClientNativePixmapFactoryOzone.
> 
> Indeed that was exactly this error! I will try your suggestion, thx.

You can probably also do something like

  --using=gfx::ClientNativePixmapFactory

and emitting the statement

  using gfx::ClientNativePixmapFactory;

at the top of the file.

[Julien Isorce, 2017-03-23 15:41:03]

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

[commit-bot: I haz the power, 2017-03-23 15:41:27]

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, 2017-03-23 16:25:50]

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

[commit-bot: I haz the power, 2017-03-23 16:25:51]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[Julien Isorce, 2017-03-23 19:00:27]

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

[commit-bot: I haz the power, 2017-03-23 19:00:58]

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, 2017-03-23 20:46:06]

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

[commit-bot: I haz the power, 2017-03-23 20:46:07]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-03-24 17:23:07]

lgtm

https://codereview.chromium.org/2711933002/diff/140001/ui/gfx/linux/client_na...
File ui/gfx/linux/client_native_pixmap_factory_dmabuf.h (right):

https://codereview.chromium.org/2711933002/diff/140001/ui/gfx/linux/client_na...
ui/gfx/linux/client_native_pixmap_factory_dmabuf.h:1: // Copyright 2015 The
Chromium Authors. All rights reserved.
2017

[Julien Isorce, 2017-03-24 18:14:21]

Thx for the review.

https://codereview.chromium.org/2711933002/diff/140001/ui/gfx/linux/client_na...
File ui/gfx/linux/client_native_pixmap_factory_dmabuf.h (right):

https://codereview.chromium.org/2711933002/diff/140001/ui/gfx/linux/client_na...
ui/gfx/linux/client_native_pixmap_factory_dmabuf.h:1: // Copyright 2015 The
Chromium Authors. All rights reserved.
On 2017/03/24 17:23:07, sadrul wrote:
> 2017

Done.

[Julien Isorce, 2017-03-24 18:14:30]

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

[commit-bot: I haz the power, 2017-03-24 18:15:11]

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, 2017-03-24 19:34:13]

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

[commit-bot: I haz the power, 2017-03-24 19:34:14]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Julien Isorce, 2017-03-24 22:30:20]

The CQ bit was checked by julien.isorce@chromium.org

[Julien Isorce, 2017-03-24 22:30:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
reveman@chromium.org, spang@chromium.org, sadrul@chromium.org
Link to the patchset: https://codereview.chromium.org/2711933002/#ps160001
(title: "Addressed sadrul's remark")

[commit-bot: I haz the power, 2017-03-24 22:31:06]

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, 2017-03-24 23:08:20]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1490394620293790,
"parent_rev": "43dd6c1192766bf70c69236bf3cac35aaa3ec79a", "commit_rev":
"7a2645913c2a61f7c53d47f16879925ccae10509"}

[commit-bot: I haz the power, 2017-03-24 23:09:26]

Description was changed from

==========
Rename ClientNativePixmapFactoryGbm to ClientNativePixmapFactoryDmabuf amd move
to ui/gfx

Also rename ClientNativePixmapGbm to ClientNativePixmapOpaque.
Also move client_native_pixmap_dmabuf.{cc,h} from ui/ozone to ui/gfx/linux.

All the touched code is not ozone specific.
This is a pre-step to enable glCreateImageCHROMIUM with linux dma buf on linux.

BUG=584248
R=reveman@chromium.org
==========

to

==========
Rename ClientNativePixmapFactoryGbm to ClientNativePixmapFactoryDmabuf amd move
to ui/gfx

Also rename ClientNativePixmapGbm to ClientNativePixmapOpaque.
Also move client_native_pixmap_dmabuf.{cc,h} from ui/ozone to ui/gfx/linux.

All the touched code is not ozone specific.
This is a pre-step to enable glCreateImageCHROMIUM with linux dma buf on linux.

BUG=584248
R=reveman@chromium.org

Review-Url: https://codereview.chromium.org/2711933002
Cr-Commit-Position: refs/heads/master@{#459592}
Committed:
https://chromium.googlesource.com/chromium/src/+/7a2645913c2a61f7c53d47f16879...
==========

[commit-bot: I haz the power, 2017-03-24 23:09:27]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/7a2645913c2a61f7c53d47f16879...