Well, this is pretty common idiom for code that deals with bit masks.
 Funny.  The problem is that the bitmask (hard to understand?) was replaced
with a simple array (easy to understand? and that is when the error was
introduced.


On Sat, Oct 9, 2010 at 2:59 PM, Zabb65 <zab...@gmail.com> wrote:

> I tend to agree with Argent on this, it should have a comment if there
> is a hack in place to make it all function.
>
> I ran it through hg blame and it claims that revision 11977 was when a
> big block in pipeline.cpp changed
>
> 11977: BOOL LLPipeline::hasRenderType(const U32 type) const
> 11977: {
> 11977:  return mRenderTypeEnabled[type];
> 11977: }
>
> previously this function was in a header and looked like
>
> BOOL hasRenderType(const U32 type) const                                {
> return (type &&
> (mRenderTypeMask & (1<<type))) ? TRUE : FALSE; }
>
> Subtle isn't it?
>
> http://hg.secondlife.com/viewer-development/changeset/c09f9bcd9d20
> however does not show this, and I suspect that it is heavily truncated
> on bitbucket, even though the full patch can be pulled from the hg
> repository itself.
>
> So, this change is good, I support it, but make the hack for
> attachment hiding obvious so that it doesn't have more creep later on.
> Comments for hacks like this are a good thing. Leaving them only in
> commit messages leaves them to be lost and forgotten and then broken
> by whoever browses the code.
>
> On Sat, Oct 9, 2010 at 12:19, Argent <secret.arg...@gmail.com> wrote:
> > I don't normally gripe about stuff like this, but somehow this one
> > triggered my twitches from 20 years of supporting PhD programmers.
> > Brilliant guys, but sometimes it's SO hard to figure out what they're
> > trying to do.
> >
> > This is a case where the trinary "?:" operator is much more readable
> > and understandable.
> >
> > (type == 0) ? FALSE : mRenderTypeEnabled[type];
> >
> > But even better:
> >
> > if(type == 0)
> >  return FALSE; // explain why here .. eg "in this context we are
> > always rendering attached prims on the head when blah blah..."
> > else
> >  return mRenderTypeEnabled[type];
> > _______________________________________________
> > Policies and (un)subscribe information available here:
> > http://wiki.secondlife.com/wiki/OpenSource-Dev
> > Please read the policies before posting to keep unmoderated posting
> privileges
> >
> _______________________________________________
> Policies and (un)subscribe information available here:
> http://wiki.secondlife.com/wiki/OpenSource-Dev
> Please read the policies before posting to keep unmoderated posting
> privileges
>
_______________________________________________
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Reply via email to