New issue
Advanced search Search tips

Issue 863011 link

Starred by 2 users
Status: Started
Owner:
alexilin@chromium.org
Cc:
piman@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

gfx::GpuMemoryBufferHandle refactoring

Project Member Reported by alexilin@chromium.org, Jul 12 
End goal:
Make GMB handle owning the underlying handle with move semantics.

Change plan:
1. Make GMBHandle owning the underlying handle on all platforms (SHM/NativePixmap/DXGI).
2. Convert GMBHandle into a class with private members to prevent inconsistencies in the internal state.
3. Modify GMBHandle's move methods to reset the type/handle after the move operation.
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 20 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0443a8fb9e6e862ee70aea4644eeea5297334416

commit 0443a8fb9e6e862ee70aea4644eeea5297334416
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Fri Jul 20 20:14:50 2018

gfx: Improve handle management in gfx::GpuMemoryBuffer

The handle returned from gfx::GpuMemoryBuffer::GetHandle() is either passed to
CloneHandleForIPC immediately or used to obtain some handle metadata (type,
GUID).

This CL replaces GpuMemoryBuffer::GetHandle() with the new method
GpuMemoryBuffer::CloneHandle() that is equivalent of calling
CloneHandleForIPC(buffer.GetHandle()). This will allow to use scoped objects for
all types of underlying platform handles, since no two GpuMemoryBufferHandles
will refer to the same system resource. New methods are also added to access
type/GUID of buffer.

As a demonstration, this CL changes the type of the
GpuMemoryBufferHandle::android_hardware_buffer from AHardwareBuffer* to
base::android::ScopedHardwareBufferHandle.

Bug:  854594 , 863011
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_layout_tests_slimming_paint_v2;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I0306f4295cf28113cf467dba3c2a2b1717ca2e67
Reviewed-on: https://chromium-review.googlesource.com/1126394
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Chrome Cunningham (In Paris) <chcunningham@chromium.org>
Reviewed-by: Ricky Liang <jcliang@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Klaus Weidner <klausw@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576964}
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/cc/layers/heads_up_display_layer_impl.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/cc/raster/bitmap_raster_buffer_provider.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/cc/raster/gpu_raster_buffer_provider.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/cc/raster/one_copy_raster_buffer_provider.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/cc/raster/staging_buffer_pool.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/cc/raster/zero_copy_raster_buffer_provider.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/cc/resources/resource_pool.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/cc/resources/resource_pool.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/cc/resources/resource_pool_unittest.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/cc/test/fake_raster_buffer_provider.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/cc/test/test_image_factory.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/cc/test/test_image_factory.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/cc/tiles/tile_manager_unittest.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/chrome/browser/android/vr/arcore_device/ar_image_transport.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/chrome/browser/android/vr/vr_shell_gl.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/components/viz/service/display_embedder/buffer_queue_unittest.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/components/viz/test/test_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/content/browser/gpu/browser_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/command_buffer/service/image_factory.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/command_buffer/tests/gl_manager.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/command_buffer/tests/texture_image_factory.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/command_buffer/tests/texture_image_factory.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/client/command_buffer_proxy_impl.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/common/gpu_memory_buffer_impl.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/common/gpu_memory_buffer_impl.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/common/gpu_memory_buffer_impl_android_hardware_buffer.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/common/gpu_memory_buffer_impl_android_hardware_buffer.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/common/gpu_memory_buffer_impl_dxgi.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/common/gpu_memory_buffer_impl_dxgi.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/common/gpu_memory_buffer_impl_io_surface.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/common/gpu_memory_buffer_impl_io_surface.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/common/gpu_memory_buffer_impl_native_pixmap.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/common/gpu_memory_buffer_impl_native_pixmap.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/common/gpu_memory_buffer_impl_shared_memory.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/common/gpu_memory_buffer_impl_shared_memory.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/service/command_buffer_stub.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/service/command_buffer_stub.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/service/gpu_channel.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/service/gpu_channel.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/service/gpu_memory_buffer_factory_native_pixmap.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/gpu/ipc/service/gpu_memory_buffer_factory_native_pixmap.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/ipc/ipc_message_utils.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/ipc/ipc_message_utils.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/media/capture/video/chromeos/local_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/media/capture/video/chromeos/stream_buffer_manager.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/media/capture/video/mock_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/media/video/gpu_memory_buffer_video_frame_pool.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/media/video/mock_gpu_video_accelerator_factories.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/third_party/blink/renderer/platform/graphics/gpu/xr_frame_transport.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/ui/gfx/gpu_memory_buffer.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/ui/gfx/gpu_memory_buffer.h
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/ui/gfx/mojo/buffer_types_struct_traits.cc
[modify] https://crrev.com/0443a8fb9e6e862ee70aea4644eeea5297334416/ui/gfx/mojo/buffer_types_struct_traits.h
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 18 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f

commit 81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Thu Oct 18 08:30:34 2018

gfx: Convert gfx::GpuMemoryBufferHandle to the new shared memory API.

