Specify ANGLE implementation in list of allowed GL implementations. [chromium/src : main]

Jonah Ryan-Davis (Gerrit)

unread,

Jul 13, 2021, 5:01:08 PM7/13/21

to ccameron, Geoff Lang, fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org

Attention is currently required from: ccameron, Geoff Lang.

Jonah Ryan-Davis would like ccameron and Geoff Lang to review this change.

View Change

Specify ANGLE implementation in list of allowed GL implementations.

This CL changes the list of allowed GLImplementations to a list of
GLImplementationParts, which specifies the ANGLE implementation as
well. Using ANGLEImplementation::kDefault will allow any ANGLE
implementation. This is a prerequisite for running headless Ozone
with SwANGLE without allowing ANGLE to run on the GPU.

Bug: chromium:1065472
Bug: chromium:1227864
Change-Id: I2de80f33756d61f92856e71098468742e217f521
---
M gpu/command_buffer/tests/gl_test_utils.cc
M ui/gl/gl_implementation.cc
M ui/gl/gl_implementation.h
M ui/gl/init/gl_factory.cc
M ui/gl/init/gl_factory.h
M ui/gl/init/gl_factory_android.cc
M ui/gl/init/gl_factory_linux_x11.cc
M ui/gl/init/gl_factory_linux_x11.h
M ui/gl/init/gl_factory_mac.cc
M ui/gl/init/gl_factory_ozone.cc
M ui/gl/init/gl_factory_win.cc
M ui/gl/test/gl_image_test_support.cc
M ui/gl/test/gl_surface_test_support.cc
M ui/ozone/platform/cast/surface_factory_cast.cc
M ui/ozone/platform/cast/surface_factory_cast.h
M ui/ozone/platform/drm/gpu/gbm_surface_factory.cc
M ui/ozone/platform/drm/gpu/gbm_surface_factory.h
M ui/ozone/platform/headless/headless_surface_factory.cc
M ui/ozone/platform/headless/headless_surface_factory.h
M ui/ozone/platform/scenic/scenic_surface_factory.cc
M ui/ozone/platform/scenic/scenic_surface_factory.h
M ui/ozone/platform/wayland/gpu/wayland_surface_factory.cc
M ui/ozone/platform/wayland/gpu/wayland_surface_factory.h
M ui/ozone/platform/windows/windows_surface_factory.cc
M ui/ozone/platform/windows/windows_surface_factory.h
M ui/ozone/platform/x11/x11_surface_factory.cc
M ui/ozone/platform/x11/x11_surface_factory.h
M ui/ozone/public/surface_factory_ozone.cc
M ui/ozone/public/surface_factory_ozone.h
29 files changed, 122 insertions(+), 82 deletions(-)

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Jonah Ryan-Davis (Gerrit)

unread,

Jul 13, 2021, 5:01:15 PM7/13/21

to fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org, ccameron, Geoff Lang, Chromium LUCI CQ, Tricium, Alexis Hétu, chromium...@chromium.org, David Worsham, Kalyan Kondapally, Robert Kroeger

Attention is currently required from: ccameron, Geoff Lang.

Patch set 4:Commit-Queue +1

View Change

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Jonah Ryan-Davis (Gerrit)

unread,

Jul 13, 2021, 5:01:56 PM7/13/21

to fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org, ccameron, Geoff Lang, Chromium LUCI CQ, Tricium, Alexis Hétu, chromium...@chromium.org, David Worsham, Kalyan Kondapally, Robert Kroeger

Attention is currently required from: ccameron, Geoff Lang.

View Change

1 comment:

Patchset:
- Patch Set #4:
  PTAL

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Jonah Ryan-Davis (Gerrit)

unread,

Jul 14, 2021, 2:16:41 PM7/14/21

to fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org, ccameron, Geoff Lang, Chromium LUCI CQ, Tricium, Alexis Hétu, chromium...@chromium.org, David Worsham, Kalyan Kondapally, Robert Kroeger

Attention is currently required from: ccameron, Geoff Lang.

View Change

2 comments:

File ui/ozone/platform/wayland/gpu/wayland_surface_factory.cc:
- Patch Set #3, Line 149: impls.push_back(gl::GLImplementationParts(gl::kGLImplementationEGLGLES2));
  use emplace_back instead of push_back (https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html)
  Please fix.
