<html><head></head><body><div>On Thu, 2018-01-11 at 07:38 -0800, Jason Ekstrand wrote:</div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jan 11, 2018 at 1:48 AM, Iago Toral <span dir="ltr"><<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>></span> wrote:<br><blockquote type="cite"><div class="gmail-HOEnZb"><div class="gmail-h5">On Wed, 2018-01-10 at 11:22 -0800, Jason Ekstrand wrote:<br>
> ---<br>
> src/mesa/drivers/dri/i965/<wbr>brw_draw.c | 41<br>
> ++++++++++++++++++++++++++++++<wbr>++++++<br>
> 1 file changed, 41 insertions(+)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/<wbr>brw_draw.c<br>
> b/src/mesa/drivers/dri/i965/<wbr>brw_draw.c<br>
> index 4945dec..9fd44e4 100644<br>
> --- a/src/mesa/drivers/dri/i965/<wbr>brw_draw.c<br>
> +++ b/src/mesa/drivers/dri/i965/<wbr>brw_draw.c<br>
> @@ -40,6 +40,7 @@<br>
> #include "swrast_setup/swrast_setup.h"<br>
> #include "drivers/common/meta.h"<br>
> #include "util/bitscan.h"<br>
> +#include "util/bitset.h"<br>
> <br>
> #include "brw_blorp.h"<br>
> #include "brw_draw.h"<br>
> @@ -371,6 +372,20 @@ intel_disable_rb_aux_buffer(<wbr>struct brw_context<br>
> *brw,<br>
> return found;<br>
> }<br>
> <br>
> +static void<br>
> +mark_textures_used_for_txf(<wbr>BITSET_WORD *used_for_txf,<br>
> + <wbr>const struct gl_program *prog)<br>
> +{<br>
> + if (!prog)<br>
> + return;<br>
> +<br>
> + unsigned mask = prog->SamplersUsed & prog-<br>
> >info.textures_used_by_txf;<br>
> + while (mask) {<br>
> + int s = u_bit_scan(&mask);<br>
> + BITSET_SET(used_for_<wbr>txf, prog->SamplerUnits[s]);<br>
> + }<br>
> +}<br>
> +<br>
> /**<br>
> * \brief Resolve buffers before drawing.<br>
> *<br>
> @@ -386,6 +401,18 @@ brw_predraw_resolve_inputs(<wbr>struct brw_context<br>
> *brw, bool rendering)<br>
> memset(brw->draw_aux_<wbr>buffer_disabled, 0,<br>
> sizeof(brw->draw_<wbr>aux_buffer_disabled));<br>
> <br>
> + BITSET_DECLARE(used_for_<wbr>txf, MAX_COMBINED_TEXTURE_IMAGE_<wbr>UNITS);<br>
> + memset(used_for_txf, 0, sizeof(used_for_txf));<br>
> + if (rendering) {<br>
> + mark_textures_used_for_<wbr>txf(used_for_txf, ctx-<br>
> >VertexProgram._Current);<br>
> + mark_textures_used_for_<wbr>txf(used_for_txf, ctx-<br>
> >TessCtrlProgram._Current);<br>
> + mark_textures_used_for_<wbr>txf(used_for_txf, ctx-<br>
> >TessEvalProgram._Current);<br>
> + mark_textures_used_for_<wbr>txf(used_for_txf, ctx-<br>
> >GeometryProgram._Current);<br>
> + mark_textures_used_for_<wbr>txf(used_for_txf, ctx-<br>
> >FragmentProgram._Current);<br>
> + } else {<br>
> + mark_textures_used_for_<wbr>txf(used_for_txf, ctx-<br>
> >ComputeProgram._Current);<br>
> + }<br>
> +<br>
> /* Resolve depth buffer and render cache of each enabled texture.<br>
> */<br>
> int maxEnabledUnit = ctx->Texture._<wbr>MaxEnabledTexImageUnit;<br>
> for (int i = 0; i <= maxEnabledUnit; i++) {<br>
> @@ -422,6 +449,20 @@ brw_predraw_resolve_inputs(<wbr>struct brw_context<br>
> *brw, bool rendering)<br>
> <wbr> min_layer, num_layers,<br>
> <wbr> disable_aux);<br>
> <br>
> + /* If any programs are using it with texelFetch, we may need<br>
> to also do<br>
> + * a prepare with an sRGB format to ensure texelFetch works<br>
> "properly".<br>
> + */<br>
<br>
</div></div>I am not sure I understand this. The only way that txf_format and<br>
view_format can be different is if the texture format is sRGB and the<br>
user has selected GL_SKIP_DECODE_EXT, right? If the user selected that,<br>
it would mean that they do not want sRGB decoded data when sampling,<br>
but I understand that is what they would get, since we are preparing<br>
the MT with the sRGB format in this case. What am I missing?<br><div class="gmail-HOEnZb"><div class="gmail-h5"></div></div><br></blockquote><div><br></div><div>The utterly insane rules around texelFetch in EXT_texture_sRGB_decode. :-) I'd quote them here but it involves a table so that may not come out well in e-mail. The short version is that, if you use texelFetch, GL_SKIP_DECODE_EXT is ignored because it's part of the sampler and texelFetch doesn't use a sampler. It's utterly insane but somehow that's where "design by committee" got us. The fallout is that if we see a texelFetch, we need to do intel_miptree_prepare_texture with both sRGB and UNORM formats.<br></div></div></div></div></blockquote><div><br></div><div>Ugh, just read the table, yeah, that not very obvious... So, the second call to intel_miptree_prepare_texture has basically two purposes:</div><div><br></div><div>1. Actually ignore GL_SKIP_DECODE_EXT for texelFetch operations</div><div>2. And trigger a resolve operation if needed</div><div><br></div><div>Ok, that makes sense now. Thanks for the explanation.</div><div><br></div><div>Patches 5-6 are:</div><div>Reviewed-by: Iago Toral Quiroga <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>></div><div><br></div><div><br></div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote type="cite"><div class="gmail-HOEnZb"><div class="gmail-h5">
> + if (BITSET_TEST(used_for_txf, i)) {<br>
> + enum isl_format txf_format =<br>
> + translate_tex_<wbr>format(brw, tex_obj->_Format,<br>
> GL_DECODE_EXT);<br>
> + if (txf_format != view_format) {<br>
> + intel_miptree_<wbr>prepare_texture(brw, tex_obj->mt,<br>
> txf_format,<br>
> + <wbr> min_level, num_levels,<br>
> + <wbr> min_layer, num_layers,<br>
> + <wbr> disable_aux);<br>
> + }<br>
> + }<br>
> +<br>
> brw_cache_flush_for_<wbr>read(brw, tex_obj->mt->bo);<br>
> <br>
> if (tex_obj->base.StencilSampling ||<br>
</div></div><br></blockquote></div><br></div></div>
</blockquote></body></html>