Merge lp:~mc-return/compiz/compiz.merge-fix1099100-thumbnail-title-text-issues.0 into lp:compiz/0.9.9

[MC Return]

Thumbnail plugin, new features:

Added a rounded background to the window title text.

Made the distance between text and thumbnail configurable.

Window-like-background or glow are now rendered around
the thumbnail live preview only, excluding the text.
This ensures no transparent hole is generated anymore and
fixes the look of the window title text rendering, while
enhancing the possibility for customization.

Code cleanup:

Return ASAP if the text plugin is not loaded.
No need to check if (t->text) twice.
Improved height calculation.

thumbnail.xml.in changes:

Added options to adjust the text distance (pixel between
thumbnail and window title text) and font background color
and opacity.

Made the window title text font white and fully opaque
to create a sane default, otherwise it would not be
visible on a transparent black background (alpha and color
are the same like the default background/glow color).

(LP: #1099100)

As requested no .xml.in cleanup this time.
A second MP will follow soon.

Note:
These will not be the final fixes for thumbnail as
there are still a few issues unsolved...

1. This plugin (like several others, btw.) requires
the text plugin to be activated to fully work, but
the user is not informed about that fact, see:
https://code.launchpad.net/~mc-return/compiz/compiz.merge-fix1099100-thumbnail-title-text-issues/+merge/143042/comments/310279
Ideally bug 1101198 should be fixed first to perfectly
solve this issue, but if you think this feature is
not necessary, we should add text requirements to those
plugins.

2. While working smoothly with gallium-radeon, there
is still flickering of the thumbnail texture itself
when using fglrx. I have not tested other gfx/driver
combinations but expect there might be similar issues.

3. This plugin won't play together nicely with multiple
docks/taskbars/launchers as it will currently just display
one thumbnail live window preview per icon.

[lp:~mc-return/compiz/compiz.merge-fix1099100-thumbnail-title-text-issues.0]

3586. By MC Return on 2013-01-25

Fixed indentation

[Sam Spilsbury (smspillaz)]

This looks fine code wise.

I'm a bit concerned as to how things will look once we start requiring that the text background be rendered in addition to the text. My understanding was that we were meant to render the thumbnail background (window) and then render the text on top of that. What was the reason that we couldn't do that? If it was that there was really a "hole" where the text was, then this sounds like a more fundamental problem with the rendering of the thumbnail background itself, and we shouldn't cover it up by rendering a text background on top of it.

Could you post a screenshot to see how it looks?

[Sam Spilsbury (smspillaz)]

review: Needs Information

[MC Return (mc-return)]

> This looks fine code wise.
>
> I'm a bit concerned as to how things will look once we start requiring that
> the text background be rendered in addition to the text. My understanding was
> that we were meant to render the thumbnail background (window) and then render
> the text on top of that. What was the reason that we couldn't do that? If it
> was that there was really a "hole" where the text was, then this sounds like a
> more fundamental problem with the rendering of the thumbnail background
> itself, and we shouldn't cover it up by rendering a text background on top of
> it.

Yes, it was a rendering problem initially. Most probably it could have been
solved differently also, by simply filling the hole for example.
I chose this solution, because it also fixes the problem and brings some
advancements and enhanced customization possibility, while hiding less of
the background and looking better at the same time.
This is kind of a solved problem combined with an upgrade and refined look.

>
> Could you post a screenshot to see how it looks?

This is how default looks now: http://imagebin.org/244365
Of course it can look very different as well, as you can configure everything
to your liking.

review: Needs Resubmitting

[Sam Spilsbury (smspillaz)]

Okay, I think its a nice and new take. What do you think Daniel?
On 27/01/2013 10:30 AM, "MC Return" <email address hidden> wrote:

> Review: Resubmit
>
> > This looks fine code wise.
> >
> > I'm a bit concerned as to how things will look once we start requiring
> that
> > the text background be rendered in addition to the text. My
> understanding was
> > that we were meant to render the thumbnail background (window) and then
> render
> > the text on top of that. What was the reason that we couldn't do that?
> If it
> > was that there was really a "hole" where the text was, then this sounds
> like a
> > more fundamental problem with the rendering of the thumbnail background
> > itself, and we shouldn't cover it up by rendering a text background on
> top of
> > it.
>
> Yes, it was a rendering problem initially. Most probably it could have been
> solved differently also, by simply filling the hole for example.
> I chose this solution, because it also fixes the problem and brings some
> advancements and enhanced customization possibility, while hiding less of
> the background and looking better at the same time.
> This is kind of a solved problem combined with an upgrade and refined look.
>
> >
> > Could you post a screenshot to see how it looks?
>
> This is how default looks now: http://imagebin.org/244365
> Of course it can look very different as well, as you can configure
> everything
> to your liking.
> --
>
> https://code.launchpad.net/~mc-return/compiz/compiz.merge-fix1099100-thumbnail-title-text-issues.0/+merge/144954
> You are reviewing the proposed merge of
> lp:~mc-return/compiz/compiz.merge-fix1099100-thumbnail-title-text-issues.0
> into lp:compiz.
>

[Sam Spilsbury (smspillaz)]

I'm fine with this as is - awaiting feedback from another reviewer.

review: Approve

[lp:~mc-return/compiz/compiz.merge-fix1099100-thumbnail-title-text-issues.0]

3587. By MC Return on 2013-01-31

Merged latest lp:compiz

[Brandon Schaefer (brandontschaefer)]

LGTM, works.

review: Approve

=== modified file 'plugins/thumbnail/src/thumbnail.cpp'
--- plugins/thumbnail/src/thumbnail.cpp	2013-01-11 23:32:06 +0000
+++ plugins/thumbnail/src/thumbnail.cpp	2013-01-31 19:13:21 +0000
@@ -32,8 +32,6 @@
 
 COMPIZ_PLUGIN_20090315 (thumbnail, ThumbPluginVTable);
 
-const unsigned short TEXT_DISTANCE = 10;
-
 void
 ThumbScreen::freeThumbText (Thumbnail  *t)
 {
@@ -48,6 +46,9 @@
 ThumbScreen::renderThumbText (Thumbnail  *t,
 		 	      bool       freeThumb)
 {
+    if (!textPluginLoaded)
+	return;
+
     CompText::Attrib tA;
 
     if (freeThumb || !t->text)
@@ -56,18 +57,23 @@
 	t->text = new CompText ();
     }
 
-    if (!textPluginLoaded)
-	return;
-
     tA.maxWidth   = t->width;
     tA.maxHeight  = 100;
 
+    // text background
+    tA.bgHMargin  = 4;
+    tA.bgVMargin  = 4;
+    tA.bgColor[0] = optionGetFontBackgroundColorRed ();
+    tA.bgColor[1] = optionGetFontBackgroundColorGreen ();
+    tA.bgColor[2] = optionGetFontBackgroundColorBlue ();
+    tA.bgColor[3] = optionGetFontBackgroundColorAlpha ();
+
     tA.size       = optionGetFontSize ();
     tA.color[0]   = optionGetFontColorRed ();
     tA.color[1]   = optionGetFontColorGreen ();
     tA.color[2]   = optionGetFontColorBlue ();
     tA.color[3]   = optionGetFontColorAlpha ();
-    tA.flags      = CompText::Ellipsized;
+    tA.flags      = CompText::WithBackground | CompText::Ellipsized;
     if (optionGetFontBold ())
 	tA.flags |= CompText::StyleBold;
     tA.family     = "Sans";
@@ -85,7 +91,7 @@
     CompRect   rect (x, y, width, height);
 
     if (t->text)
-	rect.setHeight (rect.height () + t->text->getHeight () + TEXT_DISTANCE);
+	rect.setHeight (rect.height () + t->text->getHeight () + optionGetTextDistance ());
 
     CompRegion region (rect);
 
@@ -188,7 +194,7 @@
 
     tHeight = thumb.height;
     if (thumb.text)
-	tHeight += thumb.text->getHeight () + TEXT_DISTANCE;
+	tHeight += thumb.text->getHeight () + optionGetTextDistance ();
 
     /* Could someone please explain how this works */
 
@@ -835,7 +841,7 @@
     int                   wx = t->x;
     int                   wy = t->y;
     float                 width  = t->width;
-    float                 height = t->height;
+    float                 backheight = t->height;	// background/glow height
     GLWindowPaintAttrib     sAttrib;
     unsigned int          mask = PAINT_WINDOW_TRANSFORMED_MASK |
 	                         PAINT_WINDOW_TRANSLUCENT_MASK;
@@ -846,9 +852,6 @@
 
     sAttrib = gWindow->paintAttrib ();
 
-    if (t->text)
-	height += t->text->getHeight () + TEXT_DISTANCE;
-
     /* Wrap drawWindowGeometry to make sure the general
        drawWindowGeometry function is used */
     addWindowGeometryIndex =
@@ -873,7 +876,7 @@
 	    foreach (GLTexture *tex, windowTexture)
 	    {
 		tex->enable (GLTexture::Good);
-		paintTexture (*transform, color, wx, wy, width, height, off);
+		paintTexture (*transform, color, wx, wy, width, backheight, off);
 		tex->disable ();
 	    }
 	}
@@ -887,7 +890,7 @@
 	    foreach (GLTexture *tex, glowTexture)
 	    {
 		tex->enable (GLTexture::Good);
-		paintTexture (*transform, color, wx, wy, width, height, off);
+		paintTexture (*transform, color, wx, wy, width, backheight, off);
 		tex->disable ();
 	    }
 	}
