[Mesa-dev] [PATCH 1/2] st/glsl_to_tgsi: ignore GL_TEXTURE_SRGB_DECODE_EXT for samplers used with texelFetch*()

Marek Olšák maraeo at gmail.com
Tue Oct 10 17:31:32 UTC 2017


Reviewed-by: Marek Olšák <marek.olsak at amd.com>

BTW, desktop OpenGL does have GL_TEXTURE_SRGB_DECODE_EXT, so the
comments are incorrect.

Marke

On Fri, Oct 6, 2017 at 10:39 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> See the comment for the relevant spec quote.
>
> Fixes dEQP-GLES31.functional.srgb_texture_decode.skip_decode.srgba8.texel_fetch
> --
> Note that this is on top of the texture locking series which I have
> sent out a minute ago.
> ---
>  src/mesa/main/mtypes.h                     |  1 +
>  src/mesa/state_tracker/st_atom_texture.c   | 39 +++++++++++++++++++++++++++---
>  src/mesa/state_tracker/st_cb_texture.c     |  8 +++++-
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  4 +++
>  src/mesa/state_tracker/st_sampler_view.c   | 19 +++++++++------
>  src/mesa/state_tracker/st_sampler_view.h   |  3 ++-
>  src/mesa/state_tracker/st_texture.c        |  7 +++++-
>  src/mesa/state_tracker/st_texture.h        |  7 +++---
>  8 files changed, 71 insertions(+), 17 deletions(-)
>
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index abda1a36e46..a45409ed2d9 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2081,20 +2081,21 @@ struct gl_program
>
>     bool is_arb_asm; /** Is this an ARB assembly-style program */
>
>     /** Is this program written to on disk shader cache */
>     bool program_written_to_cache;
>
>     GLbitfield64 SecondaryOutputsWritten; /**< Subset of OutputsWritten outputs written with non-zero index. */
>     GLbitfield TexturesUsed[MAX_COMBINED_TEXTURE_IMAGE_UNITS];  /**< TEXTURE_x_BIT bitmask */
>     GLbitfield SamplersUsed;   /**< Bitfield of which samplers are used */
>     GLbitfield ShadowSamplers; /**< Texture units used for shadow sampling. */
> +   GLbitfield TexelFetchSamplers; /**< Texture units used for texelFetch*(). */
>     GLbitfield ExternalSamplersUsed; /**< Texture units used for samplerExternalOES */
>
>     /* Fragement shader only fields */
>     GLboolean OriginUpperLeft;
>     GLboolean PixelCenterInteger;
>
>     /** Named parameters, constants, etc. from program text */
>     struct gl_program_parameter_list *Parameters;
>
>     /** Map from sampler unit to texture unit (set by glUniform1i()) */
> diff --git a/src/mesa/state_tracker/st_atom_texture.c b/src/mesa/state_tracker/st_atom_texture.c
> index 90828bb4cf9..c350a098097 100644
> --- a/src/mesa/state_tracker/st_atom_texture.c
> +++ b/src/mesa/state_tracker/st_atom_texture.c
> @@ -51,21 +51,22 @@
>  #include "util/u_inlines.h"
>  #include "cso_cache/cso_context.h"
>
>
>  /**
>   * Get a pipe_sampler_view object from a texture unit.
>   */
>  void
>  st_update_single_texture(struct st_context *st,
>                           struct pipe_sampler_view **sampler_view,
> -                         GLuint texUnit, bool glsl130_or_later)
> +                         GLuint texUnit, bool glsl130_or_later,
> +                         bool ignore_srgb_decode)
>  {
>     struct gl_context *ctx = st->ctx;
>     const struct gl_sampler_object *samp;
>     struct gl_texture_object *texObj;
>     struct st_texture_object *stObj;
>
>     samp = _mesa_get_samplerobj(ctx, texUnit);
>
>     texObj = ctx->Texture.Unit[texUnit]._Current;
>     assert(texObj);
> @@ -83,55 +84,85 @@ st_update_single_texture(struct st_context *st,
>        *sampler_view = NULL;
>        return;
>     }
>
>     if (texObj->TargetIndex == TEXTURE_EXTERNAL_INDEX &&
>         stObj->pt->screen->resource_changed)
>           stObj->pt->screen->resource_changed(stObj->pt->screen, stObj->pt);
>
>     *sampler_view =
>        st_get_texture_sampler_view_from_stobj(st, stObj, samp,
> -                                             glsl130_or_later);
> +                                             glsl130_or_later,
> +                                             ignore_srgb_decode);
>  }
>
>
>
>  static void
>  update_textures(struct st_context *st,
>                  enum pipe_shader_type shader_stage,
>                  const struct gl_program *prog,
>                  struct pipe_sampler_view **sampler_views,
>                  unsigned *out_num_textures)
>  {
>     const GLuint old_max = *out_num_textures;
>     GLbitfield samplers_used = prog->SamplersUsed;
> +   GLbitfield texel_fetch_samplers = prog->TexelFetchSamplers;
>     GLbitfield free_slots = ~prog->SamplersUsed;
>     GLbitfield external_samplers_used = prog->ExternalSamplersUsed;
>     GLuint unit;
>
>     if (samplers_used == 0x0 && old_max == 0)
>        return;
>
>     unsigned num_textures = 0;
>
>     /* prog->sh.data is NULL if it's ARB_fragment_program */
>     bool glsl130 = (prog->sh.data ? prog->sh.data->Version : 0) >= 130;
>
>     /* loop over sampler units (aka tex image units) */
>     for (unit = 0; samplers_used || unit < old_max;
> -        unit++, samplers_used >>= 1) {
> +        unit++, samplers_used >>= 1, texel_fetch_samplers >>= 1) {
>        struct pipe_sampler_view *sampler_view = NULL;
>
>        if (samplers_used & 1) {
>           const GLuint texUnit = prog->SamplerUnits[unit];
>
> -         st_update_single_texture(st, &sampler_view, texUnit, glsl130);
> +         /* The EXT_texture_sRGB_decode extension says:
> +          *
> +          *    "The conversion of sRGB color space components to linear color
> +          *     space is always performed if the texel lookup function is one
> +          *     of the texelFetch builtin functions.
> +          *
> +          *     Otherwise, if the texel lookup function is one of the texture
> +          *     builtin functions or one of the texture gather functions, the
> +          *     conversion of sRGB color space components to linear color space
> +          *     is controlled by the TEXTURE_SRGB_DECODE_EXT parameter.
> +          *
> +          *     If the TEXTURE_SRGB_DECODE_EXT parameter is DECODE_EXT, the
> +          *     conversion of sRGB color space components to linear color space
> +          *     is performed.
> +          *
> +          *     If the TEXTURE_SRGB_DECODE_EXT parameter is SKIP_DECODE_EXT,
> +          *     the value is returned without decoding. However, if the texture
> +          *     is also [statically] accessed with a texelFetch function, then
> +          *     the result of texture builtin functions and/or texture gather
> +          *     functions may be returned with decoding or without decoding."
> +          *
> +          * Note: the "statically" will be added to the language per
> +          *       https://cvs.khronos.org/bugzilla/show_bug.cgi?id=14934
> +          *
> +          * So we simply ignore the setting entirely for samplers that are
> +          * (statically) accessed with a texelFetch function.
> +          */
> +         st_update_single_texture(st, &sampler_view, texUnit, glsl130,
> +                                  texel_fetch_samplers & 1);
>           num_textures = unit + 1;
>        }
>
>        pipe_sampler_view_reference(&(sampler_views[unit]), sampler_view);
>     }
>
>     /* For any external samplers with multiplaner YUV, stuff the additional
>      * sampler views we need at the end.
>      *
>      * Trying to cache the sampler view in the stObj looks painful, so just
> diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
> index 44e0dd11dcd..6bbfc5f83e0 100644
> --- a/src/mesa/state_tracker/st_cb_texture.c
> +++ b/src/mesa/state_tracker/st_cb_texture.c
> @@ -3110,21 +3110,27 @@ st_NewTextureHandle(struct gl_context *ctx, struct gl_texture_object *texObj,
>     struct st_texture_object *stObj = st_texture_object(texObj);
>     struct pipe_context *pipe = st->pipe;
>     struct pipe_sampler_view *view;
>     struct pipe_sampler_state sampler = {0};
>
>     if (texObj->Target != GL_TEXTURE_BUFFER) {
>        if (!st_finalize_texture(ctx, pipe, texObj, 0))
>           return 0;
>
>        st_convert_sampler(st, texObj, sampObj, 0, &sampler);
> -      view = st_get_texture_sampler_view_from_stobj(st, stObj, sampObj, 0);
> +
> +      /* OpenGL does not have GL_TEXTURE_SRGB_DECODE_EXT, and OpenGL ES does not
> +       * have bindless handles. The two are incompatible (or rather, if bindless
> +       * handles are added to ES, there needs to be an explicit resolution of how
> +       * SRGB_DECODE should be handled).
> +       */
> +      view = st_get_texture_sampler_view_from_stobj(st, stObj, sampObj, 0, true);
>     } else {
>        view = st_get_buffer_sampler_view_from_stobj(st, stObj);
>     }
>
>     return pipe->create_texture_handle(pipe, view, &sampler);
>  }
>
>
>  static void
>  st_DeleteTextureHandle(struct gl_context *ctx, GLuint64 handle)
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index e01268bbbea..4b365c84817 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -4471,20 +4471,24 @@ count_resources(glsl_to_tgsi_visitor *v, gl_program *prog)
>              v->samplers_used |= 1u << idx;
>
>              debug_assert(idx < (int)ARRAY_SIZE(v->sampler_types));
>              v->sampler_types[idx] = inst->tex_type;
>              v->sampler_targets[idx] =
>                 st_translate_texture_target(inst->tex_target, inst->tex_shadow);
>
>              if (inst->tex_shadow) {
>                 prog->ShadowSamplers |= 1 << (inst->resource.index + i);
>              }
> +
> +            if (inst->op == TGSI_OPCODE_TXF || inst->op == TGSI_OPCODE_TXF_LZ) {
> +               prog->TexelFetchSamplers |= 1u << idx;
> +            }
>           }
>        }
>
>        if (inst->tex_target == TEXTURE_EXTERNAL_INDEX)
>           prog->ExternalSamplersUsed |= 1 << inst->resource.index;
>
>        if (inst->resource.file != PROGRAM_UNDEFINED && (
>                  is_resource_instruction(inst->op) ||
>                  inst->op == TGSI_OPCODE_STORE)) {
>           if (inst->resource.file == PROGRAM_MEMORY) {
> diff --git a/src/mesa/state_tracker/st_sampler_view.c b/src/mesa/state_tracker/st_sampler_view.c
> index de104355c04..cd5d7771ba3 100644
> --- a/src/mesa/state_tracker/st_sampler_view.c
> +++ b/src/mesa/state_tracker/st_sampler_view.c
> @@ -386,38 +386,38 @@ last_layer(const struct st_texture_object *stObj)
>     return stObj->pt->array_size - 1;
>  }
>
>
>  /**
>   * Determine the format for the texture sampler view.
>   */
>  static enum pipe_format
>  get_sampler_view_format(struct st_context *st,
>                          const struct st_texture_object *stObj,
> -                        const struct gl_sampler_object *samp)
> +                        bool srgb_skip_decode)
>  {
>     enum pipe_format format;
>
>     GLenum baseFormat = _mesa_base_tex_image(&stObj->base)->_BaseFormat;
>     format = stObj->surface_based ? stObj->surface_format : stObj->pt->format;
>
>     if (baseFormat == GL_DEPTH_COMPONENT ||
>         baseFormat == GL_DEPTH_STENCIL ||
>         baseFormat == GL_STENCIL_INDEX) {
>        if (stObj->base.StencilSampling || baseFormat == GL_STENCIL_INDEX)
>           format = util_format_stencil_only(format);
>
>        return format;
>     }
>
>     /* If sRGB decoding is off, use the linear format */
> -   if (samp->sRGBDecode == GL_SKIP_DECODE_EXT)
> +   if (srgb_skip_decode)
>        format = util_format_linear(format);
>
>     /* Use R8_UNORM for video formats */
>     switch (format) {
>     case PIPE_FORMAT_NV12:
>     case PIPE_FORMAT_IYUV:
>        format = PIPE_FORMAT_R8_UNORM;
>        break;
>     default:
>        break;
> @@ -456,60 +456,65 @@ st_create_texture_sampler_view_from_stobj(struct st_context *st,
>     templ.swizzle_a = GET_SWZ(swizzle, 3);
>
>     return st->pipe->create_sampler_view(st->pipe, stObj->pt, &templ);
>  }
>
>
>  struct pipe_sampler_view *
>  st_get_texture_sampler_view_from_stobj(struct st_context *st,
>                                         struct st_texture_object *stObj,
>                                         const struct gl_sampler_object *samp,
> -                                       bool glsl130_or_later)
> +                                       bool glsl130_or_later,
> +                                       bool ignore_srgb_decode)
>  {
>     struct st_sampler_view *sv;
>     struct pipe_sampler_view *view;
> +   bool srgb_skip_decode = false;
>
>     mtx_lock(&stObj->validate_mutex);
>     sv = st_texture_get_sampler_view(st, stObj);
>     if (!sv) {
>        mtx_unlock(&stObj->validate_mutex);
>        return NULL;
>     }
>
>     view = sv->view;
>
> +   if (!ignore_srgb_decode && samp->sRGBDecode == GL_SKIP_DECODE_EXT)
> +      srgb_skip_decode = true;
> +
>     if (view &&
>         sv->glsl130_or_later == glsl130_or_later &&
> -       sv->sRGBDecode == samp->sRGBDecode) {
> +       sv->srgb_skip_decode == srgb_skip_decode) {
>        /* Debug check: make sure that the sampler view's parameters are
>         * what they're supposed to be.
>         */
>        MAYBE_UNUSED struct pipe_sampler_view *view = sv->view;
>        assert(stObj->pt == view->texture);
>        assert(!check_sampler_swizzle(st, stObj, view, glsl130_or_later));
> -      assert(get_sampler_view_format(st, stObj, samp) == view->format);
> +      assert(get_sampler_view_format(st, stObj, srgb_skip_decode) == view->format);
>        assert(gl_target_to_pipe(stObj->base.Target) == view->target);
>        assert(stObj->base.MinLevel + stObj->base.BaseLevel ==
>               view->u.tex.first_level);
>        assert(last_level(stObj) == view->u.tex.last_level);
>        assert(stObj->layer_override || stObj->base.MinLayer == view->u.tex.first_layer);
>        assert(stObj->layer_override || last_layer(stObj) == view->u.tex.last_layer);
>        assert(!stObj->layer_override ||
>               (stObj->layer_override == view->u.tex.first_layer &&
>                stObj->layer_override == view->u.tex.last_layer));
>     }
>     else {
>        /* create new sampler view */
> -      enum pipe_format format = get_sampler_view_format(st, stObj, samp);
> +      enum pipe_format format = get_sampler_view_format(st, stObj, srgb_skip_decode);
>
>        sv->glsl130_or_later = glsl130_or_later;
> -      sv->sRGBDecode = samp->sRGBDecode;
> +      sv->srgb_skip_decode = srgb_skip_decode;
>
>        pipe_sampler_view_release(st->pipe, &sv->view);
>        view = sv->view =
>           st_create_texture_sampler_view_from_stobj(st, stObj, format, glsl130_or_later);
>     }
>
>     mtx_unlock(&stObj->validate_mutex);
>
>     return view;
>  }
> diff --git a/src/mesa/state_tracker/st_sampler_view.h b/src/mesa/state_tracker/st_sampler_view.h
> index 9a7e54cff4e..e48c4b9015f 100644
> --- a/src/mesa/state_tracker/st_sampler_view.h
> +++ b/src/mesa/state_tracker/st_sampler_view.h
> @@ -73,17 +73,18 @@ void
>  st_texture_free_sampler_views(struct st_texture_object *stObj);
>
>  struct pipe_sampler_view *
>  st_texture_get_current_sampler_view(const struct st_context *st,
>                                      const struct st_texture_object *stObj);
>
>  struct pipe_sampler_view *
>  st_get_texture_sampler_view_from_stobj(struct st_context *st,
>                                         struct st_texture_object *stObj,
>                                         const struct gl_sampler_object *samp,
> -                                       bool glsl130_or_later);
> +                                       bool glsl130_or_later,
> +                                       bool ignore_srgb_decode);
>
>  struct pipe_sampler_view *
>  st_get_buffer_sampler_view_from_stobj(struct st_context *st,
>                                        struct st_texture_object *stObj);
>
>  #endif /* ST_SAMPLER_VIEW_H */
> diff --git a/src/mesa/state_tracker/st_texture.c b/src/mesa/state_tracker/st_texture.c
> index f749fb0a9fc..3c0dfa678d9 100644
> --- a/src/mesa/state_tracker/st_texture.c
> +++ b/src/mesa/state_tracker/st_texture.c
> @@ -508,21 +508,26 @@ st_destroy_bound_image_handles(struct st_context *st)
>   * Create a texture handle from a texture unit.
>   */
>  static GLuint64
>  st_create_texture_handle_from_unit(struct st_context *st,
>                                     struct gl_program *prog, GLuint texUnit)
>  {
>     struct pipe_context *pipe = st->pipe;
>     struct pipe_sampler_view *view;
>     struct pipe_sampler_state sampler = {0};
>
> -   st_update_single_texture(st, &view, texUnit, prog->sh.data->Version >= 130);
> +   /* OpenGL does not have GL_TEXTURE_SRGB_DECODE_EXT, and OpenGL ES does not
> +    * have bindless sampler uniforms. If the two were ever enabled
> +    * simultaneously, we could mimick the behavior of non-bindless texture
> +    * units here.
> +    */
> +   st_update_single_texture(st, &view, texUnit, prog->sh.data->Version >= 130, true);
>     if (!view)
>        return 0;
>
>     if (view->target != PIPE_BUFFER)
>        st_convert_sampler_from_unit(st, &sampler, texUnit);
>
>     assert(st->ctx->Texture.Unit[texUnit]._Current);
>
>     return pipe->create_texture_handle(pipe, view, &sampler);
>  }
> diff --git a/src/mesa/state_tracker/st_texture.h b/src/mesa/state_tracker/st_texture.h
> index df5d6dda387..f2fefee63c5 100644
> --- a/src/mesa/state_tracker/st_texture.h
> +++ b/src/mesa/state_tracker/st_texture.h
> @@ -49,22 +49,22 @@ struct st_texture_image_transfer {
>
>
>  /**
>   * Container for one context's validated sampler view.
>   */
>  struct st_sampler_view {
>     struct pipe_sampler_view *view;
>
>     /** The glsl version of the shader seen during validation */
>     bool glsl130_or_later;
> -   /** The value of the sampler's sRGBDecode state during validation */
> -   GLenum sRGBDecode;
> +   /** Derived from the sampler's sRGBDecode state during validation */
> +   bool srgb_skip_decode;
>  };
>
>
>  /**
>   * Subclass of gl_texure_image.
>   */
>  struct st_texture_image
>  {
>     struct gl_texture_image base;
>
> @@ -296,21 +296,22 @@ st_convert_sampler(const struct st_context *st,
>                     struct pipe_sampler_state *sampler);
>
>  void
>  st_convert_sampler_from_unit(const struct st_context *st,
>                               struct pipe_sampler_state *sampler,
>                               GLuint texUnit);
>
>  void
>  st_update_single_texture(struct st_context *st,
>                           struct pipe_sampler_view **sampler_view,
> -                         GLuint texUnit, bool glsl130_or_later);
> +                         GLuint texUnit, bool glsl130_or_later,
> +                         bool ignore_srgb_decode);
>
>  void
>  st_make_bound_samplers_resident(struct st_context *st,
>                                  struct gl_program *prog);
>
>  void
>  st_make_bound_images_resident(struct st_context *st,
>                                struct gl_program *prog);
>
>  #endif
> --
> 2.11.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list