[Mesa-dev] [RFC PATCH 1/8] Gallium: Add new CAPS to indicate whether a driver can switch SRGB write

[Ilia Mirkin]

On Tue, Nov 13, 2018 at 12:28 PM Gert Wollny <gw.fossdev at gmail.com> wrote:
>
> From: Gert Wollny <gert.wollny at collabora.com>
>
> Add a new cap that indicates whether the drivers supports
> enabling/disabling the conversion from linear space to sRGB
> for a framebuffer attachment.
>
> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> ---
>  src/gallium/auxiliary/util/u_screen.c | 3 +++
>  src/gallium/docs/source/screen.rst    | 3 +++
>  src/gallium/include/pipe/p_defines.h  | 1 +
>  3 files changed, 7 insertions(+)
>
> diff --git a/src/gallium/auxiliary/util/u_screen.c b/src/gallium/auxiliary/util/u_screen.c
> index 73dbbee94a..1d9f367501 100644
> --- a/src/gallium/auxiliary/util/u_screen.c
> +++ b/src/gallium/auxiliary/util/u_screen.c
> @@ -326,6 +326,9 @@ u_pipe_screen_get_param_defaults(struct pipe_screen *pscreen,
>     case PIPE_CAP_MAX_VERTEX_ELEMENT_SRC_OFFSET:
>        return 2047;
>
> +   case PIPE_CAP_SRGB_WRITE_CONTROL:
> +      return 1;
> +
>     default:
>        unreachable("bad PIPE_CAP_*");
>     }
> diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst
> index 0abd164494..da677eb04b 100644
> --- a/src/gallium/docs/source/screen.rst
> +++ b/src/gallium/docs/source/screen.rst
> @@ -477,6 +477,9 @@ subpixel precision bias in bits during conservative rasterization.
>    0 means no limit.
>  * ``PIPE_CAP_MAX_VERTEX_ELEMENT_SRC_OFFSET``: The maximum supported value for
>    of pipe_vertex_element::src_offset.
> +* ``PIPE_CAP_SRGB_WRITE_CONTROL``: Indicates whether the drivers on GLES supports

I don't think mention of "GLES" is appropriate here. This is a
hardware capability spec.

> +  enabling/disabling the conversion from linear space to sRGB at framebuffer or
> +  blend time.

What does this mean in practice? util_format_is_srgb(resource->format)
== util_format_is_srgb(surface->format)? And something similar for
blits? If so, how about e.g. "PIPE_CAP_SURFACE_SRGB_CONTROL". Whatever
it is, the list of things it enables should be explicitly listed here.

I haven't read the remainder of the series, but whatever the invariant
is, I'd strongly encourage you to add asserts for it in the relevant
backend's create_surface() and blit() calls. Or at least
pipe_debug_messages.

Cheers,

  -ilia

>
>  .. _pipe_capf:
>
> diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
> index 693f041b1d..7838b18be8 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -826,6 +826,7 @@ enum pipe_cap
>     PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER_BUFFERS,
>     PIPE_CAP_MAX_TEXTURE_UPLOAD_MEMORY_BUDGET,
>     PIPE_CAP_MAX_VERTEX_ELEMENT_SRC_OFFSET,
> +   PIPE_CAP_SRGB_WRITE_CONTROL,
>  };
>
>  /**
> --
> 2.18.1
>