[Mesa-dev] [PATCH] mesa: fix texture updates for ATI_fragment_shader

[Timothy Arceri]

On 16/10/17 04:10, Miklós Máté wrote:
> Hi,
> 
> I'd like to ask you to revert this change.
> 
> As Ian Romanick pointed out this makes ATI_fs behave like ARB_fp, 
> however there is a major difference between the two: with ATI_fs there 
> is no way of knowing the texture targets until the draw call. When an 
> ATI_fs is created, st_init_atifs_prog() sets every texture target to 
> TEXTURE_2D_BIT, and st_fixup_atifs() sets the correct one, but 
> unfortunately _mesa_update_texture_state() is called between them. After 
> this patch update_program_texture_state() validates the texture targets 
> to match the bound texture units, and thus rejects sampling from cube 
> maps. This results in broken rendering in Knights of the Old Republic.

Before reverting anything are you able to create a piglit test that 
reproduces the issue, so that this doesn't get broken again in future?

Ideally we would have one for the crash Marek is seeing also.


> 
> MM
> 
> On 27/09/17 17:39, Marek Olšák wrote:
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> ---
>>   src/mesa/main/texstate.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
>> index 269e291..edd2253 100644
>> --- a/src/mesa/main/texstate.c
>> +++ b/src/mesa/main/texstate.c
>> @@ -833,23 +833,25 @@ update_ff_texture_state(struct gl_context *ctx,
>>   void
>>   _mesa_update_texture_state(struct gl_context *ctx)
>>   {
>>      struct gl_program *prog[MESA_SHADER_STAGES];
>>      int i;
>>      int old_max_unit = ctx->Texture._MaxEnabledTexImageUnit;
>>      BITSET_DECLARE(enabled_texture_units, 
>> MAX_COMBINED_TEXTURE_IMAGE_UNITS);
>>      memcpy(prog, ctx->_Shader->CurrentProgram, sizeof(prog));
>> -   if (prog[MESA_SHADER_FRAGMENT] == NULL &&
>> -       _mesa_arb_fragment_program_enabled(ctx)) {
>> -      prog[MESA_SHADER_FRAGMENT] = ctx->FragmentProgram.Current;
>> +   if (prog[MESA_SHADER_FRAGMENT] == NULL) {
>> +      if (_mesa_arb_fragment_program_enabled(ctx))
>> +         prog[MESA_SHADER_FRAGMENT] = ctx->FragmentProgram.Current;
>> +      else if (_mesa_ati_fragment_shader_enabled(ctx))
>> +         prog[MESA_SHADER_FRAGMENT] = 
>> ctx->ATIFragmentShader.Current->Program;
>>      }
>>      /* TODO: only set this if there are actual changes */
>>      ctx->NewState |= _NEW_TEXTURE_OBJECT | _NEW_TEXTURE_STATE;
>>      ctx->Texture._GenFlags = 0x0;
>>      ctx->Texture._TexMatEnabled = 0x0;
>>      ctx->Texture._TexGenEnabled = 0x0;
>>      ctx->Texture._MaxEnabledTexImageUnit = -1;
>>      ctx->Texture._EnabledCoordUnits = 0x0;
> 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev