Merge lp:~nick-dedekind/unity/lp1066788.cairo-text-stitching into lp:unity

[Nick Dedekind]

Split StaticCairoText text into multiple cached textures when the height of the required texture is over GpuInfo::MaxTextureSize. (LP: #1066788)

= Problem description =

https://bugs.launchpad.net/unity/+bug/1066788
Text labels with greater texture height than capable in OpenGL will not render correctly.

= The fix =

Split StaticCairoText text into multiple cached textures when the height of the required texture is over GpuInfo::MaxTextureSize.

= Test coverage =

Added unit test for StaticCairoText to test splitting of texture.
test-gtest --gtest_filter=TestStaticCairoText.TextTextureSize

[Brandon Schaefer (brandontschaefer)]

Hmm code looks good, and confirmed fixes the crash for me...but there is no test :(

Possibly add a manual test that says to open the StandaloneApplicationPreivew...(but they would have to compile...)

Or possibly a unit test that would attempt to make a texture that is to large and confirm that it is not returning NULL.

[Didier Roche (didrocks)]

Hey Brandon, Nick,

as discussed during the week pre-UDS, no more manual tests permitted as we will have automated daily upload to ubuntu, and so, no way to have manual perf performed before a release. Please instrument this part of code so that we can have an unit test.

[Stephen M. Webb (bregma)]

Needs test.

review: Needs Fixing

[Nick Dedekind (nick-dedekind)]

Added test

[Brandon Schaefer (brandontschaefer)]

Looks good to me.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/100/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/100/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/100/console

review: Needs Fixing (continuous-integration)

[Nick Dedekind (nick-dedekind)]

removed gcc-4.7 code and merged with trunk to resolve conflicts.

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/170/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/170/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/170/console

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/186/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/186/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/186/console

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/187/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/187/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/187/console

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/196/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/196/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/196/console

review: Needs Fixing (continuous-integration)

[Nick Dedekind (nick-dedekind)]

fixed conflict merging with trunk.

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/201/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/201/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/201/console

review: Needs Fixing (continuous-integration)

[lp:~nick-dedekind/unity/lp1066788.cairo-text-stitching]

2877. By Nick Dedekind on 2012-12-04

Another 64bit fix

=== modified file 'dash/previews/StandaloneApplicationPreview.cpp'
--- dash/previews/StandaloneApplicationPreview.cpp	2012-09-13 11:43:46 +0000
+++ dash/previews/StandaloneApplicationPreview.cpp	2012-12-04 11:32:23 +0000
@@ -156,8 +156,9 @@
   app_name << "Skype";
 
   const char* subtitle = "Version 3.2, Size 32 MB";
-  const char* description = "Skype is a proprietary voice-over-Internet Protocol service and software application originally created by Niklas Zennström and Janus Friis in 2003, and owned by Microsoft since 2011. \
-The service allows users to communicate with peers by voice, video, and instant messaging over the Internet. Phone calls may be placed to recipients on the traditional telephone networks. Calls to other users within the Skype service are free of charge, while calls to landline telephones and mobile phones are charged via a debit-based user account system.";
+  std::stringstream description;
+  for (int i = 0; i < 700; i++)
+    description << "Application description " << i << std::endl;
 
  // creates a generic preview object
   glib::Object<GIcon> iconHint1(g_icon_new_for_string("/usr/share/unity/5/lens-nav-music.svg", NULL));
@@ -179,7 +180,7 @@
   unity_protocol_preview_set_image_source_uri(proto_obj, "file:///home/nick/Skype.png");
   unity_protocol_preview_set_title(proto_obj, app_name.str().c_str());
   unity_protocol_preview_set_subtitle(proto_obj, subtitle);
-  unity_protocol_preview_set_description(proto_obj, description);
+  unity_protocol_preview_set_description(proto_obj, description.str().c_str());
   unity_protocol_preview_add_action(proto_obj, "uninstall", "Uninstall", iconHint1, 0);
   unity_protocol_preview_add_action_with_hints(proto_obj, "launch", "Download", iconHint2, 0, action_hints1);
   unity_protocol_preview_add_info_hint(proto_obj, "time", "Total time", iconHint1, g_variant_new("s", "16 h 34miin 45sec"));

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2012-12-03 15:31:29 +0000
+++ tests/CMakeLists.txt	2012-12-04 11:32:23 +0000
@@ -246,6 +246,7 @@
                  test_single_monitor_launcher_icon.cpp
                  test_expo_launcher_icon.cpp
                  test_showdesktop_handler.cpp
+								 test_static_cairo_text.cpp
                  test_switcher_controller.cpp
                  test_switcher_model.cpp
                  test_texture_cache.cpp

=== modified file 'tests/test_previews_application.cpp'
--- tests/test_previews_application.cpp	2012-09-13 16:11:54 +0000
+++ tests/test_previews_application.cpp	2012-12-04 11:32:23 +0000
@@ -14,7 +14,7 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
- * Authored by: Andrea Azzarone <[e-mail]>
+ * Authored by: Nick Dedekind <[e-mail]>
  */
 
 #include <list>

=== modified file 'tests/test_previews_generic.cpp'
--- tests/test_previews_generic.cpp	2012-09-13 16:11:54 +0000
+++ tests/test_previews_generic.cpp	2012-12-04 11:32:23 +0000
@@ -14,7 +14,7 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
- * Authored by: Andrea Azzarone <[e-mail]>
+ * Authored by: Nick Dedekind <[e-mail]>
  */
 
 #include <list>

=== modified file 'tests/test_previews_movie.cpp'
--- tests/test_previews_movie.cpp	2012-09-13 16:11:54 +0000
+++ tests/test_previews_movie.cpp	2012-12-04 11:32:23 +0000
@@ -14,7 +14,7 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
- * Authored by: Andrea Azzarone <[e-mail]>
+ * Authored by: Nick Dedekind <[e-mail]>
  */
 
 #include <list>

=== modified file 'tests/test_previews_music.cpp'
--- tests/test_previews_music.cpp	2012-09-13 16:11:54 +0000
+++ tests/test_previews_music.cpp	2012-12-04 11:32:23 +0000
@@ -14,7 +14,7 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
- * Authored by: Andrea Azzarone <[e-mail]>
+ * Authored by: Nick Dedekind <[e-mail]>
  */
 
 #include <list>

=== modified file 'tests/test_previews_social.cpp'
--- tests/test_previews_social.cpp	2012-09-14 19:53:57 +0000
+++ tests/test_previews_social.cpp	2012-12-04 11:32:23 +0000
@@ -14,8 +14,7 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
- * Authored by: Andrea Azzarone <[e-mail]>
- *              Ken VanDine <[e-mail]>
+ * Authored by: Ken VanDine <[e-mail]>
  */
 
 #include <list>

=== added file 'tests/test_static_cairo_text.cpp'
--- tests/test_static_cairo_text.cpp	1970-01-01 00:00:00 +0000
+++ tests/test_static_cairo_text.cpp	2012-12-04 11:32:23 +0000
@@ -0,0 +1,73 @@
+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
+/*
+ * Copyright (C) 2012 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Nick Dedekind <[e-mail]>
+ */
+
+#include <list>
+#include <gmock/gmock.h>
+using namespace testing;
+
+#include <Nux/Nux.h>
+#include <unity-shared/StaticCairoText.h>
+#include "test_utils.h"
+
+using namespace unity;
+
+namespace
+{
+
+class MockStaticCairoText : public nux::StaticCairoText
+{
+public:
+  MockStaticCairoText():StaticCairoText("") {}
+
+  using StaticCairoText::GetTextureStartIndices;
+  using StaticCairoText::GetTextureEndIndices;
+};
+
+TEST(TestStaticCairoText, TextTextureSize)
+{
+  // Test multi-texture stitching support.
+
+  nux::ObjectPtr<MockStaticCairoText> text(new MockStaticCairoText());
+  text->SetLines(-2000);
+  text->SetMaximumWidth(100);
+
+  std::stringstream ss;
+  std::vector<unsigned> starts;
+  std::vector<unsigned> ends;
+  while(starts.size() < 3)
+  {
+    for (int i = 0; i < 100; i++)
+      ss << "Test string\n";
+    text->SetText(ss.str());
+
+    starts = text->GetTextureStartIndices();
+    ends = text->GetTextureEndIndices();
+    ASSERT_TRUE(starts.size() == ends.size());
+
+    for (unsigned int start_index = 0; start_index < starts.size(); start_index++)
+    {
+      if (start_index > 0)
+      {
+        ASSERT_EQ(starts[start_index], ends[start_index-1]+1);
+      }
+    }
+  }
+}
+
+}

=== modified file 'unity-shared/StaticCairoText.cpp'
--- unity-shared/StaticCairoText.cpp	2012-11-23 16:07:12 +0000
+++ unity-shared/StaticCairoText.cpp	2012-12-04 11:32:23 +0000
@@ -17,6 +17,7 @@
  * Authored by: Jay Taoko <[e-mail]>
  *              Mirco Müller <[e-mail]>
  *              Tim Penhey <[e-mail]>
+ *              Nick Dedekind <[e-mail]>
  */
 
 #include "StaticCairoText.h"
@@ -52,10 +53,26 @@
   PangoAlignment GetPangoAlignment() const;
 
   std::string GetEffectiveFont() const;
+
+  struct CacheTexture
+  {
+    typedef std::shared_ptr<CacheTexture> Ptr;
+    CacheTexture()
+    : start_index(0)
+    , length((unsigned)std::string::npos)
+    , height(0)
+    {}
+
+    unsigned start_index;
+    unsigned length;
+    unsigned height;
+
+    std::shared_ptr<CairoGraphics> cr;
+  };
   Size GetTextExtents() const;
 
   void SetAttributes(PangoLayout* layout);
-  void DrawText(cairo_t* cr, int width, int height, int line_spacing, Color const& color);
+  void DrawText(CacheTexture::Ptr cached_texture, int width, int height, int line_spacing, Color const& color);
 
   void UpdateTexture();
   void OnFontChanged();
@@ -69,6 +86,7 @@
   mutable Size cached_extent_;
   mutable Size cached_base_;
   mutable int baseline_;
+  mutable std::list<CacheTexture::Ptr> cache_textures_;
 
   std::string text_;
   Color text_color_;
@@ -80,7 +98,7 @@
 
   std::string font_;
 
-  BaseTexturePtr texture2D_;
+  std::list<BaseTexturePtr> textures2D_;
 
   Size pre_layout_size_;
 
@@ -217,7 +235,7 @@
   SetBaseSize(pimpl->cached_extent_.width,
               pimpl->cached_extent_.height);
 
-  if (pimpl->texture2D_.IsNull())
+  if (pimpl->textures2D_.empty())
   {
     pimpl->UpdateTexture();
   }
@@ -254,7 +272,7 @@
 {
   Geometry const& base = GetGeometry();
 
-  if (pimpl->texture2D_.IsNull() ||
+  if (pimpl->textures2D_.empty() ||
       pimpl->cached_base_.width != base.width ||
       pimpl->cached_base_.height != base.height)
   {
@@ -284,13 +302,24 @@
                        base.height,
                        col);
 
-  gfxContext.QRP_1Tex(base.x,
-                      base.y + ((base.height - pimpl->cached_extent_.height) / 2),
-                      base.width,
-                      base.height,
-                      pimpl->texture2D_->GetDeviceTexture(),
-                      texxform,
-                      pimpl->text_color_);
+  int current_y = base.y + ((base.height - pimpl->cached_extent_.height) / 2);
+
+  for (BaseTexturePtr tex : pimpl->textures2D_)
+  {
+    nux::ObjectPtr<nux::IOpenGLBaseTexture> text_tex = tex->GetDeviceTexture();
+    if (!text_tex)
+      break;
+
+    gfxContext.QRP_1Tex(base.x,
+                    current_y,
+                    text_tex->GetWidth(),
+                    text_tex->GetHeight(),
+                    text_tex,
+                    texxform,
+                    pimpl->text_color_);
+
+    current_y += text_tex->GetHeight();
+  }
 
   gfxContext.GetRenderStates().SetBlend(alpha, src, dest);
 
@@ -427,6 +456,44 @@
   height = s.height;
 }
 
+std::vector<unsigned> StaticCairoText::GetTextureStartIndices()
+{
+  pimpl->GetTextExtents();
+
+  std::vector<unsigned> list;
+  auto iter = pimpl->cache_textures_.begin();
+  for (; iter != pimpl->cache_textures_.end(); ++iter)
+  {
+    Impl::CacheTexture::Ptr const& cached_texture = *iter;
+    list.push_back(cached_texture->start_index);
+  }
+  return list;
+}
+
+std::vector<unsigned> StaticCairoText::GetTextureEndIndices()
+{
+  pimpl->GetTextExtents();
+
+  std::vector<unsigned> list;
+  auto iter = pimpl->cache_textures_.begin();
+  for (; iter != pimpl->cache_textures_.end(); ++iter)
+  {
+    Impl::CacheTexture::Ptr const& cached_texture = *iter;
+    if (cached_texture->length == (unsigned)std::string::npos)
+    {
+      list.push_back((unsigned)std::string::npos);
+    }
+    else
+    {
+      if (cached_texture->start_index > 0 || cached_texture->length > 0)
+        list.push_back((unsigned)(cached_texture->start_index + cached_texture->length - 1));
+      else
+        list.push_back(0);
+    }
+  }
+  return list;
+}
+
 std::string StaticCairoText::Impl::GetEffectiveFont() const
 {
   if (font_.empty())
@@ -506,6 +573,50 @@
   baseline_ = pango_layout_get_baseline(layout) / PANGO_SCALE;
   need_new_extent_cache_ = false;
 
+  cache_textures_.clear();
+  PangoLayoutIter* iter = pango_layout_get_iter(layout);
+  CacheTexture::Ptr current_tex(new CacheTexture());
+  const int max_height = GetGraphicsDisplay()->GetGpuDevice()->GetGpuInfo().GetMaxTextureSize();
+  if (max_height < 0)
+    return nux::Size(0, 0);
+
+  do
+  {
+    PangoLayoutLine* line = pango_layout_iter_get_line_readonly(iter);
+    int y0 = 0, y1 = 0;
+    pango_layout_iter_get_line_yrange(iter, &y0, &y1);
+    y0 /= PANGO_SCALE;
+    y1 /= PANGO_SCALE;
+
+    if (line->start_index < 0 || y1 < y0)
+    {
+      current_tex.reset();
+      break;      
+    }
+    unsigned line_start_index = line->start_index;
+    unsigned height = y1-y0;
+
+    if (current_tex->height + height > (unsigned)max_height)
+    {
+      if (line_start_index > current_tex->start_index)
+        current_tex->length = line_start_index - current_tex->start_index;
+      else
+        current_tex->length = 0;
+      cache_textures_.push_back(current_tex);
+
+      // new texture.
+      current_tex.reset(new CacheTexture());
+      current_tex->start_index = line_start_index;
+      current_tex->height = 0;
+    }
+    current_tex->height += height;
+  }
+  while(pango_layout_iter_next_line(iter));
+  
+  if (current_tex) { cache_textures_.push_back(current_tex); }
+
+  pango_layout_iter_free(iter);
+
   // clean up
   pango_font_description_free(desc);
   g_object_unref(layout);
@@ -545,12 +656,17 @@
   pango_layout_set_attributes(layout, attr_list);
 }
 
-void StaticCairoText::Impl::DrawText(cairo_t* cr,
+void StaticCairoText::Impl::DrawText(CacheTexture::Ptr cached_texture,
                                      int width,
                                      int height,
                                      int line_spacing,
                                      Color const& color)
 {
+  if (!cached_texture)
+    return;
+  cached_texture->cr.reset(new CairoGraphics(CAIRO_FORMAT_ARGB32, width, height));
+  cairo_t* cr = cached_texture->cr->GetInternalContext();
+
   PangoLayout*          layout     = NULL;
   PangoFontDescription* desc       = NULL;
   PangoContext*         pangoCtx   = NULL;
@@ -558,18 +674,19 @@
   GdkScreen*            screen     = gdk_screen_get_default();    // not ref'ed
   GtkSettings*          settings   = gtk_settings_get_default();  // not ref'ed
 
+  std::string text = text_.substr(cached_texture->start_index, cached_texture->length);
+
   std::string font(GetEffectiveFont());
-
   cairo_set_font_options(cr, gdk_screen_get_font_options(screen));
+
   layout = pango_cairo_create_layout(cr);
-
-  
   desc = pango_font_description_from_string(font.c_str());
+
   pango_layout_set_font_description(layout, desc);
   pango_layout_set_wrap(layout, PANGO_WRAP_WORD_CHAR);
   pango_layout_set_ellipsize(layout, GetPangoEllipsizeMode());
   pango_layout_set_alignment(layout, GetPangoAlignment());
-  pango_layout_set_markup(layout, text_.c_str(), -1);
+  pango_layout_set_markup(layout, text.c_str(), -1);
   pango_layout_set_width(layout, width * PANGO_SCALE);
   pango_layout_set_height(layout, height * PANGO_SCALE);
   pango_layout_set_spacing(layout, line_spacing * PANGO_SCALE);
@@ -615,16 +732,18 @@
 {
   Size size = GetTextExtents();
   parent_->SetBaseSize(size.width, size.height);
-  // Now reget the internal geometry as it is clipped by the max size.
-  Geometry const& geo = parent_->GetGeometry();
-
-  CairoGraphics cairo_graphics(CAIRO_FORMAT_ARGB32,
-                               geo.width, geo.height);
-
-  DrawText(cairo_graphics.GetInternalContext(),
-           geo.width, geo.height, line_spacing_, text_color_);
-
-  texture2D_ = texture_ptr_from_cairo_graphics(cairo_graphics);
+  nux::Geometry const& geo = parent_->GetGeometry();
+
+  textures2D_.clear();
+  auto iter = cache_textures_.begin();
+  for (; iter != cache_textures_.end(); ++iter)
+  {
+    CacheTexture::Ptr const& cached_texture = *iter;
+    DrawText(cached_texture,
+             geo.width, cached_texture->height, line_spacing_, text_color_);
+
+    textures2D_.push_back(texture_ptr_from_cairo_graphics(*cached_texture->cr));
+  }
 }
 
 void StaticCairoText::Impl::FontChanged(GObject* gobject,

=== modified file 'unity-shared/StaticCairoText.h'
--- unity-shared/StaticCairoText.h	2012-11-23 16:07:12 +0000
+++ unity-shared/StaticCairoText.h	2012-12-04 11:32:23 +0000
@@ -110,6 +110,9 @@
   // Key navigation
   virtual bool AcceptKeyNavFocus();
 
+  std::vector<unsigned> GetTextureStartIndices();
+  std::vector<unsigned> GetTextureEndIndices();
+
 private:
   struct Impl;
   Impl* pimpl;