New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 900044 link

Starred by 2 users
Status: Assigned
Owner:
piman@chromium.org
Cc:
█ hoegsberg@chromium.org
dsinclair@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocking:
issue 870116



Sign in to add a comment

Sanitize internalformat handling in GpuMemoryBuffer/GLImage

Project Member Reported by piman@chromium.org, Oct 30 
There is a lot of redundant information flowing through the system, as in almost all places the GMB's BufferFormat fully describes the internalformat that would be used in GL. There's also a lot of ambiguity about the purpose of the internalformat. It results in a lot of duplication and inconsistencies in enum translation.

Where appropriate, remove the internalformat in this code.

Comment 1 by piman@chromium.org, Oct 30

Blocking: 870116
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 30 

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

commit 7e7665c3fd26aff0bb63f417ad57053f9c68dd8b
Author: Antoine Labour <piman@chromium.org>
Date: Tue Oct 30 21:28:49 2018

Remove internalformat from ImageFactory entrypoints

Both CreateImageForGpuMemoryBuffer and CreateAnonymousImage take a BufferFormat
which explains the format, and all callers pass a consistent internalformat
(equal to the result of gpu::InternalFormatForGpuMemoryBufferFormat). Most
GLImage implementations refuse a mismatch anyway.

Bug: 900044
Change-Id: I7b1ba0711bc4242f263932c40644f884cbe081ce
Reviewed-on: https://chromium-review.googlesource.com/c/1306780
Commit-Queue: Antoine Labour <piman@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604018}
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/cc/test/test_image_factory.cc
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/cc/test/test_image_factory.h
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/command_buffer/service/image_factory.cc
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/command_buffer/service/image_factory.h
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/command_buffer/service/raster_decoder.cc
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/command_buffer/service/shared_image_backing_factory_gl_texture.cc
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/command_buffer/tests/gl_manager.cc
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/command_buffer/tests/texture_image_factory.cc
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/command_buffer/tests/texture_image_factory.h
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/ipc/service/gpu_channel.cc
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.cc
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.h
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.cc
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/ipc/service/gpu_memory_buffer_factory_dxgi.h
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.cc
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/ipc/service/gpu_memory_buffer_factory_io_surface.h
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/ipc/service/gpu_memory_buffer_factory_native_pixmap.cc
[modify] https://crrev.com/7e7665c3fd26aff0bb63f417ad57053f9c68dd8b/gpu/ipc/service/gpu_memory_buffer_factory_native_pixmap.h
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 31 

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

commit 6ffacb3fa72c4f413ff275bfb31dad5bbab675bc
Author: Antoine Labour <piman@chromium.org>
Date: Wed Oct 31 01:55:28 2018

Remove internalformat argument from GLImage*Memory

All callers set it to something consistent with the BufferFormat (i.e.
the value of gpu::InternalFormatForGpuMemoryBufferFormat), and so is
redundant. For GetInternalFormat, we can directly extract it from the
BufferFormat.

Bug: 900044
Change-Id: I5e135d60dedf3854fbc28a29abcc26054dd663f7
Reviewed-on: https://chromium-review.googlesource.com/c/1306993
Commit-Queue: Antoine Labour <piman@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604108}
[modify] https://crrev.com/6ffacb3fa72c4f413ff275bfb31dad5bbab675bc/cc/test/test_image_factory.cc
[modify] https://crrev.com/6ffacb3fa72c4f413ff275bfb31dad5bbab675bc/gpu/command_buffer/tests/gl_manager.cc
[modify] https://crrev.com/6ffacb3fa72c4f413ff275bfb31dad5bbab675bc/gpu/ipc/in_process_command_buffer.cc
[modify] https://crrev.com/6ffacb3fa72c4f413ff275bfb31dad5bbab675bc/gpu/ipc/service/direct_composition_surface_win_unittest.cc
[modify] https://crrev.com/6ffacb3fa72c4f413ff275bfb31dad5bbab675bc/gpu/ipc/service/gpu_channel.cc
[modify] https://crrev.com/6ffacb3fa72c4f413ff275bfb31dad5bbab675bc/ui/gl/gl_image_memory.cc
[modify] https://crrev.com/6ffacb3fa72c4f413ff275bfb31dad5bbab675bc/ui/gl/gl_image_memory.h
[modify] https://crrev.com/6ffacb3fa72c4f413ff275bfb31dad5bbab675bc/ui/gl/gl_image_ref_counted_memory.cc
[modify] https://crrev.com/6ffacb3fa72c4f413ff275bfb31dad5bbab675bc/ui/gl/gl_image_ref_counted_memory.h
[modify] https://crrev.com/6ffacb3fa72c4f413ff275bfb31dad5bbab675bc/ui/gl/gl_image_ref_counted_memory_unittest.cc
[modify] https://crrev.com/6ffacb3fa72c4f413ff275bfb31dad5bbab675bc/ui/gl/gl_image_shared_memory.cc
[modify] https://crrev.com/6ffacb3fa72c4f413ff275bfb31dad5bbab675bc/ui/gl/gl_image_shared_memory.h
[modify] https://crrev.com/6ffacb3fa72c4f413ff275bfb31dad5bbab675bc/ui/gl/gl_image_shared_memory_unittest.cc

