ozone: Allow using swiftshader with --use-gl=swiftshader.

ozone: Allow using swiftshader with --use-gl=swiftshader.

This hooks up the ozone code so it can deal with --use-gl=swiftshader
flag correctly.

BUG=704285
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2926423002
Cr-Commit-Position: refs/heads/master@{#478817}
Committed: https://chromium.googlesource.com/chromium/src/+/1e117683d1819528521a763e94162035d42f1826

[sadrul, 2017-06-08 22:55:54]

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

[commit-bot: I haz the power, 2017-06-08 22:56:31]

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-06-08 23:02:35]

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

[commit-bot: I haz the power, 2017-06-08 23:02:35]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  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-...)

[sadrul, 2017-06-09 02:43:09]

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

[commit-bot: I haz the power, 2017-06-09 02:43:26]

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-06-09 03:39:44]

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

[commit-bot: I haz the power, 2017-06-09 03:39:44]

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

[sadrul, 2017-06-09 14:55:31]

Description was changed from

==========
[wip] ozone: Allow using swiftshader with --use-gl=swiftshader.

This hooks up the ozone code so it can deal with --use-gl=swiftshader
flag correctly. This does not actually work yet though, failing to
load with the following error:

 swiftshader/libGLESv2.so: undefined symbol: _ZN3egl7Display14getSharedImageEPv

BUG=704285
==========

to

==========
[wip] ozone: Allow using swiftshader with --use-gl=swiftshader.

This hooks up the ozone code so it can deal with --use-gl=swiftshader
flag correctly.

BUG=704285
==========

[sadrul, 2017-06-09 15:04:17]

Description was changed from

==========
[wip] ozone: Allow using swiftshader with --use-gl=swiftshader.

This hooks up the ozone code so it can deal with --use-gl=swiftshader
flag correctly.

BUG=704285
==========

to

==========
[wip] ozone: Allow using swiftshader with --use-gl=swiftshader.

This hooks up the ozone code so it can deal with --use-gl=swiftshader
flag correctly.

BUG=704285
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[sadrul, 2017-06-09 15:04:35]

Description was changed from

==========
[wip] ozone: Allow using swiftshader with --use-gl=swiftshader.

This hooks up the ozone code so it can deal with --use-gl=swiftshader
flag correctly.

BUG=704285
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
ozone: Allow using swiftshader with --use-gl=swiftshader.

This hooks up the ozone code so it can deal with --use-gl=swiftshader
flag correctly.

BUG=704285
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[sadrul, 2017-06-09 15:05:43]

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

[commit-bot: I haz the power, 2017-06-09 15:05:56]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-06-09 15:08:04]

sadrul@chromium.org changed reviewers:
+ rjkroege@chromium.org

[sadrul, 2017-06-09 15:08:46]

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

[commit-bot: I haz the power, 2017-06-09 15:09:01]

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-06-09 16:39:22]

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

[commit-bot: I haz the power, 2017-06-09 16:39:23]

Dry run: This issue passed the CQ dry run.

[rjkroege, 2017-06-12 17:23:40]

lgtm

1  concern
1  question

Awesome! Thanks so much for doing.

https://codereview.chromium.org/2926423002/diff/80001/ui/ozone/platform/wayla...
File ui/ozone/platform/wayland/wayland_surface_factory.cc (right):

https://codereview.chromium.org/2926423002/diff/80001/ui/ozone/platform/wayla...
ui/ozone/platform/wayland/wayland_surface_factory.cc:180: setenv("EGL_PLATFORM",
"wayland", 0);
I suspect that the setenv should be skipped if we're using swiftshader. This is
a subtlety that we ought to ask tonikitoo@ about.

https://codereview.chromium.org/2926423002/diff/80001/ui/ozone/platform/x11/x...
File ui/ozone/platform/x11/x11_surface_factory.cc (right):

https://codereview.chromium.org/2926423002/diff/80001/ui/ozone/platform/x11/x...
ui/ozone/platform/x11/x11_surface_factory.cc:176: case
gl::kGLImplementationSwiftShaderGL:
it ought to be that the swiftshader oxygen build should work in soft vnc without
any special hackery. does it?

[tonikitoo, 2017-06-12 18:32:16]

tonikitoo@igalia.com changed reviewers:
+ tonikitoo@igalia.com

[tonikitoo, 2017-06-12 18:32:17]

+forney@

https://codereview.chromium.org/2926423002/diff/80001/ui/ozone/platform/wayla...
File ui/ozone/platform/wayland/wayland_surface_factory.cc (right):

https://codereview.chromium.org/2926423002/diff/80001/ui/ozone/platform/wayla...
ui/ozone/platform/wayland/wayland_surface_factory.cc:180: setenv("EGL_PLATFORM",
"wayland", 0);
On 2017/06/12 17:23:40, rjkroege wrote:
> I suspect that the setenv should be skipped if we're using swiftshader. This
is
> a subtlety that we ought to ask tonikitoo@ about.

I believe the comment pasted below is relevant (source:
https://github.com/01org/ozone-wayland/blob/master/wayland/display.cc#L167)

  // The variable EGL_PLATFORM specifies native platform to be used by the
  // drivers (atleast on Mesa). When the variable is not set, Mesa uses the
  // first platform listed in --with-egl-platforms during compilation. Thus, we
  // ensure here that wayland is set as the native platform. However, we don't
  // override the EGL_PLATFORM value in case it has already been set.

It make makes sense to me to skip setting this env variable in the case of
swiftshader use. However, I am not sure it guarantees Wayland rendering will
work (it might depend on how mesa is configured/built?).

[sadrul, 2017-06-12 19:36:59]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2017-06-12 19:37:00]

