cube.cpp, code cleanup:
#include "privates.h", not <privates.h>.
Merged if condition checks.
Declaration of variables, where you need them to improve
the code structure and readability.
Use C++ standard bool for topDir, bottomDir, allCaps and
mCapsPainted[output] (bool true/false instead Bool TRUE/FALSE).
Initialize all of the class member variables in the constructor.
Replaced if (priv->mSky.size () > 0) with if (!priv->mSky.empty ()).
Removed redundant brackets.
Fixed indentation.
Added TODO.
cube.cpp, code speedup:
Do not call optionGetInactiveOpacity () twice, instead save
the value as float inactiveOpacity and use this variable in
the following calculation.
Introduced the variables float halfHsize = hsize / 2.0; and
float tsSize = 360.0f / size; and used those in following
calculations to not have to recalculate those values multiple
times.
Do not calculate M_PI / sides and 2 * M_PI / sides multiple times
in loops, instead save those values in the GLfloats mps and tmps
and use those values inside the loops.
Use GLdouble mDist2 = 0.5 / mDistance; in the GLdouble clipPlane*
arrays, instead of recalculating this value multiple times.
Use GLfloat oneMinusFStepX = 1.0f - fStepX; and
GLfloat oneMinusFStepY = 1.0f - fStepY; instead of calculating
those values multiple times.
Use float outputWidth = outputPtr->width (); and float outputHeight =
outputPtr->height (); instead of calling the functions multiple
times.
Introduced the GLfloat normInvert = 0.5f * invert; and used this
variable instead of recalculating this value multiple times.
Introduced the GLfloats mSkyWidth and mSkyHeight and used those
in the following loops instead of calling mSkySize.width () and
mSkySize.height () multiple times.
Try to avoid redundant GL_BLEND state changes, because OpenGL
will blindly change the global state, no matter what it currently
is set to and that state change is expensive. So we query and
save the actual blending state in the variable GLboolean
glBlendEnabled = glIsEnabled (GL_BLEND); and just enable it if
it isn't already and just disable it, if it was disabled before.
(LP: #1101422, LP: #1195977)
cube.cpp, other fixes:
Return false in function PrivateCubeScreen::updateGeometry (int, int)
if sides should ever be 0, which should never happen, but should make
Coverity happy.
(LP: #1101541)
Initialize all class member variables in the PrivateCubeScreen::
PrivateCubeScreen (CompScreen *) constructor.
(mTc, mNOutput, mOutput, mOutputMask, mCleared, mCapsPainted)
(LP: #1101575)
149 - memset (mCleared, 0, sizeof (Bool) * screen->outputDevs ().size ());
150 + memset (mCleared, 0, sizeof (Bool) * screen->outputDevs ().size ());
283 - if (xMoveAdd < -hsize / 2)
284 + if (xMoveAdd < -halfHsize)
285 xMoveAdd += hsize;
473 GLScreenPaintAttrib sa = sAttrib;
474 - sa.xRotate += xRotate;
475 - sa.vRotate += vRotate;
476 + sa.xRotate += xRotate;
477 + sa.vRotate += vRotate;
While whitespace alignment can sometimes be helpful, use it judicially. Typically, it makes a lot of sense in mutliple-assignment type scenarios. Otherwise it can become distracting. I've picked out some examples where I think such alignment hurts more than it helps.
490 - /* distance we move the camera back when unfolding the cube.
491 + /* TODO: Distance we move the camera back when unfolding the cube.
492 currently hardcoded to 1.5 but it should probably be optional. */
The relevant todo is the fact that it could be optional, not the distance the cube is being moved.
The other changes are mostly fine for now.
Something to keep in mind: The linked bug report about the redundant state changes goes more towards the fundamental architecture of compiz rather than something that can be fixed on a case by case basis. Currently the only mechanism that we have in order to avoid such calls into OpenGL is effectively more calls into OpenGL by querying the state, which in some cases can actually be worse than the state change because it requires looking up the current state of the pipeline as it might have been modified internally.
In this case, it doesn't really matter too much - we check to see if a particular state was enabled or disabled before doing both (eg, if enabled, leave it enabled and don't disable it later, if not enabled, disable it later).