This CL replaces the deprecated base::SharedMemoryHandle in
gfx::GpuMemoryBufferHandle with the base::UnsafeSharedMemoryRegion.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I527a84876f8b87ddc652cf2f780a2893cb419c46
Bug:  854594 , 863011
Reviewed-on: https://chromium-review.googlesource.com/c/1106340
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600693}
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/cc/test/test_image_factory.cc
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/components/exo/display.cc
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/components/exo/display.h
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/components/exo/display_unittest.cc
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/components/exo/shared_memory.cc
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/components/exo/shared_memory.h
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/components/exo/shared_memory_unittest.cc
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/components/exo/wayland/server.cc
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/components/viz/host/host_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/components/viz/test/test_gpu_memory_buffer_manager.cc
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/gpu/ipc/common/gpu_memory_buffer_impl_shared_memory.cc
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/gpu/ipc/common/gpu_memory_buffer_impl_shared_memory.h
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/gpu/ipc/service/gpu_channel.cc
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/ui/gfx/gpu_memory_buffer.h
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/ui/gfx/ipc/gfx_param_traits_macros.h
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/ui/gfx/mojo/buffer_types.mojom
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/ui/gfx/mojo/buffer_types_struct_traits.cc
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/ui/gfx/mojo/buffer_types_struct_traits.h
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/ui/gfx/mojo/struct_traits_unittest.cc
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/ui/gl/gl_image_shared_memory.cc
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/ui/gl/gl_image_shared_memory.h
[modify] https://crrev.com/81c4128e3f75163f0a99c021c3bfc8ce5c0d7a0f/ui/gl/gl_image_shared_memory_unittest.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 19 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c80632d2b16884970961dc9a12bddd3a4ca50b5

commit 9c80632d2b16884970961dc9a12bddd3a4ca50b5
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Fri Oct 19 13:36:22 2018

gfx: Use union in mojom::GpuMemoryBufferHandle

The Mojo union more closely matches the actual intent of GpuMemoryBufferHandle,
since the handle can store only one of several types of platform handles.

GpuMemoryBufferHandle has |id| member that should be present all the time, so
mojom::GpuMemoryBufferHandle remains a Mojo struct and the union is embedded
into it.

Bug: 863011
Change-Id: I5dc9100392cd0a6f701e93ac4c91ca50e5077967
Reviewed-on: https://chromium-review.googlesource.com/c/1288609
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601130}
[modify] https://crrev.com/9c80632d2b16884970961dc9a12bddd3a4ca50b5/ui/gfx/mojo/buffer_types.mojom
[modify] https://crrev.com/9c80632d2b16884970961dc9a12bddd3a4ca50b5/ui/gfx/mojo/buffer_types_struct_traits.cc
[modify] https://crrev.com/9c80632d2b16884970961dc9a12bddd3a4ca50b5/ui/gfx/mojo/buffer_types_struct_traits.h
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 30 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/701a0bc9998a37cbe7dd4c2421b16f721b307510

commit 701a0bc9998a37cbe7dd4c2421b16f721b307510
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Tue Oct 30 20:34:29 2018

gpu: Add a (de)serialization test to native GpuMemoryBuffers

Recent bug in GpuMemoryBufferHandle deserialization on Android (fixed
in [1]) showed that there is no test coverage for seriazliation of
native GMB, at least on Android.

This CL adds a test checking serialization correctness for all subtypes
of GpuMemoryBufferImpl.

[1] https://crrev.com/c/1297469

Bug: 863011
Change-Id: I9688c1eed4b6dc7c5f2f7b1a20b08c2d62a35824
Reviewed-on: https://chromium-review.googlesource.com/c/1298209
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603999}
[modify] https://crrev.com/701a0bc9998a37cbe7dd4c2421b16f721b307510/gpu/BUILD.gn
[modify] https://crrev.com/701a0bc9998a37cbe7dd4c2421b16f721b307510/gpu/ipc/common/gpu_memory_buffer_impl_test_template.h
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 30 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c374ad0a3b3cfd71f1ba2fb1dab3f6202994539b

commit c374ad0a3b3cfd71f1ba2fb1dab3f6202994539b
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Tue Oct 30 23:12:38 2018

gfx: Make all GpuMemoryBufferTypes platform scoped

Some GpuMemoryBufferPlatformHandle union members are used only on one
platform but are defined for all platforms. This CL restricts
platform-specific members to desired platform.

Bug: 863011
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I6c09e49db157d9d91dc5caeac939ca334f9054ba
Reviewed-on: https://chromium-review.googlesource.com/c/1290917
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604049}
[modify] https://crrev.com/c374ad0a3b3cfd71f1ba2fb1dab3f6202994539b/ui/gfx/mojo/BUILD.gn
[modify] https://crrev.com/c374ad0a3b3cfd71f1ba2fb1dab3f6202994539b/ui/gfx/mojo/buffer_types.mojom
[modify] https://crrev.com/c374ad0a3b3cfd71f1ba2fb1dab3f6202994539b/ui/gfx/mojo/buffer_types_struct_traits.cc
[modify] https://crrev.com/c374ad0a3b3cfd71f1ba2fb1dab3f6202994539b/ui/gfx/mojo/buffer_types_struct_traits.h
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 5 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6df439ca8463a517458dcb6a21436a9fa23e4fca

commit 6df439ca8463a517458dcb6a21436a9fa23e4fca
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Wed Dec 05 11:54:46 2018

media/gpu/v4l2/V4L2SVDA: Remove unique_ptr for dmabuf_fds on ImportBufferForPictureTask()

std::unique_ptr is no longer necessary. There are two ways to remove unique_ptr
for std::vector<base::ScopedFD> on ImportBufferForPictureTask(). The first one
is to use base::Bind+base::Passed(), and the second one is base::BindOnce() +
std::move(). The second one seems straight-forward, understandable and safety.
So the first way is adopted.

Bug: 863011
Test: VDA unittest on kevin
Change-Id: I731d709cbef19be62b53b607f42437bcc30eda75
Reviewed-on: https://chromium-review.googlesource.com/c/1358614
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613941}
[modify] https://crrev.com/6df439ca8463a517458dcb6a21436a9fa23e4fca/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc
[modify] https://crrev.com/6df439ca8463a517458dcb6a21436a9fa23e4fca/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.h
►

Sign in to add a comment