On 02/01/2013 08:13 AM, Jose Fonseca wrote:
----- Original Message -----If we call glTexImage2D() with a generic compression format (e.g. intFormat=GL_COMPRESSED_RGBA) we can't choose a DXT format if we don't have the external DXT compression library. We weren't actually enforcing this before since the pipe_screen::is_format_supported(DXT) query has no dependency on the DXT compression library. Now if we're given a generic compressed format and we can't do DXT compression we'll fall back to a non-compressed format. Note: This is a candidate for the stable branches. --- src/mesa/state_tracker/st_cb_drawpixels.c | 6 ++- src/mesa/state_tracker/st_cb_texture.c | 2 +- src/mesa/state_tracker/st_format.c | 47 ++++++++++++++++++++++++----- src/mesa/state_tracker/st_format.h | 2 +- src/mesa/state_tracker/st_texture.c | 3 +- 5 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c b/src/mesa/state_tracker/st_cb_drawpixels.c index c944b81..c3c5326 100644 --- a/src/mesa/state_tracker/st_cb_drawpixels.c +++ b/src/mesa/state_tracker/st_cb_drawpixels.c @@ -1499,14 +1499,16 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx, GLint srcy, if (type == GL_DEPTH) { texFormat = st_choose_format(screen, GL_DEPTH_COMPONENT, GL_NONE, GL_NONE, st->internal_target, - sample_count, PIPE_BIND_DEPTH_STENCIL); + sample_count, PIPE_BIND_DEPTH_STENCIL, + FALSE); assert(texFormat != PIPE_FORMAT_NONE); } else { /* default color format */ texFormat = st_choose_format(screen, GL_RGBA, GL_NONE, GL_NONE, st->internal_target, - sample_count, PIPE_BIND_SAMPLER_VIEW); + sample_count, PIPE_BIND_SAMPLER_VIEW, + FALSE); assert(texFormat != PIPE_FORMAT_NONE); } } diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c index 3cea2df..80a440d 100644 --- a/src/mesa/state_tracker/st_cb_texture.c +++ b/src/mesa/state_tracker/st_cb_texture.c @@ -597,7 +597,7 @@ decompress_with_blit(struct gl_context * ctx, /* Find the best match for the format+type combo. */ pipe_format = st_choose_format(pipe->screen, GL_RGBA8, format, type, - pipe_target, 0, bind); + pipe_target, 0, bind, FALSE); if (pipe_format == PIPE_FORMAT_NONE) { /* unable to get an rgba format!?! */ _mesa_problem(ctx, "%s: cannot find a supported format", __func__); diff --git a/src/mesa/state_tracker/st_format.c b/src/mesa/state_tracker/st_format.c index 7ef0639..c3b286a 100644 --- a/src/mesa/state_tracker/st_format.c +++ b/src/mesa/state_tracker/st_format.c @@ -1397,19 +1397,48 @@ static const struct format_mapping format_map[] = { /** + * Is the given pipe format a DXT format? + */ +static boolean +is_dxt_format(enum pipe_format format) +{ + switch (format) { + case PIPE_FORMAT_DXT1_RGB: + case PIPE_FORMAT_DXT1_RGBA: + case PIPE_FORMAT_DXT3_RGBA: + case PIPE_FORMAT_DXT5_RGBA: + case PIPE_FORMAT_DXT1_SRGB: + case PIPE_FORMAT_DXT1_SRGBA: + case PIPE_FORMAT_DXT3_SRGBA: + case PIPE_FORMAT_DXT5_SRGBA: + return TRUE; + default: + return FALSE; + } +}I think you could use u_format.h's util_format_is_s3tc here.
Yup, I had grepped for "dxt" instead of "s3tc".
+ +/** * Return first supported format from the given list. + * \param allow_dxt indicates whether it's OK to return a DXT format. */ static enum pipe_format find_supported_format(struct pipe_screen *screen, const enum pipe_format formats[], enum pipe_texture_target target, unsigned sample_count, - unsigned tex_usage) + unsigned tex_usage, + boolean allow_dxt) { uint i; for (i = 0; formats[i]; i++) { if (screen->is_format_supported(screen, formats[i], target, sample_count, tex_usage)) { + if (!allow_dxt&& is_dxt_format(formats[i])) { + /* we can't return a dxt format, continue searching */ + continue; + } + return formats[i]; } } @@ -1519,7 +1548,7 @@ enum pipe_format st_choose_format(struct pipe_screen *screen, GLenum internalFormat, GLenum format, GLenum type, enum pipe_texture_target target, unsigned sample_count, - unsigned bindings) + unsigned bindings, boolean allow_dxt)It looks like parameter is serving two duties: - some cases it is set to FALSE because we don't have the DXTn compression library available - some cases it is set to FALSE because we don't want compressed textures, but truth is that setting FALSE will only black lists DXT and happily return non-DXT compressed libraries (KTX, etc).
The cases which I think you're looking at (st_cb_drawpixels.c, decompress_with_blit(), st_choose_renderbuffer_format(), etc) will never choose a compressed format so the allow_dxt argument's value doesn't matter. I didn't feel like making allow_dxt a tri-valued (true/false/dont-care) parameter.
Remember, the only time we choose a compressed format is when the "internalFormat" parameter is a generic or specific compressed format. That can only happen when we're called via glTexImage().
I can improve the comments to explain this.
If so, then I'd suggest rename allow_dxt to allow_compressed, and check ctx->Mesa_DXTn when is_dxt_format() is true. Otherwise the series looks good.
Thanks for reviewing! -Brian _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
