[lacros][hdr] Add RGBA_F16 support for ChromeOS and Lacros [chromium/src : main]

Femi Adegunloye (Gerrit)

unread,

Sep 8, 2023, 4:27:16 PM9/8/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Peter McNeeley.

View Change

2 comments:

Patchset:
- Patch Set #3:
  I understand that results in double composition, but at least the images are loading. […]
  Discussed offline with Peter
Patchset:
- Patch Set #4:
  Status update:
  Incorporated changes from Peter's CL.
  Images load fine, video plays fine, no distorted colors. Things appear to work, but
  I'm seeing errors in the logs though when I play HDR video, my guess is they originate from glTexImage2D -
  2023-09-08T18:39:59.968249Z ERROR chrome[3285:3285]: [gl_utils.cc(398)] [.RenderCompositor-0xb0000be9b00] GL_INVALID_FRAMEBUFFER_OPERATION: Framebuffer is incomplete: Attachment is not renderable.

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

Peter McNeeley (Gerrit)

unread,

Sep 10, 2023, 9:16:12 PM9/10/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, chromium...@chromium.org

Attention is currently required from: Femi Adegunloye.

View Change

2 comments:

Patchset:
- Patch Set #4:
  Can you also try now that i have updated angle

File ui/ozone/common/native_pixmap_egl_binding.cc:

Patch Set #4, Line 267:

::GetInternalFormat() {
  if (format_ == gfx::BufferFormat::RGBA_4444) {
    return GL_RGB_YCBCR_P010_CHROMIUM;
  }

  if (format_ == gfx::BufferFormat::RGBA_F16) {
    return GL_RGBA16F_EXT;
  }

I dont have this change. Perhaps this is what the issue is...

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

Femi Adegunloye (Gerrit)

unread,

Sep 12, 2023, 5:34:27 PM9/12/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Peter McNeeley.

View Change

3 comments:

Patchset:
- Patch Set #4:
  Status update: […]
  Other messages in the log:
  2023-09-08T20:30:03.972144Z ERROR chrome[18559:18559]: [shared_image_manager.cc(272)] SharedImageManager::ProduceOverlay: Trying to Produce a Overlay representation from a non-existent mailbox.
  2023-09-08T20:30:03.972268Z ERROR chrome[18559:18559]: [skia_output_device_buffer_queue.cc(368)] Invalid mailbox.

- Patch Set #4:
  Can you also try now that i have updated angle

- Getting the same results, even after updating.
  Status:
  Ignoring the scanout errors, it's really only the GL_INVALID_FRAMEBUFFER_OPERATION error that remains in the logs. It prints about 10 lines of error the first time an HDR video is played, then never again. I would even consider committing now and worrying about it later
  But there is another problem where the brightness flickers when you switch between or hover over tabs. With 'Tint Composited Content' enabled, you can see the window is being composited at the same time. Could be a coincidence, or related to the flashing.
  The flashing does not occur when delegated compositing is disabled.
File ui/ozone/common/native_pixmap_egl_binding.cc:

Patch Set #4, Line 267:

::GetInternalFormat() {
  if (format_ == gfx::BufferFormat::RGBA_4444) {
    return GL_RGB_YCBCR_P010_CHROMIUM;
  }

  if (format_ == gfx::BufferFormat::RGBA_F16) {
    return GL_RGBA16F_EXT;
  }

I dont have this change. Perhaps this is what the issue is...

- This line hasn't made any difference either way, I will probably remove it, but it's not what's causing the error.

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

Femi Adegunloye (Gerrit)

unread,

Sep 12, 2023, 5:36:45 PM9/12/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Peter McNeeley.

View Change

1 comment:

Patchset:
- Patch Set #4:
  Other messages in the log: […]
  the invalid mailbox might be about P010 and unrelated to RGBA_F16:
  2023-09-12T21:21:34.538616Z ERROR chrome[17953:17953]: [shared_image_factory.cc(915)] Could not find SharedImageBackingFactory with params: usage: Gles2|Raster|DisplayRead|Scanout, format: P010_LEGACY, share_between_threads: 0, gmb_type: shared_memory, debug_label: MediaGmbVideoFramePool_Cid:16_Pid:0_Cid:16_Pid:0

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

Femi Adegunloye (Gerrit)

unread,

Oct 2, 2023, 1:41:52 PM10/2/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org

Attention is currently required from: Peter McNeeley.

Femi Adegunloye uploaded patch set #5 to this change.

View Change

[lacros][hdr] Add RGBA_F16 support for ChromeOS and Lacros

RGBA_F16 buffers aren't currently supported. This CL enables this format
using DRM_ABGR16161616F.

TEST=build and deploy
BUG=b/297885530

Change-Id: I4caedf1147e4203d0f8bf51755aec811a609c6f6
---
M components/viz/service/display/direct_renderer.cc
M components/viz/service/display/overlay_candidate_factory.cc
M gpu/command_buffer/common/gpu_memory_buffer_support.cc
M gpu/command_buffer/service/shared_image/gl_ozone_image_representation.cc
M ui/gfx/linux/drm_util_linux.cc
M ui/ozone/common/native_pixmap_egl_binding.cc
M ui/ozone/platform/wayland/host/wayland_screen.cc
7 files changed, 25 insertions(+), 8 deletions(-)

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

Femi Adegunloye (Gerrit)

unread,

Oct 4, 2023, 6:56:28 PM10/4/23

to Zhenyao Mo, Mitsuru Oshima, Daniel Nicoara, Peter McNeeley, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org

Attention is currently required from: Daniel Nicoara, Mitsuru Oshima, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

Femi Adegunloye would like Zhenyao Mo, Mitsuru Oshima, Daniel Nicoara and Peter McNeeley to review this change.

View Change

[lacros][hdr] Add RGBA_F16 support for ChromeOS and Lacros

RGBA_F16 buffers aren't currently supported. This CL enables this format
using DRM_ABGR16161616F.

TEST=build and deploy
BUG=b/297885530

Change-Id: I4caedf1147e4203d0f8bf51755aec811a609c6f6
---
M components/viz/service/display/direct_renderer.cc
M components/viz/service/display/overlay_candidate_factory.cc
M gpu/command_buffer/common/gpu_memory_buffer_support.cc
M gpu/command_buffer/service/shared_image/gl_ozone_image_representation.cc
M ui/gfx/linux/drm_util_linux.cc
M ui/ozone/common/native_pixmap_egl_binding.cc
M ui/ozone/platform/wayland/host/wayland_screen.cc
7 files changed, 25 insertions(+), 8 deletions(-)

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

Femi Adegunloye (Gerrit)

unread,

Oct 4, 2023, 6:56:36 PM10/4/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Daniel Nicoara, Mitsuru Oshima, Peter McNeeley, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Mitsuru Oshima, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

View Change

1 comment:

File ui/ozone/common/native_pixmap_egl_binding.cc:

Patch Set #4, Line 267:

::GetInternalFormat() {
  if (format_ == gfx::BufferFormat::RGBA_4444) {
    return GL_RGB_YCBCR_P010_CHROMIUM;
  }

  if (format_ == gfx::BufferFormat::RGBA_F16) {
    return GL_RGBA16F_EXT;
  }

- This line hasn't made any difference either way, I will probably remove it, but it's not what's caus […]
  I'm going to keep this change even though it hasn't really had an effect because I do believe it's the correct format for glTexImage2D when requesting half float buffers.

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

Mitsuru Oshima (Gerrit)

unread,

Oct 4, 2023, 8:00:04 PM10/4/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Daniel Nicoara, Peter McNeeley, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

View Change

2 comments:

File ui/gfx/linux/drm_util_linux.cc:
- Patch Set #5, Line 101: return true;
  nit: remove this?
File ui/ozone/platform/wayland/host/wayland_screen.cc:
- Patch Set #5, Line 241: bool enable_hdr = true;
  do we need this variable?

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

Peter McNeeley (Gerrit)

unread,

Oct 5, 2023, 10:02:06 AM10/5/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Daniel Nicoara, Mitsuru Oshima, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Peter McNeeley, Zhenyao Mo.

View Change

2 comments:

File gpu/command_buffer/service/shared_image/gl_ozone_image_representation.cc:
- Patch Set #4, Line 51: GL_RGBA
  I wonder if this is correct
File ui/ozone/platform/wayland/host/wayland_screen.cc:
- Patch Set #5, Line 236: color_management_output
  We had to add at least another colorspace to the wayland protocol. You will need to conditionally enable HDR on the version that you provided this color space otherwise old versions of ash will be running HDR when it is not fully supported.

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

Peter McNeeley (Gerrit)

unread,

Oct 5, 2023, 10:14:40 AM10/5/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Daniel Nicoara, Mitsuru Oshima, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Mitsuru Oshima, Peter McNeeley, Zhenyao Mo.

View Change

2 comments:

File components/viz/service/display/direct_renderer.cc:

Patch Set #5, Line 1117:

#if BUILDFLAG(IS_CHROMEOS_LACROS)
  // TODO(crbug.com/1317015): add support RGBA_F16 in LaCrOS.
  auto format = color_space.IsHDR()
                    ? SinglePlaneFormat::kRGBA_F16
                    : PlatformColor::BestSupportedTextureFormat(caps);
#else
  auto format = color_space.IsHDR()
                    ? SinglePlaneFormat::kRGBA_F16
                    : PlatformColor::BestSupportedTextureFormat(caps);
#endif

Are these two lines now identical? Should we merge them?

File ui/gfx/linux/drm_util_linux.cc:

- Patch Set #5, Line 101: return true;
  nit: remove this?

- I think you mean move not remove?

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

Peter McNeeley (Gerrit)

unread,

Oct 5, 2023, 5:00:12 PM10/5/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Daniel Nicoara, Mitsuru Oshima, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Mitsuru Oshima, Peter McNeeley, Zhenyao Mo.

View Change

1 comment:

File gpu/command_buffer/service/shared_image/gl_ozone_image_representation.cc:

- Patch Set #4, Line 51: GL_RGBA
  I wonder if this is correct

- To follow up with this comment. I am wondering if we should be specifying |internal_format| instead here. Do you know

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

Femi Adegunloye (Gerrit)

unread,

Oct 5, 2023, 6:22:19 PM10/5/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Mitsuru Oshima, Peter McNeeley, Zhenyao Mo.

Femi Adegunloye uploaded patch set #6 to this change.

View Change

[lacros][hdr] Add RGBA_F16 support for ChromeOS and Lacros

RGBA_F16 buffers aren't currently supported. This CL enables this format
using DRM_ABGR16161616F.

TEST=build and deploy
BUG=b/297885530

Change-Id: I4caedf1147e4203d0f8bf51755aec811a609c6f6
---
M components/viz/service/display/direct_renderer.cc
M components/viz/service/display/overlay_candidate_factory.cc
M gpu/command_buffer/common/gpu_memory_buffer_support.cc
M gpu/command_buffer/service/shared_image/gl_ozone_image_representation.cc
M ui/gfx/linux/drm_util_linux.cc
M ui/ozone/common/native_pixmap_egl_binding.cc
M ui/ozone/platform/wayland/host/wayland_screen.cc

M ui/ozone/platform/wayland/host/wayland_zcr_color_manager.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager.h
9 files changed, 30 insertions(+), 19 deletions(-)

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

Femi Adegunloye (Gerrit)

unread,

Oct 5, 2023, 6:23:11 PM10/5/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Daniel Nicoara, Mitsuru Oshima, Peter McNeeley, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Mitsuru Oshima, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

View Change

5 comments:

File components/viz/service/display/direct_renderer.cc:

Patch Set #5, Line 1117:

#if BUILDFLAG(IS_CHROMEOS_LACROS)
  // TODO(crbug.com/1317015): add support RGBA_F16 in LaCrOS.
  auto format = color_space.IsHDR()
                    ? SinglePlaneFormat::kRGBA_F16
                    : PlatformColor::BestSupportedTextureFormat(caps);
#else
  auto format = color_space.IsHDR()
                    ? SinglePlaneFormat::kRGBA_F16
                    : PlatformColor::BestSupportedTextureFormat(caps);
#endif

Are these two lines now identical? Should we merge them?

- Done
File gpu/command_buffer/service/shared_image/gl_ozone_image_representation.cc:

- Patch Set #4, Line 51: GL_RGBA
  I wonder if this is correct

- I believe this should represent the data_format arg that would be passed into glTexImage2D() so should be GL_RGBA because RGBA16F includes three color channels and an alpha.
  ---
  But let me double check. There are a couple of data/internal format functions and it's possible I'm confused or the code is wrong.
File ui/gfx/linux/drm_util_linux.cc:

- Patch Set #5, Line 101: return true;

- I think you mean move not remove?
  Moved.
File ui/ozone/platform/wayland/host/wayland_screen.cc:

- Patch Set #5, Line 236: color_management_output

- We had to add at least another colorspace to the wayland protocol. […]
  Done

- Patch Set #5, Line 241: bool enable_hdr = true;
  do we need this variable?

- No, but I've replaced with a bool that checks that the protocol version is greater than 6.

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

Peter McNeeley (Gerrit)

unread,

Oct 6, 2023, 10:31:21 AM10/6/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Daniel Nicoara, Mitsuru Oshima, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Mitsuru Oshima, Peter McNeeley, Zhenyao Mo.

View Change

4 comments:

File gpu/command_buffer/common/gpu_memory_buffer_support.cc:
- Patch Set #6, Line 231: RGBA_F16
  we might want to limit this to only chromeos or even chromeos lacros to avoid knock-ons. In fact I think this is quite important that we do.
File ui/ozone/common/native_pixmap_egl_binding.cc:
- Patch Set #6, Line 272:

 if (format_ == gfx::BufferFormat::RGBA_F16) {
    return GL_RGBA16F_EXT;
  }

- is there any reason why not to do it in the function below
  aka "native_pixmap_gl_binding.cc" file
  'BufferFormatToGLInternalFormatDefaultMapping'
  https://source.chromium.org/chromium/chromium/src/+/main:ui/ozone/public/native_pixmap_gl_binding.cc;l=16;bpv=1;bpt=1?q=BufferFormatToGLInternalFormatDefaultMapping&ss=chromium%2Fchromium%2Fsrc
File ui/ozone/platform/wayland/host/wayland_screen.cc:
- Patch Set #6, Line 103: RGBA_F16
  is this an issue.
- Patch Set #6, Line 239: 6
  Can we specify the macro number that is generated not a magic number 6.
  Kramer has done some of this as a refactor here
  https://chromium-review.googlesource.com/c/chromium/src/+/4897182

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

Peter McNeeley (Gerrit)

unread,

Oct 6, 2023, 10:33:25 AM10/6/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Daniel Nicoara, Mitsuru Oshima, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Mitsuru Oshima, Peter McNeeley, Zhenyao Mo.

View Change

1 comment:

File ui/ozone/platform/wayland/host/wayland_screen.cc:
- Patch Set #6, Line 239: ge_6
  same with this 6. 6 is a magic number and does not have any meaning

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

Peter McNeeley (Gerrit)

unread,

Oct 6, 2023, 10:40:39 AM10/6/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Daniel Nicoara, Mitsuru Oshima, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Mitsuru Oshima, Peter McNeeley, Zhenyao Mo.

View Change

1 comment:

File ui/ozone/platform/wayland/host/wayland_screen.cc:
- Patch Set #6, Line 103: RGBA_F16
  is this an issue.
  Sorry this was draft.

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

Mitsuru Oshima (Gerrit)

unread,

Oct 6, 2023, 11:30:12 AM10/6/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Daniel Nicoara, Peter McNeeley, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

View Change

2 comments:

File ui/gfx/linux/drm_util_linux.cc:

- Patch Set #5, Line 101: return true;

- Moved.
  I meant remove, since both return true.
File ui/ozone/platform/wayland/host/wayland_screen.cc:
- Patch Set #6, Line 239: 6
  Can we specify the macro number that is generated not a magic number 6. […]
  +1

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

Femi Adegunloye (Gerrit)

unread,

Oct 6, 2023, 2:13:18 PM10/6/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

Femi Adegunloye uploaded patch set #7 to this change.

View Change

[lacros][hdr] Add RGBA_F16 support for ChromeOS and Lacros

RGBA_F16 buffers aren't currently supported. This CL enables this format
using DRM_ABGR16161616F.

TEST=build and deploy
BUG=b/297885530

Change-Id: I4caedf1147e4203d0f8bf51755aec811a609c6f6
---
M components/viz/service/display/direct_renderer.cc
M components/viz/service/display/overlay_candidate_factory.cc
M gpu/command_buffer/common/gpu_memory_buffer_support.cc
M gpu/command_buffer/service/shared_image/gl_ozone_image_representation.cc
M ui/gfx/linux/drm_util_linux.cc
M ui/ozone/common/native_pixmap_egl_binding.cc
M ui/ozone/platform/wayland/host/wayland_screen.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager.h

M ui/ozone/public/native_pixmap_gl_binding.cc
10 files changed, 33 insertions(+), 20 deletions(-)

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

Femi Adegunloye (Gerrit)

unread,

Oct 6, 2023, 2:15:46 PM10/6/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Daniel Nicoara, Mitsuru Oshima, Peter McNeeley, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Mitsuru Oshima, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

Patch set 7:Commit-Queue +1

View Change

5 comments:

File gpu/command_buffer/common/gpu_memory_buffer_support.cc:

- Patch Set #6, Line 231: RGBA_F16

- we might want to limit this to only chromeos or even chromeos lacros to avoid knock-ons. […]
  I set to limit it to ChromeOS
File gpu/command_buffer/service/shared_image/gl_ozone_image_representation.cc:

- Patch Set #4, Line 51: GL_RGBA

- I believe this should represent the data_format arg that would be passed into glTexImage2D() so shou […]
  Pretty sure this is correct.
  InternalFormat (GL_RGBA16F_EXT) comes from -
  NativePixmapEGLBinding::GetInternalFormat() and
  NativePixmapGLBinding::BufferFormatToGLInternalFormatDefaultMapping()
  DataFormat (GL_RGBA) comes from -
  GetDataFormatFromInternalFormat()
  DataType (GL_HALF_FLOAT_OES) comes from -
  NativePixmapEGLBinding:: BufferFormatToGLDataType()
File ui/ozone/common/native_pixmap_egl_binding.cc:

Patch Set #6, Line 272:

 if (format_ == gfx::BufferFormat::RGBA_F16) {
    return GL_RGBA16F_EXT;
  }

- is there any reason why not to do it in the function below […]
  Hm no, there shouldn't be. I checked an all behavior seems the same. I updated BufferFormatToGLInternalFormatDefaultMapping() to return GL_RGBA16F_EXT format for RGBA_F16
File ui/ozone/platform/wayland/host/wayland_screen.cc:
- Patch Set #6, Line 239: ge_6
  same with this 6. […]
  Done

- Patch Set #6, Line 239: 6

- +1
  Replaced with SRGB_HDR Since Version.

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

Mitsuru Oshima (Gerrit)

unread,

Oct 6, 2023, 2:47:01 PM10/6/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Chromium LUCI CQ, Daniel Nicoara, Peter McNeeley, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

Patch set 7:Code-Review +1

View Change

3 comments:

Patchset:
- Patch Set #7:
  lgtm my bits % nits
File gpu/command_buffer/common/gpu_memory_buffer_support.cc:
- Patch Set #7, Line 234: {
  nit: ) {
File ui/ozone/platform/wayland/host/wayland_screen.cc:
- Patch Set #7, Line 241: ZCR_COLOR_MANAGER_V1_EOTF_NAMES_SRGB_HDR_SINCE_VERSION;
  nit: _supported is more common, so
  srgb_hdr_supported
  would be better

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

Femi Adegunloye (Gerrit)

unread,

Oct 6, 2023, 3:24:58 PM10/6/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

Femi Adegunloye uploaded patch set #8 to this change.

View Change

[lacros][hdr] Add RGBA_F16 support for ChromeOS and Lacros

RGBA_F16 buffers aren't currently supported. This CL enables this format
using DRM_ABGR16161616F.

TEST=build and deploy
BUG=b/297885530

Change-Id: I4caedf1147e4203d0f8bf51755aec811a609c6f6
---
M components/viz/service/display/direct_renderer.cc
M components/viz/service/display/overlay_candidate_factory.cc
M gpu/command_buffer/common/gpu_memory_buffer_support.cc
M gpu/command_buffer/service/shared_image/gl_ozone_image_representation.cc
M ui/gfx/linux/drm_util_linux.cc
M ui/ozone/common/native_pixmap_egl_binding.cc
M ui/ozone/platform/wayland/host/wayland_screen.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager.h
M ui/ozone/public/native_pixmap_gl_binding.cc
10 files changed, 33 insertions(+), 20 deletions(-)

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

Femi Adegunloye (Gerrit)

unread,

Oct 6, 2023, 3:29:29 PM10/6/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Mitsuru Oshima, Chromium LUCI CQ, Daniel Nicoara, Peter McNeeley, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

Patch set 8:Commit-Queue +1

View Change

2 comments:

File gpu/command_buffer/common/gpu_memory_buffer_support.cc:

- Patch Set #7, Line 234: {
  nit: ) {

- Done
File ui/ozone/platform/wayland/host/wayland_screen.cc:

- Patch Set #7, Line 241: ZCR_COLOR_MANAGER_V1_EOTF_NAMES_SRGB_HDR_SINCE_VERSION;

- nit: _supported is more common, so […]
  Done

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

Femi Adegunloye (Gerrit)

unread,

Oct 23, 2023, 2:43:20 PM10/23/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Code Review Nudger, Mitsuru Oshima, Chromium LUCI CQ, Daniel Nicoara, Peter McNeeley, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

View Change

2 comments:

Patchset:
- Patch Set #4:
  Getting the same results, even after updating. […]
  Peter looked into this and it seems that Ash already exhibits this behavior. Since the goal for lacros is parity, this is fine.
  The flicker occurs because the transition between SDR and HDR is instant which can be jarring. A smooth ramp in brightness between HDR and SDR is eventually planned for HDR development.
- Patch Set #4:
  the invalid mailbox might be about P010 and unrelated to RGBA_F16: […]
  Unrelated.

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

Peter McNeeley (Gerrit)

unread,

Oct 23, 2023, 2:45:29 PM10/23/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Code Review Nudger, Mitsuru Oshima, Chromium LUCI CQ, Daniel Nicoara, Zhenyao Mo, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Peter McNeeley, Zhenyao Mo.

Patch set 8:Code-Review +1

View Change

1 comment:

Patchset:
- Patch Set #8:
  viz lgtm

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

Zhenyao Mo (Gerrit)

unread,

Oct 23, 2023, 2:48:56 PM10/23/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Peter McNeeley, Code Review Nudger, Mitsuru Oshima, Chromium LUCI CQ, Daniel Nicoara, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Peter McNeeley.

Patch set 8:Code-Review +1

View Change

2 comments:

Patchset:
- Patch Set #8:
  lgtm with one nit
File gpu/command_buffer/common/gpu_memory_buffer_support.cc:
- Patch Set #8, Line 232: || format == gfx::BufferFormat::RGBA_F16) {
  nit: you could move this to the middle of the || list, then it won't be awkward looking like it is now.

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

Femi Adegunloye (Gerrit)

unread,

Oct 24, 2023, 5:04:32 PM10/24/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Mitsuru Oshima, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

Femi Adegunloye uploaded patch set #9 to this change.

View Change

The following approvals got outdated and were removed: Code-Review+1 by Mitsuru Oshima, Code-Review+1 by Peter McNeeley, Code-Review+1 by Zhenyao Mo

The change is no longer submittable: Code-Owners and Code-Review are unsatisfied now.

[lacros][hdr] Add RGBA_F16 support for ChromeOS and Lacros

RGBA_F16 buffers aren't currently supported. This CL enables this format
using DRM_ABGR16161616F.

TEST=build and deploy
BUG=b/297885530

Change-Id: I4caedf1147e4203d0f8bf51755aec811a609c6f6
---
M components/viz/service/display/direct_renderer.cc
M components/viz/service/display/overlay_candidate_factory.cc
M gpu/command_buffer/common/gpu_memory_buffer_support.cc

M gpu/command_buffer/service/shared_image/ozone_image_gl_textures_holder.cc


M ui/gfx/linux/drm_util_linux.cc
M ui/ozone/common/native_pixmap_egl_binding.cc
M ui/ozone/platform/wayland/host/wayland_screen.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager.h
M ui/ozone/public/native_pixmap_gl_binding.cc

10 files changed, 30 insertions(+), 19 deletions(-)

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

Femi Adegunloye (Gerrit)

unread,

Oct 24, 2023, 5:12:18 PM10/24/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Mitsuru Oshima, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

Femi Adegunloye uploaded patch set #10 to this change.

View Change

[lacros][hdr] Add RGBA_F16 support for ChromeOS and Lacros



RGBA_F16 buffers aren't currently supported. This CL enables this format
using DRM_ABGR16161616F.

TEST=build and deploy
BUG=b/297885530

Change-Id: I4caedf1147e4203d0f8bf51755aec811a609c6f6
---
M components/viz/service/display/direct_renderer.cc
M components/viz/service/display/overlay_candidate_factory.cc
M gpu/command_buffer/common/gpu_memory_buffer_support.cc
M gpu/command_buffer/service/shared_image/ozone_image_gl_textures_holder.cc
M ui/gfx/linux/drm_util_linux.cc
M ui/ozone/common/native_pixmap_egl_binding.cc
M ui/ozone/platform/wayland/host/wayland_screen.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager.h
M ui/ozone/public/native_pixmap_gl_binding.cc

10 files changed, 29 insertions(+), 19 deletions(-)

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

Femi Adegunloye (Gerrit)

unread,

Oct 24, 2023, 5:12:58 PM10/24/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Zhenyao Mo, Peter McNeeley, Code Review Nudger, Mitsuru Oshima, Chromium LUCI CQ, Daniel Nicoara, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Mitsuru Oshima, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

View Change

2 comments:

Patchset:
- Patch Set #9:
  Fixed merge conflict. Could everyone please vote again, thanks
File gpu/command_buffer/common/gpu_memory_buffer_support.cc:

- Patch Set #8, Line 232: || format == gfx::BufferFormat::RGBA_F16) {

- nit: you could move this to the middle of the || list, then it won't be awkward looking like it is n […]
  Done

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

Femi Adegunloye (Gerrit)

unread,

Oct 24, 2023, 5:13:04 PM10/24/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Zhenyao Mo, Peter McNeeley, Code Review Nudger, Mitsuru Oshima, Chromium LUCI CQ, Daniel Nicoara, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Mitsuru Oshima, Peter McNeeley, Peter McNeeley, Zhenyao Mo.

Patch set 10:Commit-Queue +1

View Change

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

Peter McNeeley (Gerrit)

unread,

Oct 24, 2023, 5:19:56 PM10/24/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Zhenyao Mo, Code Review Nudger, Mitsuru Oshima, Chromium LUCI CQ, Daniel Nicoara, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Mitsuru Oshima, Peter McNeeley, Zhenyao Mo.

Patch set 10:Code-Review +1

View Change

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

Mitsuru Oshima (Gerrit)

unread,

Oct 25, 2023, 2:08:17 AM10/25/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Peter McNeeley, Zhenyao Mo, Code Review Nudger, Chromium LUCI CQ, Daniel Nicoara, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Peter McNeeley, Zhenyao Mo.

Patch set 10:Code-Review +1

View Change

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

Zhenyao Mo (Gerrit)

unread,

Oct 25, 2023, 1:33:13 PM10/25/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Mitsuru Oshima, Peter McNeeley, Code Review Nudger, Chromium LUCI CQ, Daniel Nicoara, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Femi Adegunloye, Peter McNeeley.

Patch set 10:Code-Review +1

View Change

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

Femi Adegunloye (Gerrit)

unread,

Oct 25, 2023, 1:39:22 PM10/25/23

to penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Zhenyao Mo, Mitsuru Oshima, Peter McNeeley, Code Review Nudger, Chromium LUCI CQ, Daniel Nicoara, Peter McNeeley, chromium...@chromium.org

Attention is currently required from: Daniel Nicoara, Peter McNeeley.

Patch set 10:Commit-Queue +2

View Change

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

Chromium LUCI CQ (Gerrit)

unread,

Oct 25, 2023, 1:44:05 PM10/25/23

to Femi Adegunloye, penghu...@chromium.org, cc-...@chromium.org, ozone-...@chromium.org, Zhenyao Mo, Mitsuru Oshima, Peter McNeeley, Code Review Nudger, Daniel Nicoara, Peter McNeeley, chromium...@chromium.org

Chromium LUCI CQ submitted this change.

View Change

Approvals:
  Zhenyao Mo: Looks good to me
  Mitsuru Oshima: Looks good to me
  Femi Adegunloye: Commit
  Peter McNeeley: Looks good to me

[lacros][hdr] Add RGBA_F16 support for ChromeOS and Lacros

RGBA_F16 buffers aren't currently supported. This CL enables this format
using DRM_ABGR16161616F.

TEST=build and deploy
BUG=b/297885530

Change-Id: I4caedf1147e4203d0f8bf51755aec811a609c6f6

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4827374
Reviewed-by: Peter McNeeley <peterm...@chromium.org>
Reviewed-by: Zhenyao Mo <z...@chromium.org>
Commit-Queue: Femi Adegunloye <mrf...@google.com>
Reviewed-by: Mitsuru Oshima <osh...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1214951}


---
M components/viz/service/display/direct_renderer.cc
M components/viz/service/display/overlay_candidate_factory.cc
M gpu/command_buffer/common/gpu_memory_buffer_support.cc
M gpu/command_buffer/service/shared_image/ozone_image_gl_textures_holder.cc
M ui/gfx/linux/drm_util_linux.cc
M ui/ozone/common/native_pixmap_egl_binding.cc
M ui/ozone/platform/wayland/host/wayland_screen.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager.cc
M ui/ozone/platform/wayland/host/wayland_zcr_color_manager.h
M ui/ozone/public/native_pixmap_gl_binding.cc
10 files changed, 29 insertions(+), 19 deletions(-)

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

Reply all

Reply to author

Forward