Code review comment for lp:~smspillaz/compiz/compiz..merge-fix-1075578-workspacenames-flickering-during-display.damage_correctly

I'll explain again what's going on here:

8 +namespace
9 +{
10 + const float TEXT_BORDER = 10.0f;
11 +}
12 +

I pulled "border" out of drawText and made it a constant. I figure it must be for the damage border around the text itself.

23 +CompPoint
24 +WSNamesScreen::getTextPlacementPosition ()
...

All I've done here is pull out the code which determines where the text actually goes on screen into a separate function. We can use it again later in both the paint and damage code, eg:

62 +void
63 +WSNamesScreen::damageTextArea ()
64 +{
65 + const CompPoint pos (getTextPlacementPosition ());
66 +

and

77 +void
78 +WSNamesScreen::drawText (const GLMatrix &matrix)
79 +{
80 + GLfloat alpha = 0.0f;
81 +
82 + /* assign y (for the lower corner!) according to the setting */
83 + const CompPoint p = getTextPlacementPosition ();

In the damage function here:

62 +void
63 +WSNamesScreen::damageTextArea ()
64 +{
65 + const CompPoint pos (getTextPlacementPosition ());
66 +
67 + /* The placement position is from the lower corner, so we
68 + * need to move it back up by height */
69 + CompRect area (pos.x () - TEXT_BORDER,
70 + pos.y () - TEXT_BORDER - textData.getHeight () ,
71 + textData.getWidth () + TEXT_BORDER * 2,
72 + textData.getHeight () + TEXT_BORDER * 2);
73 +
74 + cScreen->damageRegion (area);
75 +}

The only gotcha was that the workspacenames plugin is a little unconventional and gives the co-ords for the text area relative to its bottom-left corner as opposed to its top-left corner. So we have to subtract the height of the text area in order to get the top-left corner to pass it to damageRegion.

127 + /* Only damage when the */
128 + if (shouldDrawText ())
129 + damageTextArea ();
130 +
131 + cScreen->donePaint ();
132
133 - cScreen->donePaint ();
134 + /* Clear text data if done with fadeout */
135 + if (!timer && !timeoutHandle.active ())
136 + textData.clear ();

We don't want to be calling damageScreen or damageRegion on every frame. Its bad for power usage and performance. So, we only call it if we should be drawing text to the screen on this frame.

I also moved the call to textData.clear () from:

115 -
116 - if (!timer)
117 - textData.clear ();

to:

135 + if (!timer && !timeoutHandle.active ())
136 + textData.clear ();

In donePaint. I don't know why it was like that in this plugin, but the best practice for this is to always clean up paint-related data after animations have completed in donePaint, so that you paint at least one frame at the end as though there was no text object.