[Mesa-dev] [PATCH 1/2] st/glsl_to_tgsi: ignore GL_TEXTURE_SRGB_DECODE_EXT for samplers used with texelFetch*()
Nicolai Hähnle
nhaehnle at gmail.com
Wed Oct 11 21:27:53 UTC 2017
On 10.10.2017 19:31, Marek Olšák wrote:
> 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.
Huh, you're right. No idea where I got that idea from. I've changed the
comment to note a TODO about having to clarify the interaction between
SRGB_DECODE and bindless.
Cheers,
Nicolai
>
> 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
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list