Comment 4 by vmi...@chromium.org, Nov 2

Labels: -Type-Bug Type-Feature
Status: Assigned (was: Untriaged)

Comment 5 by hoegsberg@google.com, Jan 11

Cc: hoegsberg@chromium.org

Comment 6 by hoegsberg@google.com, Jan 11 

Yay, good to see this land.
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 11 

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

commit 561141ebb714c8cb308bc47b04d37468637dfb70
Author: Antoine Labour <piman@chromium.org>
Date: Fri Jan 11 19:35:13 2019

Replace GLImageNativePixmap internalformat with BufferFormat

All callers pass an internalformat consistent with
gpu::InternalFormatForGpuMemoryBufferFormat, so there's no need to
specify redundantly the internalformat and the BufferFormat. Instead
just pass in the BufferFormat, which simplifies all callers and
increases consistency.

Bug: 900044
Change-Id: I19916a6f53a19e1419e3312a2ea4368d79fd95e4
Reviewed-on: https://chromium-review.googlesource.com/c/1405286
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622094}
[modify] https://crrev.com/561141ebb714c8cb308bc47b04d37468637dfb70/gpu/command_buffer/tests/gl_test_utils.cc
[modify] https://crrev.com/561141ebb714c8cb308bc47b04d37468637dfb70/gpu/ipc/service/gpu_memory_buffer_factory_native_pixmap.cc
[modify] https://crrev.com/561141ebb714c8cb308bc47b04d37468637dfb70/media/gpu/v4l2/generic_v4l2_device.cc
[modify] https://crrev.com/561141ebb714c8cb308bc47b04d37468637dfb70/media/gpu/vaapi/vaapi_picture_native_pixmap.cc
[modify] https://crrev.com/561141ebb714c8cb308bc47b04d37468637dfb70/media/gpu/vaapi/vaapi_picture_native_pixmap.h
[modify] https://crrev.com/561141ebb714c8cb308bc47b04d37468637dfb70/media/gpu/vaapi/vaapi_picture_native_pixmap_egl.cc
[modify] https://crrev.com/561141ebb714c8cb308bc47b04d37468637dfb70/media/gpu/vaapi/vaapi_picture_native_pixmap_ozone.cc
[modify] https://crrev.com/561141ebb714c8cb308bc47b04d37468637dfb70/ui/gl/gl_image_native_pixmap.cc
[modify] https://crrev.com/561141ebb714c8cb308bc47b04d37468637dfb70/ui/gl/gl_image_native_pixmap.h
[modify] https://crrev.com/561141ebb714c8cb308bc47b04d37468637dfb70/ui/gl/gl_image_native_pixmap_unittest.cc
[modify] https://crrev.com/561141ebb714c8cb308bc47b04d37468637dfb70/ui/ozone/demo/skia/skia_surfaceless_gl_renderer.cc
[modify] https://crrev.com/561141ebb714c8cb308bc47b04d37468637dfb70/ui/ozone/demo/surfaceless_gl_renderer.cc
[modify] https://crrev.com/561141ebb714c8cb308bc47b04d37468637dfb70/ui/ozone/gl/gl_image_ozone_native_pixmap_unittest.cc
►

Sign in to add a comment