@@ -897,7 +900,8 @@
 
 	if (t->text)
 	{
-	    float ox = 0.0;
+	    float ox = 0.0f;
+	    float height = backheight + t->text->getHeight () + optionGetTextDistance ();
 
 	    if (t->text->getWidth () < width)
 		ox = (width - t->text->getWidth ()) / 2.0;

=== modified file 'plugins/thumbnail/thumbnail.xml.in'
--- plugins/thumbnail/thumbnail.xml.in	2012-10-15 10:31:51 +0000
+++ plugins/thumbnail/thumbnail.xml.in	2013-01-31 19:13:21 +0000
@@ -93,6 +93,13 @@
             <_long>Should be the window title Bold.</_long>
             <default>true</default>
           </option>
+          <option name="text_distance" type="int">
+            <_short>Text Distance</_short>
+            <_long>The gap between the thumbnail and the window title text (in pixel).</_long>
+            <default>10</default>
+            <min>0</min>
+            <max>32</max>
+          </option>
           <option name="font_size" type="int">
             <_short>Font Size</_short>
             <_long>Window title Font Size.</_long>
@@ -104,10 +111,20 @@
             <_short>Font Color</_short>
             <_long>Window title Font Color.</_long>
             <default>
+              <red>0xffff</red>
+              <green>0xffff</green>
+              <blue>0xffff</blue>
+              <alpha>0xffff</alpha>
+            </default>
+          </option>
+          <option name="font_background_color" type="color">
+            <_short>Font Background Color</_short>
+            <_long>Color and opacity of the rounded rectangle title text background.</_long>
+            <default>
               <red>0x0000</red>
               <green>0x0000</green>
               <blue>0x0000</blue>
-              <alpha>0xffff</alpha>
+              <alpha>0x7fff</alpha>
             </default>
           </option>
         </subgroup>