[//ui] Remove unneeded header comment on GBMSupportX11 [chromium/src : main]
Colin Blundell (Gerrit)
Colin Blundell added 1 comment![]( "Open in Gerrit")
const std::vector<gfx::BufferUsageAndFormat> supported_configs_;Is there a reason this is actually [needed](https://source.chromium.org/chromium/chromium/src/+/main:ui/gfx/linux/gbm_support_x11.cc;l=136-144?q=gbm_support_x11.cc&ss=chromium), e.g. is it expensive to try to create a buffer and fail or do the `supported_configs_` results actually sometimes give different results than actually just trying to create the buffer in the course of normal execution and we want to privilege the `supported_configs_` results?
Related details
- Vasiliy Telezhnikov
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. |
Vasiliy Telezhnikov (Gerrit)
Vasiliy Telezhnikov voted and added 2 comments![]( "Open in Gerrit")
Votes added by Vasiliy Telezhnikov
| Code-Review | +1 |
2 comments
const std::vector<gfx::BufferUsageAndFormat> supported_configs_;Is there a reason this is actually [needed](https://source.chromium.org/chromium/chromium/src/+/main:ui/gfx/linux/gbm_support_x11.cc;l=136-144?q=gbm_support_x11.cc&ss=chromium), e.g. is it expensive to try to create a buffer and fail or do the `supported_configs_` results actually sometimes give different results than actually just trying to create the buffer in the course of normal execution and we want to privilege the `supported_configs_` results?
Well, trying to create buffer is kernel round-trip, not free, not particularly expensive.
At some point that code was a DCHECK, though I don't remember why we decided to handle it gracefully.
In general I think this boils to what we will do next. If we'd need to implement `IsNativePixmapConfigSupported` or if decide to change [this](https://source.chromium.org/chromium/chromium/src/+/main:gpu/command_buffer/service/shared_image/shared_image_factory.cc;drc=f94df6678b9dba1f9faf292374830260cc90ae54;l=389) to always try first and try shmem if it fails then we probably want to cache the results.
Related details
- Colin Blundell
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. |
Colin Blundell (Gerrit)
Colin Blundell voted and added 2 comments![]( "Open in Gerrit")
Votes added by Colin Blundell
| Commit-Queue | +2 |
2 comments
Thanks!
const std::vector<gfx::BufferUsageAndFormat> supported_configs_;Vasiliy TelezhnikovIs there a reason this is actually [needed](https://source.chromium.org/chromium/chromium/src/+/main:ui/gfx/linux/gbm_support_x11.cc;l=136-144?q=gbm_support_x11.cc&ss=chromium), e.g. is it expensive to try to create a buffer and fail or do the `supported_configs_` results actually sometimes give different results than actually just trying to create the buffer in the course of normal execution and we want to privilege the `supported_configs_` results?
Well, trying to create buffer is kernel round-trip, not free, not particularly expensive.
At some point that code was a DCHECK, though I don't remember why we decided to handle it gracefully.
In general I think this boils to what we will do next. If we'd need to implement `IsNativePixmapConfigSupported` or if decide to change [this](https://source.chromium.org/chromium/chromium/src/+/main:gpu/command_buffer/service/shared_image/shared_image_factory.cc;drc=f94df6678b9dba1f9faf292374830260cc90ae54;l=389) to always try first and try shmem if it fails then we probably want to cache the results.
Thanks, that makes sense! So we'll leave it as-is for now.
Related details
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
[//ui] Remove unneeded header comment on GBMSupportX11
This is no longer exposed on the interface, so there's no reason for a
comment about it in the header.
- M ui/gfx/linux/gbm_support_x11.h
Code-Review: +1 by Vasiliy Telezhnikov
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |