[Mesa-dev] [PATCH 3/7] mesa: correctly manage GroupStackDepth

[Timothy Arceri]

On Mon, 2015-11-23 at 00:20 +0000, Emil Velikov wrote:
> On 22 November 2015 at 23:53, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> > On Sun, Nov 22, 2015 at 6:50 PM, Emil Velikov <emil.l.velikov at gmail.com>
> > wrote:
> > > On 22 November 2015 at 23:22, Ilia Mirkin <imirkin at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev