Tomeu Vizoso <[email protected]> writes: > Virgl could save a lot of work converting buffers in the host side > between formats if Mesa supported a bunch of other formats when reading > pixels. > > This commit adds cases to handle specific formats so that the values > reported by the two calls match more closely the underlying native > formats. > > In GLES is important that IMPLEMENTATION_COLOR_READ_* return the native > format and data type because the spec only allows reading with those, > besides GL_RGBA or GL_RGBA_INTEGER. > > Additionally, because virgl currently doesn't implement such > conversions, this commit fixes several tests in > dEQP-GLES3.functional.fbo.color.clear.*, when using virgl in the guest > side. > > Additionally, assert that those functions' result match > _mesa_format_matches_format_and_type, so both functions are kept in > sync. > > Signed-off-by: Tomeu Vizoso <[email protected]> > > v2: * Let R10G10B10A2_UINT fall back to GL_RGBA_INTEGER (Eric Anholt) > * Assert with _mesa_format_matches_format_and_type (Eric Anholt) > --- > src/mesa/main/framebuffer.c | 117 ++++++++++++++++++++++++------------ > 1 file changed, 80 insertions(+), 37 deletions(-) > > diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c > index 8e751b453b75..6be4ecbc58f9 100644 > --- a/src/mesa/main/framebuffer.c > +++ b/src/mesa/main/framebuffer.c > @@ -834,22 +834,72 @@ _mesa_get_color_read_format(struct gl_context *ctx, > } > else { > const mesa_format format = fb->_ColorReadBuffer->Format; > - const GLenum data_type = _mesa_get_format_datatype(format); > - > - if (format == MESA_FORMAT_B8G8R8A8_UNORM) > - return GL_BGRA; > - else if (format == MESA_FORMAT_B5G6R5_UNORM) > - return GL_RGB; > - else if (format == MESA_FORMAT_R_UNORM8) > - return GL_RED; > - > - switch (data_type) { > - case GL_UNSIGNED_INT: > - case GL_INT: > - return GL_RGBA_INTEGER; > + GLenum gl_format, data_type; > + GLuint comps; > + > + _mesa_uncompressed_format_to_type_and_comps(format, &data_type, > &comps); > + > + switch(format) {
Needs a space: "switch (format) {"
> + case MESA_FORMAT_RGBA_UINT8:
> + gl_format = GL_RGBA_INTEGER;
> + break;
> + case MESA_FORMAT_B8G8R8A8_UNORM:
> + gl_format = GL_BGRA;
> + break;
> + case MESA_FORMAT_B5G6R5_UNORM:
> + case MESA_FORMAT_R11G11B10_FLOAT:
> + gl_format = GL_RGB;
> + break;
> + case MESA_FORMAT_RG_FLOAT32:
> + case MESA_FORMAT_RG_FLOAT16:
> + case MESA_FORMAT_R8G8_UNORM:
> + gl_format = GL_RG;
> + break;
> + case MESA_FORMAT_RG_SINT32:
> + case MESA_FORMAT_RG_UINT32:
> + case MESA_FORMAT_RG_SINT16:
> + case MESA_FORMAT_RG_UINT16:
> + case MESA_FORMAT_RG_SINT8:
> + case MESA_FORMAT_RG_UINT8:
> + gl_format = GL_RG_INTEGER;
> + break;
> + case MESA_FORMAT_R_FLOAT32:
> + case MESA_FORMAT_R_FLOAT16:
> + case MESA_FORMAT_R_UNORM8:
> + gl_format = GL_RED;
> + break;
> + case MESA_FORMAT_R_SINT32:
> + case MESA_FORMAT_R_UINT32:
> + case MESA_FORMAT_R_SINT16:
> + case MESA_FORMAT_R_UINT16:
> + case MESA_FORMAT_R_SINT8:
> + case MESA_FORMAT_R_UINT8:
> + gl_format = GL_RED_INTEGER;
> + break;
> default:
> - return GL_RGBA;
> + switch (data_type) {
> + case GL_UNSIGNED_INT:
> + case GL_UNSIGNED_INT_2_10_10_10_REV:
> + case GL_UNSIGNED_SHORT:
> + case GL_INT:
> + case GL_SHORT:
> + case GL_BYTE:
> + gl_format = GL_RGBA_INTEGER;
> + break;
> + default:
> + gl_format = GL_RGBA;
> + break;
> + }
> }
> +
> + /* GL_RGBA and GL_RGBA_INTEGER should be always valid */
> + assert(gl_format == GL_RGBA ||
> + gl_format == GL_RGBA_INTEGER ||
> + _mesa_format_matches_format_and_type(format, gl_format,
> + data_type,
> + ctx->Pack.SwapBytes,
> NULL));
> +
I like the idea, but it looks like this assertion won't work out --
R8G8_UNORM will always assertion fail on !littleEndian, for example.
Assertion failing when SwapBytes changes would also be bad.
> + return gl_format;
> }
> }
>
> @@ -882,29 +932,22 @@ _mesa_get_color_read_type(struct gl_context *ctx,
> return GL_NONE;
> }
> else {
> - const GLenum format = fb->_ColorReadBuffer->Format;
> - const GLenum data_type = _mesa_get_format_datatype(format);
> -
> - if (format == MESA_FORMAT_B5G6R5_UNORM)
> - return GL_UNSIGNED_SHORT_5_6_5;
> -
> - if (format == MESA_FORMAT_B10G10R10A2_UNORM ||
> - format == MESA_FORMAT_B10G10R10X2_UNORM ||
> - format == MESA_FORMAT_R10G10B10A2_UNORM ||
> - format == MESA_FORMAT_R10G10B10X2_UNORM)
> - return GL_UNSIGNED_INT_2_10_10_10_REV;
> -
> - switch (data_type) {
> - case GL_SIGNED_NORMALIZED:
> - return GL_BYTE;
> - case GL_UNSIGNED_INT:
> - case GL_INT:
> - case GL_FLOAT:
> - return data_type;
> - case GL_UNSIGNED_NORMALIZED:
> - default:
> - return GL_UNSIGNED_BYTE;
> - }
> + const mesa_format format = fb->_ColorReadBuffer->Format;
> + GLenum gl_format, data_type;
> + GLuint comps;
> +
> + _mesa_uncompressed_format_to_type_and_comps(format, &data_type,
> &comps);
> +
> + gl_format = _mesa_get_color_read_format(ctx, fb, NULL);
> +
> + /* GL_RGBA and GL_RGBA_INTEGER should be always valid */
> + assert(gl_format == GL_RGBA ||
> + gl_format == GL_RGBA_INTEGER ||
> + _mesa_format_matches_format_and_type(format, gl_format,
> + data_type,
> + ctx->Pack.SwapBytes,
> NULL));
> +
> + return data_type;
> }
> }
>
> --
> 2.17.0
>
> _______________________________________________
> mesa-dev mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
