Unity

Merge lp:~brandontschaefer/unity/lp.1371857-fix into lp:unity

  1. lp.1371857-fix
  2. Merge into trunk
Proposed by Brandon Schaefer
Status: Rejected
Rejected by: Christopher Townsend
Proposed branch: lp:~brandontschaefer/unity/lp.1371857-fix
Merge into: lp:unity
Diff against target: 90 lines (+15/-17)
1 file modified
unity-shared/StaticCairoText.cpp (+15/-17)
To merge this branch: bzr merge lp:~brandontschaefer/unity/lp.1371857-fix
Reviewer Review Type Date Requested Status
Christopher Townsend Disapprove
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+238340@code.launchpad.net

Commit message

Move notify::font_change over to the sig manager. This way we dont run into a possibly NULL setting when u-s-d goes down.

Description of the change

Move notify::font_change over to the sig manager. This way we dont run into a possibly NULL setting when u-s-d goes down.

To post a comment you must log in.
lp:~brandontschaefer/unity/lp.1371857-fix updated
3877. By Brandon Schaefer

* Dont use a sig manager for 1 signal.

3878. By Brandon Schaefer

* Move back to a sig manager, as it watchs for object destruction

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:3876
http://jenkins.qa.ubuntu.com/job/unity-ci/1096/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-utopic-amd64-ci/183/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-utopic-armhf-ci/183
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-utopic-i386-ci/183/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-ci/1096/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3878
http://jenkins.qa.ubuntu.com/job/unity-ci/1097/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-utopic-amd64-ci/184
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-utopic-armhf-ci/184
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-utopic-i386-ci/184

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-ci/1097/rebuild

review: Approve (continuous-integration)
lp:~brandontschaefer/unity/lp.1371857-fix updated
3879. By Brandon Schaefer

* Opps commented that out :)

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:3879
http://jenkins.qa.ubuntu.com/job/unity-ci/1099/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-utopic-amd64-ci/186
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-utopic-armhf-ci/186
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-utopic-i386-ci/186

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-ci/1099/rebuild

review: Approve (continuous-integration)
Revision history for this message
Christopher Townsend (townsend) wrote :

Disapproving due to using lp:~3v1n0/unity/glib-signal-safer-cb instead for the fix.

review: Disapprove

Unmerged revisions

3879. By Brandon Schaefer

* Opps commented that out :)

3878. By Brandon Schaefer

* Move back to a sig manager, as it watchs for object destruction

3877. By Brandon Schaefer

* Dont use a sig manager for 1 signal.

3876. By Brandon Schaefer

* Move over to use a sig manager to handle the notify::font_changed.
  Otherwise we can get a NULL gtk_setting from: gtk_settings_get_default()

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'unity-shared/StaticCairoText.cpp'
--- unity-shared/StaticCairoText.cpp 2014-03-20 05:05:21 +0000
+++ unity-shared/StaticCairoText.cpp 2014-10-14 23:14:57 +0000
@@ -34,6 +34,7 @@
34#include <pango/pangocairo.h>34#include <pango/pangocairo.h>
3535
36#include <UnityCore/GLibWrapper.h>36#include <UnityCore/GLibWrapper.h>
37#include <UnityCore/GLibSignal.h>
3738
38#include "CairoTexture.h"39#include "CairoTexture.h"
39#include "UnitySettings.h"40#include "UnitySettings.h"
@@ -42,6 +43,13 @@
4243
43namespace unity44namespace unity
44{45{
46
47namespace
48{
49 const std::string GNOME_UI_SETTINGS = "org.gnome.desktop.interface";
50 const std::string GNOME_FONT_NAME = "font-name";
51}
52
45struct StaticCairoText::Impl : sigc::trackable53struct StaticCairoText::Impl : sigc::trackable
46{54{
47 Impl(StaticCairoText* parent, std::string const& text);55 Impl(StaticCairoText* parent, std::string const& text);
@@ -77,8 +85,6 @@
77 void UpdateTexture();85 void UpdateTexture();
78 void OnFontChanged();86 void OnFontChanged();
7987
80 static void FontChanged(GObject* gobject, GParamSpec* pspec, gpointer data);
81
82 StaticCairoText* parent_;88 StaticCairoText* parent_;
83 bool accept_key_nav_focus_;89 bool accept_key_nav_focus_;
84 mutable bool need_new_extent_cache_;90 mutable bool need_new_extent_cache_;
@@ -102,6 +108,9 @@
102108
103 Size pre_layout_size_;109 Size pre_layout_size_;
104110
111 glib::Object<GSettings> gnome_ui_settings_;
112 glib::SignalManager signals_;
113
105 int lines_;114 int lines_;
106 int actual_lines_;115 int actual_lines_;
107 float line_spacing_;116 float line_spacing_;
@@ -119,6 +128,7 @@
119 , align_(NUX_ALIGN_LEFT)128 , align_(NUX_ALIGN_LEFT)
120 , valign_(NUX_ALIGN_CENTRE)129 , valign_(NUX_ALIGN_CENTRE)
121 , underline_(NUX_UNDERLINE_NONE)130 , underline_(NUX_UNDERLINE_NONE)
131 , gnome_ui_settings_(g_settings_new(GNOME_UI_SETTINGS.c_str()))
122 , lines_(-2) // should find out why -2...132 , lines_(-2) // should find out why -2...
123 // the desired height of the layout in Pango units if positive, or desired133 // the desired height of the layout in Pango units if positive, or desired
124 // number of lines if negative.134 // number of lines if negative.
@@ -126,19 +136,15 @@
126 , line_spacing_(0.5)136 , line_spacing_(0.5)
127 , scale_(1.0f)137 , scale_(1.0f)
128{138{
129 GtkSettings* settings = gtk_settings_get_default(); // not ref'ed139 signals_.Add<void, GSettings*, const gchar*>(gnome_ui_settings_, "notify::" + GNOME_FONT_NAME, [this] (GSettings*, const gchar* t) {
130 g_signal_connect(settings, "notify::gtk-font-name",140 OnFontChanged();
131 (GCallback)FontChanged, this);141 });
132142
133 Settings::Instance().font_scaling.changed.connect(sigc::hide(sigc::mem_fun(this, &Impl::OnFontChanged)));143 Settings::Instance().font_scaling.changed.connect(sigc::hide(sigc::mem_fun(this, &Impl::OnFontChanged)));
134}144}
135145
136StaticCairoText::Impl::~Impl()146StaticCairoText::Impl::~Impl()
137{147{
138 GtkSettings* settings = gtk_settings_get_default(); // not ref'ed
139 g_signal_handlers_disconnect_by_func(settings,
140 (void*)FontChanged,
141 this);
142}148}
143149
144PangoEllipsizeMode StaticCairoText::Impl::GetPangoEllipsizeMode() const150PangoEllipsizeMode StaticCairoText::Impl::GetPangoEllipsizeMode() const
@@ -780,14 +786,6 @@
780 }786 }
781}787}
782788
783void StaticCairoText::Impl::FontChanged(GObject* gobject,
784 GParamSpec* pspec,
785 gpointer data)
786{
787 StaticCairoText::Impl* self = static_cast<StaticCairoText::Impl*>(data);
788 self->OnFontChanged();
789}
790
791void StaticCairoText::Impl::OnFontChanged()789void StaticCairoText::Impl::OnFontChanged()
792{790{
793 need_new_extent_cache_ = true;791 need_new_extent_cache_ = true;