https://codereview.chromium.org/2926423002/diff/80001/ui/ozone/platform/wayla...
File ui/ozone/platform/wayland/wayland_surface_factory.cc (right):

https://codereview.chromium.org/2926423002/diff/80001/ui/ozone/platform/wayla...
ui/ozone/platform/wayland/wayland_surface_factory.cc:180: setenv("EGL_PLATFORM",
"wayland", 0);
On 2017/06/12 18:32:17, tonikitoo wrote:
> On 2017/06/12 17:23:40, rjkroege wrote:
> > I suspect that the setenv should be skipped if we're using swiftshader. This
> is
> > a subtlety that we ought to ask tonikitoo@ about.
> 
> I believe the comment pasted below is relevant (source:
> https://github.com/01org/ozone-wayland/blob/master/wayland/display.cc#L167)
> 
>   // The variable EGL_PLATFORM specifies native platform to be used by the
>   // drivers (atleast on Mesa). When the variable is not set, Mesa uses the
>   // first platform listed in --with-egl-platforms during compilation. Thus,
we
>   // ensure here that wayland is set as the native platform. However, we don't
>   // override the EGL_PLATFORM value in case it has already been set.
> 
> It make makes sense to me to skip setting this env variable in the case of
> swiftshader use. However, I am not sure it guarantees Wayland rendering will
> work (it might depend on how mesa is configured/built?).

Sounds like the safest option is to always set the env variable here for now,
until we can confirm it's not needed for swiftshader.

https://codereview.chromium.org/2926423002/diff/80001/ui/ozone/platform/x11/x...
File ui/ozone/platform/x11/x11_surface_factory.cc (right):

https://codereview.chromium.org/2926423002/diff/80001/ui/ozone/platform/x11/x...
ui/ozone/platform/x11/x11_surface_factory.cc:176: case
gl::kGLImplementationSwiftShaderGL:
On 2017/06/12 17:23:40, rjkroege wrote:
> it ought to be that the swiftshader oxygen build should work in soft vnc
without
> any special hackery. does it?

Yep.

[tonikitoo, 2017-06-12 19:39:00]

https://codereview.chromium.org/2926423002/diff/80001/ui/ozone/platform/x11/x...
File ui/ozone/platform/x11/x11_surface_factory.cc (right):

https://codereview.chromium.org/2926423002/diff/80001/ui/ozone/platform/x11/x...
ui/ozone/platform/x11/x11_surface_factory.cc:176: case
gl::kGLImplementationSwiftShaderGL:
On 2017/06/12 19:37:00, sadrul wrote:
> On 2017/06/12 17:23:40, rjkroege wrote:
> > it ought to be that the swiftshader oxygen build should work in soft vnc
> without
> > any special hackery. does it?
> 
> Yep.

Ok with me. Maybe add a comment in the code for the record?

[sadrul, 2017-06-12 21:32:13]

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

[commit-bot: I haz the power, 2017-06-12 21:32:49]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-06-12 21:40:05]

On 2017/06/12 19:39:00, tonikitoo wrote:
>
https://codereview.chromium.org/2926423002/diff/80001/ui/ozone/platform/x11/x...
> File ui/ozone/platform/x11/x11_surface_factory.cc (right):
> 
>
https://codereview.chromium.org/2926423002/diff/80001/ui/ozone/platform/x11/x...
> ui/ozone/platform/x11/x11_surface_factory.cc:176: case
> gl::kGLImplementationSwiftShaderGL:
> On 2017/06/12 19:37:00, sadrul wrote:
> > On 2017/06/12 17:23:40, rjkroege wrote:
> > > it ought to be that the swiftshader oxygen build should work in soft vnc
> > without
> > > any special hackery. does it?
> > 
> > Yep.
> 
> Ok with me. Maybe add a comment in the code for the record?

Added a comment.

[sadrul, 2017-06-12 21:40:23]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2017-06-12 21:40:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
rjkroege@chromium.org
Link to the patchset: https://codereview.chromium.org/2926423002/#ps100001
(title: ".")

[commit-bot: I haz the power, 2017-06-12 21:40:41]

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-06-12 23:23:39]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1497303623391110,
"parent_rev": "d73ae5a93f4bc96f55a8e5b1a9d4b2c037700475", "commit_rev":
"1e117683d1819528521a763e94162035d42f1826"}

[commit-bot: I haz the power, 2017-06-12 23:23:49]

Description was changed from

==========
ozone: Allow using swiftshader with --use-gl=swiftshader.

This hooks up the ozone code so it can deal with --use-gl=swiftshader
flag correctly.

BUG=704285
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
ozone: Allow using swiftshader with --use-gl=swiftshader.

This hooks up the ozone code so it can deal with --use-gl=swiftshader
flag correctly.

BUG=704285
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2926423002
Cr-Commit-Position: refs/heads/master@{#478817}
Committed:
https://chromium.googlesource.com/chromium/src/+/1e117683d1819528521a763e9416...
==========

[commit-bot: I haz the power, 2017-06-12 23:23:51]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/1e117683d1819528521a763e9416...