[Mesa-dev] [PATCH] mesa: Save and restore NV_fog_distance & FOG_COORD_SRC state

[Nicholas Miell]

On 06/25/2018 12:04 PM, Ian Romanick wrote:
> On 06/25/2018 11:53 AM, Nicholas Miell wrote:
>> FOG_DISTANCE_MODE_NV & FOG_COORD_SRC weren't getting saved
>> into display lists or restored on glPopAttrib(GL_FOG_BIT).
> 
> Good catch.  How did you detect this?  Do we have any test cases?
> 

I honestly don't remember, I've been sitting on this patch for years and
only dusted it off and updated it now that that there's an actual effort
to implement the 4.6 compat profile.

I think at some point I realized that my initial implementation of
NV_fog_distance was incomplete and at the same time I noticed that the
existing EXT_fog_coord implementation was broken in the same way.

Writing extension specs as a change list against the OpenGL spec is a
terrible way to convey information. The extensions adds entries to
tables, but don't mention that e.g. the table in question is the
comprehensive listing of all fog state or that elsewhere in the OpenGL
spec that table is referenced in the discussion of what must be saved by
PushAttrib.

>>
>> Signed-off-by: Nicholas Miell <nmiell at gmail.com>
>> ---
>>  src/mesa/main/attrib.c | 4 ++++
>>  src/mesa/main/dlist.c  | 2 ++
>>  2 files changed, 6 insertions(+)
>>
>> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
>> index cbe93ab6faa..29d7089989e 100644
>> --- a/src/mesa/main/attrib.c
>> +++ b/src/mesa/main/attrib.c
>> @@ -1177,6 +1177,10 @@ _mesa_PopAttrib(void)
>>                 _mesa_Fogf(GL_FOG_END, fog->End);
>>                 _mesa_Fogf(GL_FOG_INDEX, fog->Index);
>>                 _mesa_Fogi(GL_FOG_MODE, fog->Mode);
>> +               _mesa_Fogi(GL_FOG_COORD_SRC, fog->FogCoordinateSource);
>> +
>> +               if (ctx->Extensions.NV_fog_distance)
>> +                 _mesa_Fogi(GL_FOG_DISTANCE_MODE_NV, fog->FogDistanceMode);
> 
> Presumably _mesa_PushAttrib already does the right thing?
> 

PushAttrib is just a memcpy of the gl_fog_attrib struct, so the
individual elements always get saved. Its only in PopAttrib where the
driver needs to be informed.

>>              }
>>              break;
>>           case GL_HINT_BIT:
>> diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c
>> index 4fc451000b5..8428791f9e8 100644
>> --- a/src/mesa/main/dlist.c
>> +++ b/src/mesa/main/dlist.c
>> @@ -2611,6 +2611,8 @@ save_Fogiv(GLenum pname, const GLint *params)
>>     case GL_FOG_START:
>>     case GL_FOG_END:
>>     case GL_FOG_INDEX:
>> +   case GL_FOG_COORD_SRC:
>> +   case GL_FOG_DISTANCE_MODE_NV:
>>        p[0] = (GLfloat) *params;
>>        p[1] = 0.0f;
>>        p[2] = 0.0f;
>>
>