- Patch Set #3, Line 150: impls.push_back(
  use emplace_back instead of push_back (https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html)
  Please fix.

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Jonah Ryan-Davis (Gerrit)

unread,

Jul 14, 2021, 2:18:32 PM7/14/21

to fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org, ccameron, Geoff Lang, Chromium LUCI CQ, Tricium, Alexis Hétu, chromium...@chromium.org, David Worsham, Kalyan Kondapally, Robert Kroeger

Attention is currently required from: ccameron, Geoff Lang.

View Change

2 comments:

File ui/ozone/platform/headless/headless_surface_factory.cc:
- Patch Set #5, Line 231: gl::GLImplementationParts(gl::ANGLEImplementation::kSwiftShader)};
  These changes were meant for a follow-up CL, will remove
- Patch Set #5, Line 240:
```
 case gl::kGLImplementationEGLANGLE:
      return (implementation.angle == gl::ANGLEImplementation::kSwiftShader)
                 ? swiftshader_implementation_.get()
                 : nullptr;
```
  These changes were meant for a follow-up CL, will remove

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Jonah Ryan-Davis (Gerrit)

unread,

Jul 14, 2021, 4:58:37 PM7/14/21

to fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org, ccameron, Geoff Lang, Chromium LUCI CQ, Tricium, Alexis Hétu, chromium...@chromium.org, David Worsham, Kalyan Kondapally, Robert Kroeger

Attention is currently required from: ccameron, Geoff Lang.

View Change

2 comments:

File ui/ozone/platform/headless/headless_surface_factory.cc:

- Patch Set #5, Line 231: gl::GLImplementationParts(gl::ANGLEImplementation::kSwiftShader)};
  These changes were meant for a follow-up CL, will remove

- Done

Patch Set #5, Line 240:

 case gl::kGLImplementationEGLANGLE:
      return (implementation.angle == gl::ANGLEImplementation::kSwiftShader)
                 ? swiftshader_implementation_.get()
                 : nullptr;

These changes were meant for a follow-up CL, will remove

- Done

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Geoff Lang (Gerrit)

unread,

Jul 16, 2021, 3:50:15 PM7/16/21

to Jonah Ryan-Davis, fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org, ccameron, Chromium LUCI CQ, Tricium, Alexis Hétu, chromium...@chromium.org, David Worsham, Kalyan Kondapally, Robert Kroeger

Attention is currently required from: ccameron, Jonah Ryan-Davis.

Patch set 7:Code-Review +1

View Change

1 comment:

Patchset:
- Patch Set #7:
  LGTM

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Michael Spang (Gerrit)

unread,

Jul 16, 2021, 5:12:56 PM7/16/21

to Jonah Ryan-Davis, fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org, Geoff Lang, ccameron, Chromium LUCI CQ, Tricium, Alexis Hétu, chromium...@chromium.org, David Worsham, Kalyan Kondapally, Robert Kroeger

Attention is currently required from: ccameron, Jonah Ryan-Davis.

Patch set 7:Code-Review +1

View Change

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Jonah Ryan-Davis (Gerrit)

unread,

Jul 16, 2021, 5:16:54 PM7/16/21

to Kenneth Russell, fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org, Michael Spang, Geoff Lang, ccameron

Attention is currently required from: ccameron, Kenneth Russell.

Jonah Ryan-Davis would like Kenneth Russell to review this change.

View Change

29 files changed, 143 insertions(+), 82 deletions(-)

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Kenneth Russell (Gerrit)

unread,

Jul 16, 2021, 5:20:58 PM7/16/21

to Jonah Ryan-Davis, fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org, Michael Spang, Geoff Lang, ccameron, Chromium LUCI CQ, Tricium, Alexis Hétu, chromium...@chromium.org, David Worsham, Kalyan Kondapally, Robert Kroeger

Attention is currently required from: ccameron, Jonah Ryan-Davis.

Patch set 7:Code-Review +1

View Change

2 comments:

Patchset:
- Patch Set #7:
  lgtm once this passes tests, and with a comment added.
File ui/gl/gl_implementation.cc:
- Patch Set #7, Line 62: if (gl == kGLImplementationEGLANGLE &&
  Could you please add a comment here describing exactly what this is testing for and why?

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Jonah Ryan-Davis (Gerrit)

unread,

Jul 16, 2021, 5:39:12 PM7/16/21

to fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org, Kenneth Russell, Michael Spang, Geoff Lang, ccameron, Chromium LUCI CQ, Tricium, Alexis Hétu, chromium...@chromium.org, David Worsham, Kalyan Kondapally, Robert Kroeger

Attention is currently required from: ccameron.

Jonah Ryan-Davis removed a vote from this change.

View Change

Removed Code-Review+1 by Jonah Ryan-Davis <jon...@google.com>

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Jonah Ryan-Davis (Gerrit)

unread,

Jul 16, 2021, 5:39:17 PM7/16/21

to fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org, Kenneth Russell, Michael Spang, Geoff Lang, ccameron, Chromium LUCI CQ, Tricium, Alexis Hétu, chromium...@chromium.org, David Worsham, Kalyan Kondapally, Robert Kroeger

Attention is currently required from: ccameron.

Patch set 8:Code-Review +1

View Change

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Jonah Ryan-Davis (Gerrit)

unread,

Jul 16, 2021, 5:39:26 PM7/16/21

to fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org, Kenneth Russell, Michael Spang, Geoff Lang, ccameron, Chromium LUCI CQ, Tricium, Alexis Hétu, chromium...@chromium.org, David Worsham, Kalyan Kondapally, Robert Kroeger

Attention is currently required from: ccameron.

View Change

1 comment:

File ui/gl/gl_implementation.cc:

- Patch Set #7, Line 62: if (gl == kGLImplementationEGLANGLE &&
  Could you please add a comment here describing exactly what this is testing for and why?

- Done

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Jonah Ryan-Davis (Gerrit)

unread,

Jul 17, 2021, 2:14:22 PM7/17/21

to fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org, Kenneth Russell, Michael Spang, Geoff Lang, ccameron, Chromium LUCI CQ, Tricium, Alexis Hétu, chromium...@chromium.org, David Worsham, Kalyan Kondapally, Robert Kroeger

Attention is currently required from: ccameron.

Patch set 8:Commit-Queue +2

View Change

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Chromium LUCI CQ (Gerrit)

unread,

Jul 17, 2021, 2:17:26 PM7/17/21

to Jonah Ryan-Davis, fuchsia...@chromium.org, headless...@chromium.org, mac-r...@chromium.org, ozone-...@chromium.org, spang...@chromium.org, Kenneth Russell, Michael Spang, Geoff Lang, ccameron, Tricium, Alexis Hétu, chromium...@chromium.org, David Worsham, Kalyan Kondapally, Robert Kroeger

Chromium LUCI CQ submitted this change.

View Change

Approvals:
  Michael Spang: Looks good to me
  Geoff Lang: Looks good to me
  Kenneth Russell: Looks good to me
  Jonah Ryan-Davis: Commit

Specify ANGLE implementation in list of allowed GL implementations.

This CL changes the list of allowed GLImplementations to a list of
GLImplementationParts, which specifies the ANGLE implementation as
well. Using ANGLEImplementation::kDefault will allow any ANGLE
implementation. This is a prerequisite for running headless Ozone
with SwANGLE without allowing ANGLE to run on the GPU.

Bug: chromium:1065472
Bug: chromium:1227864
Change-Id: I2de80f33756d61f92856e71098468742e217f521

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3024410
Reviewed-by: Geoff Lang <geof...@chromium.org>
Reviewed-by: Michael Spang <sp...@chromium.org>
Reviewed-by: Kenneth Russell <k...@chromium.org>
Commit-Queue: Jonah Ryan-Davis <jon...@google.com>
Cr-Commit-Position: refs/heads/master@{#902831}

29 files changed, 146 insertions(+), 82 deletions(-)

7 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

The name of the file: ui/gl/gl_implementation.cc
Insertions: 3, Deletions: 0.

```
@@ +60:63 @@
+    // Given a vector of GLImplementationParts, this function checks if "this"
+    // GLImplementation is found in the list, with a special case where if the
+    // list contains ANGLE/kDefault, "this" may be any ANGLE implementation.

```

To view, visit change 3024410. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward