Merge lp:~mandel/unity/fix-static-cairo-text into lp:unity

[Manuel de la Peña]

- Fixes #1071327 by ensuring that the PreLayoutManagement method does not call SetBaseSize unless is really needed else (if it is called) the method will be called again making the nux layout get into an infinite loop.

Fixes #1071327 by ensuring that the PreLayoutManagement method does not call SetBaseSize unless is really needed else (if it is called) the method will be called again making the nux layout get into an infinite loop.

Tests can be ran doing ./tests/test-gtest --gtest_filter=*TestStaticCairoText* from the build dir.

[Nick Dedekind (nick-dedekind)]

LGTM.
Tests provided succeed.

review: Approve

[Marco Trevisan (Treviño) (3v1n0)]

Waiting for a proper fix in nux, this is fine.

review: Approve

[Brandon Schaefer (brandontschaefer)]

This causes a regression in the tooltips...

http://i.imgur.com/cVZUb.jpg

review: Needs Fixing

[Tim Penhey (thumper)]

https://code.launchpad.net/~thumper/unity/revert-r2960/+merge/138896 reverting this change until tooltips fixed.

[Tim Penhey (thumper)]

A test should be added to work out why tooltip rendering broke, and make the tests pass.

review: Needs Fixing

[Manuel de la Peña (mandel)]

Sorry, I did not check that the launcher would work correctly and I just focused on the Dash. Is a very simple fix will push asap (The min height of the label has to be the one returned by extend).

=== modified file 'tests/test_static_cairo_text.cpp'
--- tests/test_static_cairo_text.cpp	2012-11-30 09:56:52 +0000
+++ tests/test_static_cairo_text.cpp	2012-12-07 16:24:25 +0000
@@ -33,17 +33,29 @@
 class MockStaticCairoText : public nux::StaticCairoText
 {
 public:
+  MOCK_METHOD2(SetBaseSize, void(int, int));
+
   MockStaticCairoText():StaticCairoText("") {}
 
   using StaticCairoText::GetTextureStartIndices;
   using StaticCairoText::GetTextureEndIndices;
-};
-
-TEST(TestStaticCairoText, TextTextureSize)
-{
+  using StaticCairoText::PreLayoutManagement;
+};
+
+class TestStaticCairoText : public ::testing::Test
+{
+  protected:
+    TestStaticCairoText() : Test()
+    {
+      text = new MockStaticCairoText();
+    }
+    nux::ObjectPtr<MockStaticCairoText> text;
+};
+
+TEST_F(TestStaticCairoText, TextTextureSize)
+{
+  EXPECT_CALL(*text.GetPointer(), SetBaseSize(_, _)).Times(AnyNumber());
   // Test multi-texture stitching support.
-
-  nux::ObjectPtr<MockStaticCairoText> text(new MockStaticCairoText());
   text->SetLines(-2000);
   text->SetMaximumWidth(100);
 
@@ -70,4 +82,17 @@
   }
 }
 
+TEST_F(TestStaticCairoText, TextPreLayoutManagementMultipleCalls)
+{
+  EXPECT_CALL(*text.GetPointer(), SetBaseSize(_, _)).Times(2);
+
+  text->PreLayoutManagement();
+
+  // the first prelayout methods should have called set base size and therefore
+  // we should not call it again
+  EXPECT_CALL(*text.GetPointer(), SetBaseSize(_, _)).Times(0);
+
+  text->PreLayoutManagement();
+}
+
 }

=== modified file 'unity-shared/StaticCairoText.cpp'
--- unity-shared/StaticCairoText.cpp	2012-12-04 11:30:14 +0000
+++ unity-shared/StaticCairoText.cpp	2012-12-07 16:24:25 +0000
@@ -228,12 +228,21 @@
 
 void StaticCairoText::PreLayoutManagement()
 {
-  Geometry geo = GetGeometry();
-  pimpl->pre_layout_size_.width = geo.width;
-  pimpl->pre_layout_size_.height = geo.height;
-
-  SetBaseSize(pimpl->cached_extent_.width,
-              pimpl->cached_extent_.height);
+  Geometry const& geo = GetGeometry();
+
+  // only update the size if an only if and only if the width and height of
+  // the geo are diff
+  // FIXME: nux should be smart enough to not need this, we should get that
+  // fixed.
+  if(pimpl->pre_layout_size_.width != geo.width
+    || pimpl->pre_layout_size_.height != geo.height)
+  {
+    pimpl->pre_layout_size_.width = geo.width;
+    pimpl->pre_layout_size_.height = geo.height;
+
+    SetBaseSize(pimpl->cached_extent_.width,
+                pimpl->cached_extent_.height);
+  }
 
   if (pimpl->textures2D_.empty())
   {