2017-04-08 18:37 GMT+02:00 Marek Olšák <mar...@gmail.com>: > On Sat, Apr 8, 2017 at 12:53 AM, Gustaw Smolarczyk <wielkie...@gmail.com> > wrote: >> 2017-04-07 23:56 GMT+02:00 Marek Olšák <mar...@gmail.com>: >>> On Fri, Apr 7, 2017 at 6:54 PM, Gustaw Smolarczyk <wielkie...@gmail.com> >>> wrote: >>>> 2017-04-07 16:31 GMT+02:00 Marek Olšák <mar...@gmail.com>: >>>>> On Thu, Mar 30, 2017 at 8:09 PM, Gustaw Smolarczyk <wielkie...@gmail.com> >>>>> wrote: >>>>>> Signed-off-by: Gustaw Smolarczyk <wielkie...@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.
Yes, I understand how it's important not to modify some state before the driver (or vbo module or st/mesa) is done using it for things that were being buffered (like the vertices). Since currently _PackedMode is not used there, it shouldn't be a problem. And I think it's always better to use _PackedEnabledMode instead of _PackedMode since it also tells you whether the whole fog processing is enabled or not. Maybe I should put a comment saying that _PackedMode is for internal use and shouldn't be used in any other way than helping _PackedEnabledMode initialization? > >> >> 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. I am not sure what you mean by "git link". Did I forget about something while using git-format-patch/git-send-email before sending the series? Or do you want a git repository you can pull the series from? Gustaw _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev