[opensource-dev] Fix for "Attachments displayed in mouselook" bug

Ponzu lee.ponzu at gmail.com
Sun Oct 10 09:47:23 PDT 2010


This code below means it used to be inline, but now is a function call
(unless the compiler is optimizing or something).  You guys know the code
waaaay better tahn I, but is it worth moving it back to the .h file?

return type && mRenderTypeEnabled[type]

looks hard to beat for inlining, and it is probably called a zillion times.



On Sat, Oct 9, 2010 at 2:59 PM, Zabb65 <zabb65 at 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.argent at 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20101010/678552a0/attachment.htm 


More information about the opensource-dev mailing list