Use `const struct pixel_format_info *` instead of plain DRM or Pixman format codes
Currently we have:
struct pixman_renderer_output_options {
/** Composite into a shadow buffer, copying to the hardware buffer */
bool use_shadow;
/** Initial framebuffer size */
struct weston_size fb_size;
/** Initial DRM pixel format */
uint32_t drm_format;
};
The uint32_t drm_format
field should be changed into const struct pixel_format_info *fb_pfmt
. DRM formats are not native to Pixman renderer.
I am trying to standardize everything in the compositor to use const struct pixel_format_info *
as much as possible, because it helps debugging (easy to get the name of the format), it ensures that we actually have a pixel format info for any format we happen use, and it is API-agnostic in that it can provide DRM, Pixman, or OpenGL format codes as needed.
Likewise weston_output_update_capture_info()
should be reverted to take const struct pixel_format_info *
.
There is now a problem in the struct pixman_renderer_interface
: a backend needs to pass a DRM format in pixman_renderer_output_options
which gets used only for weston_output_update_capture_info()
. The API to actually create pixman weston_renderbuffers
takes a pixman_format_code_t
instead. There is no connection between these two while they are still required to match.
The weston_renderbuffer
APIs should take const struct pixel_format_info *
as well.
Maybe changing pixel formats needs to be forbidden:
- have the initial pixel format in output-create-options
- allocate renderbuffers for a specific output which determines the size and format:
-
create_image_from_ptr()
asserts that the given size and format matches the recorded size and format -
create_image()
has no size and format arguments at all
-
Changing pixel formats is so uncommon that one might as well just tear down the whole renderer-output and create a new one.
There already is the assumption that all renderbuffers allocated for a certain output will be used with that output only, because of the new damage tracking that replaced the extra-hw-damage. I think it's a good direction, it just needs to be taken further.
This way the capture-info management will fit the architecture well rather than being something you need to remember to update on the side. That's what the resize API was created for.
How would that sound?
Btw. pixman_renderer_interface::create_image_no_clear
could be called just create_image
. The "no clear" has no meaning when there is no existing pointer being passed in. The implementation just uses pixman_image_create_bits_no_clear()
to avoid memsetting the memory to zero. Alternatively, pixman_renderer_interface::create_image_no_clear
could be simply deleted if passing a NULL pointer to create_image_from_ptr
does the equivalent. But then all callers need to pass in correct size and format.