On Sat, Apr 8, 2017 at 12:53 AM, Gustaw Smolarczyk <wielkiegie at gmail.com> wrote:
> 2017-04-07 23:56 GMT+02:00 Marek Olšák <maraeo at gmail.com>:
>> On Fri, Apr 7, 2017 at 6:54 PM, Gustaw Smolarczyk <wielkiegie at gmail.com> wrote:
>>> 2017-04-07 16:31 GMT+02:00 Marek Olšák <maraeo at gmail.com>:
>>>> On Thu, Mar 30, 2017 at 8:09 PM, Gustaw Smolarczyk <wielkiegie at gmail.com> wrote:
>>>>> Signed-off-by: Gustaw Smolarczyk <wielkiegie at gmail.com>
>>>>> ---
>>>>> src/mesa/main/enable.c | 1 +
>>>>> src/mesa/main/fog.c | 9 +++++++++
>>>>> src/mesa/main/mtypes.h | 14 ++++++++++++++
>>>>> 3 files changed, 24 insertions(+)
>>>>>
>>>>> diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c
>>>>> index d9d63a6b4b..ef278a318a 100644
>>>>> --- a/src/mesa/main/enable.c
>>>>> +++ b/src/mesa/main/enable.c
>>>>> @@ -385,6 +385,7 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, GLboolean state)
>>>>> return;
>>>>> FLUSH_VERTICES(ctx, _NEW_FOG);
>>>>> ctx->Fog.Enabled = state;
>>>>> + ctx->Fog._PackedEnabledMode = state ? ctx->Fog._PackedMode : FOG_NONE;
>>>>> break;
>>>>> case GL_LIGHT0:
>>>>> case GL_LIGHT1:
>>>>> diff --git a/src/mesa/main/fog.c b/src/mesa/main/fog.c
>>>>> index 1ad939cfde..76e65080b7 100644
>>>>> --- a/src/mesa/main/fog.c
>>>>> +++ b/src/mesa/main/fog.c
>>>>> @@ -102,8 +102,13 @@ _mesa_Fogfv( GLenum pname, const GLfloat *params )
>>>>> m = (GLenum) (GLint) *params;
>>>>> switch (m) {
>>>>> case GL_LINEAR:
>>>>> + ctx->Fog._PackedMode = FOG_LINEAR;
>>>>> + break;
>>>>> case GL_EXP:
>>>>> + ctx->Fog._PackedMode = FOG_EXP;
>>>>> + break;
>>>>> case GL_EXP2:
>>>>> + ctx->Fog._PackedMode = FOG_EXP2;
>>>>
>>>> Perhaps these should be set before FLUSH_VERTICES?
>>>>
>>>> Marek
>>>
>>> That would make us need two switch() statements instead of one. Also,
>>> if ctx->Fog.Mode == m then we are essentially writing the same values
>>> so nothing changes - doing it after the check shouldn't affect
>>> correctness in any way. I might be wrong, though. _PackedMode is only
>>> ever used to manage _PackedEnabledMode (here and in _mesa_set_enable),
>>> so I think we should be safe.
>>>
>>> Since this simplification is pretty minor, I can change the code if you want.
>>
>> It's OK if _PackedMode is only a temporary variable used after
>> FLUSH_VERTICES. It can't be used directly in places that don't call
>> FLUSH_VERTICES though (e.g. ff_fragment_program.cpp).
>
> Since current _PackedMode usage is restricted to mesa/main just after
> FLUSH_VERTICES is called (and will probably stay this way), I think
> the prerequisites are fulfilled.
The prerequisites are fulfilled only if _PackedMode isn't used by
FLUSH_VERTICES itself.
Mesa doesn't send all draw calls to drivers. Some draw calls are
queued in the vbo module. FLUSH_VERTICES flushes the queue. All state
changes should happen after FLUSH_VERTICES. If they happen before
FLUSH_VERTICES, they could affect previous draw calls. It's basically
the same as changing the order of GL calls in an incorrect way. We
should make sure that it never happens.
>
> If you are ok with the patches, could please push the whole series? I
> don't have commit access.
If you send me a git link to the whole series, I'll push it.
Marek