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

Issue 836488 link

Starred by 4 users
Status: Fixed
Owner:
█ sohanjg@chromium.org
Last visit > 30 days ago
Closed: Jul 3
Cc:
danakj@chromium.org
piman@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task



Sign in to add a comment

Remove TransferableResource::buffer_format

Project Member Reported by piman@chromium.org, Apr 24 2018 
It is redundant with TransferableResource::format, and potentially misleading because it is sometimes set even though no GpuMemoryBuffer was allocated for that resource. Even further it might lead to trusted code being confused if they don't match each other (e.g. verifying size through one but looking up pixels through the other).

Comment 1 by piman@chromium.org, Apr 24 2018 

AFAICT, the only places where they don't match (i.e. buffer_format != viz::BufferFormat(format) in prod are:
https://cs.chromium.org/chromium/src/cc/resources/video_resource_updater.cc?sq=package:chromium&dr=CSs&l=717
https://cs.chromium.org/chromium/src/components/exo/buffer.cc?sq=package:chromium&dr=CSs&l=469

In both of these, where buffer_format is set but format is not (or set to RGBA_8888 by default), and that is probably an omission/bug more than desired behavior (might cause issues in e.g. SkiaRenderer).

Comment 2 by sohanjg@chromium.org, Jun 13 2018 

piman@, I can have a look at this if you haven't started it yet ?

Comment 3 by piman@chromium.org, Jun 13 2018

Cc: piman@chromium.org
Owner: sohanjg@chromium.org
All yours :)
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 2 

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

commit fc3065ef8b01b61681ce95b1bab768869672d29a
Author: sohan <sohan.jyoti@huawei.com>
Date: Mon Jul 02 22:36:04 2018

viz: Add external GMB formats to ResourceFormat.

This adds external GpuMemoryBuffer formats to ResourceFormat, currently this
helps prevent redundant/misleading usage of formats in TransferableResource.

BUG= 836488 

Change-Id: I6d7cb07b31d2fd307de1cac15fdb305351bfaf4a
Reviewed-on: https://chromium-review.googlesource.com/1101678
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Sohan Jyoti Ghosh <sohan.jyoti@huawei.com>
Cr-Commit-Position: refs/heads/master@{#572026}
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/cc/layers/heads_up_display_layer_impl.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/cc/raster/raster_buffer_provider.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/cc/resources/resource_pool.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/cc/resources/resource_pool_unittest.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/components/exo/buffer.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/components/viz/client/client_resource_provider_unittest.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/components/viz/common/resources/platform_color.h
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/components/viz/common/resources/platform_color_unittest.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/components/viz/common/resources/resource_format.h
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/components/viz/common/resources/resource_format_utils.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/components/viz/common/resources/resource_format_utils.h
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/components/viz/common/resources/transferable_resource.h
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/components/viz/service/display/display_resource_provider.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/components/viz/service/display/gl_renderer.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/components/viz/service/display/renderer_pixeltest.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/components/viz/service/display/scoped_render_pass_texture.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/gpu/command_buffer/client/raster_implementation_gles.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/gpu/command_buffer/service/raster_cmd_validation_implementation_autogen.h
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/gpu/command_buffer/service/raster_decoder.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/media/renderers/video_resource_updater.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/media/renderers/video_resource_updater_unittest.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/services/viz/public/cpp/compositing/struct_traits_unittest.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/services/viz/public/cpp/compositing/transferable_resource_struct_traits.cc
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/services/viz/public/cpp/compositing/transferable_resource_struct_traits.h
[modify] https://crrev.com/fc3065ef8b01b61681ce95b1bab768869672d29a/services/viz/public/interfaces/compositing/transferable_resource.mojom

Comment 5 by sohanjg@chromium.org, Jul 3

Status: Fixed (was: Available)
►

Sign in to add a comment