On Mon, 2015-11-23 at 00:20 +0000, Emil Velikov wrote: > On 22 November 2015 at 23:53, Ilia Mirkin <imir...@alum.mit.edu> wrote: > > On Sun, Nov 22, 2015 at 6:50 PM, Emil Velikov <emil.l.veli...@gmail.com> > > wrote: > > > On 22 November 2015 at 23:22, Ilia Mirkin <imir...@alum.mit.edu> wrote: > > > > There are some places that you're not fixing up... > > > I thought I fixed all cases - which ones do you have in mind ? The > > > ones in debug_destroy() or the comparison in _mesa_PushDebugGroup() ? > > > > There was a check for <= 0 which would now have to be <= 1 I guess? > Did not see this one. Thanks. > > > And something else, I already forgot. > > > Which is ? > > > > > > > > would this > > > > alternatively be resolved by returning GroupStackDepth+1 in > > > > _mesa_get_debug_state_int and leaving everything else alone? > > > > > > > True and it will result in a shorter patch. Then again the name would > > > be wrong :-\ > > > > So rename it to CurrentGroup or something. Having to constantly do n-1 > > everywhere increases the likelihood of bugs/misunderstanding down the > > line. > > > I'm inclined to go with "keep things as defined in the spec" be that > names or definitions (see length in one of the later patches). > Although if people feel so much against that so be it (i.e. looking > for a third person to cast their view).
I have to agree with Ilia, I think its better to have the code easy to understand rather than match spec names/definitions is this case. You can always add a comment to explain what the variable is. > > Thanks > Emil > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
