Convert callers to BytesPerPixel() [chromium/src : main]
Kyle Charbonneau (Gerrit)
New activity on the change![]( "Open in Gerrit")
Related details
- Colin Blundell
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Colin Blundell (Gerrit)
Colin Blundell voted and added 2 comments![]( "Open in Gerrit")
Votes added by Colin Blundell
| Code-Review | +1 |
2 comments
int bytes_per_element = format.BytesPerPixel();This will now crash for ETC1 whereas the previous code would return null. Is that intentional, or do you know that this entrypoint will never be hit for ETC1? If you don't know that, maybe want to retain a check for ETC1 specifically that returns nullptr here?
Related details
- Kyle Charbonneau
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kyle Charbonneau (Gerrit)
Kyle Charbonneau added 1 comment![]( "Open in Gerrit")
int bytes_per_element = format.BytesPerPixel();This will now crash for ETC1 whereas the previous code would return null. Is that intentional, or do you know that this entrypoint will never be hit for ETC1? If you don't know that, maybe want to retain a check for ETC1 specifically that returns nullptr here?
It's intentional. ETC1 is only used on Android so we can rule out D3DImageBacking from that. It's also not in [this list](https://source.chromium.org/chromium/chromium/src/+/main:gpu/command_buffer/service/shared_image/d3d_image_backing_factory.cc;l=145-146;drc=45a1118875a1b3a2a1ae9fee639cf1b546a43547) and there is no way to update ETC1 textures after creation so it would have to be.
ETC1 is only supported in WrappedSk/WrappedGraphite/GLTexture backings. I don't think we actually create GLTextureImageBacking with ETC1 anymore but the code exists and we'd have to verify it's unused.
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Colin Blundell (Gerrit)
Colin Blundell added 1 comment![]( "Open in Gerrit")
int bytes_per_element = format.BytesPerPixel();Kyle CharbonneauThis will now crash for ETC1 whereas the previous code would return null. Is that intentional, or do you know that this entrypoint will never be hit for ETC1? If you don't know that, maybe want to retain a check for ETC1 specifically that returns nullptr here?
It's intentional. ETC1 is only used on Android so we can rule out D3DImageBacking from that. It's also not in [this list](https://source.chromium.org/chromium/chromium/src/+/main:gpu/command_buffer/service/shared_image/d3d_image_backing_factory.cc;l=145-146;drc=45a1118875a1b3a2a1ae9fee639cf1b546a43547) and there is no way to update ETC1 textures after creation so it would have to be.
ETC1 is only supported in WrappedSk/WrappedGraphite/GLTexture backings. I don't think we actually create GLTextureImageBacking with ETC1 anymore but the code exists and we'd have to verify it's unused.
- Kyle Charbonneau
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kyle Charbonneau (Gerrit)
Kyle Charbonneau voted Commit-Queue+2![]( "Open in Gerrit")
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Convert callers to BytesPerPixel()
All callers of SharedImageFormat::BitsPerPixel() that divide by 8 can
just call BytesPerPixel(). That is all callers so this also removes
BitsPerPixel().
The only caller that isn't simply replacing BitsPerPixel() / 8 is
TryAllocateSkDataForBitmap)(). The logic there was identical to
EstimatedSizeInBytes() so that is used instead.
- M components/viz/common/resources/shared_image_format.cc
- M components/viz/common/resources/shared_image_format.h
- M gpu/command_buffer/service/shared_image/ahardwarebuffer_image_backing_factory.cc
- M gpu/command_buffer/service/shared_image/d3d_image_backing_factory.cc
- M gpu/command_buffer/service/shared_image/iosurface_image_backing.mm
- M gpu/command_buffer/service/webgpu_decoder_impl.cc
- M third_party/blink/renderer/core/html/canvas/canvas_rendering_context.cc
- M third_party/blink/renderer/platform/graphics/gpu/drawing_buffer.cc
- M ui/gfx/mac/io_surface.cc
- M ui/ozone/platform/flatland/flatland_sysmem_buffer_collection.cc
Code-Review: +1 by Colin Blundell
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |