The original code looks very efficient, especially for inlining.
Anyone have any idea if it actually does what it is supposed to do by
accident or by design?
On 11/10/2010 3:47 AM, Ponzu wrote:
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 <zab...@gmail.com
<mailto: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
<mailto: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
--
Tateru Nino
http://dwellonit.taterunino.net/
_______________________________________________
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
