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

Proposed by Brandon Schaefer on 2014-10-14
Status: Rejected
Rejected by: Christopher Townsend on 2014-10-15
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 2014-10-14 Disapprove on 2014-10-15
PS Jenkins bot (community) continuous-integration Approve on 2014-10-15
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.
3877. By Brandon Schaefer on 2014-10-14

* Dont use a sig manager for 1 signal.

3878. By Brandon Schaefer on 2014-10-14

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

PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
3879. By Brandon Schaefer on 2014-10-14

* Opps commented that out :)

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
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 on 2014-10-14

* Opps commented that out :)

3878. By Brandon Schaefer on 2014-10-14

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

3877. By Brandon Schaefer on 2014-10-14

* Dont use a sig manager for 1 signal.

3876. By Brandon Schaefer on 2014-10-14

* 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
1=== modified file 'unity-shared/StaticCairoText.cpp'
2--- unity-shared/StaticCairoText.cpp 2014-03-20 05:05:21 +0000
3+++ unity-shared/StaticCairoText.cpp 2014-10-14 23:14:57 +0000
4@@ -34,6 +34,7 @@
5 #include <pango/pangocairo.h>
6
7 #include <UnityCore/GLibWrapper.h>
8+#include <UnityCore/GLibSignal.h>
9
10 #include "CairoTexture.h"
11 #include "UnitySettings.h"
12@@ -42,6 +43,13 @@
13
14 namespace unity
15 {
16+
17+namespace
18+{
19+ const std::string GNOME_UI_SETTINGS = "org.gnome.desktop.interface";
20+ const std::string GNOME_FONT_NAME = "font-name";
21+}
22+
23 struct StaticCairoText::Impl : sigc::trackable
24 {
25 Impl(StaticCairoText* parent, std::string const& text);
26@@ -77,8 +85,6 @@
27 void UpdateTexture();
28 void OnFontChanged();
29
30- static void FontChanged(GObject* gobject, GParamSpec* pspec, gpointer data);
31-
32 StaticCairoText* parent_;
33 bool accept_key_nav_focus_;
34 mutable bool need_new_extent_cache_;
35@@ -102,6 +108,9 @@
36
37 Size pre_layout_size_;
38
39+ glib::Object<GSettings> gnome_ui_settings_;
40+ glib::SignalManager signals_;
41+
42 int lines_;
43 int actual_lines_;
44 float line_spacing_;
45@@ -119,6 +128,7 @@
46 , align_(NUX_ALIGN_LEFT)
47 , valign_(NUX_ALIGN_CENTRE)
48 , underline_(NUX_UNDERLINE_NONE)
49+ , gnome_ui_settings_(g_settings_new(GNOME_UI_SETTINGS.c_str()))
50 , lines_(-2) // should find out why -2...
51 // the desired height of the layout in Pango units if positive, or desired
52 // number of lines if negative.
53@@ -126,19 +136,15 @@
54 , line_spacing_(0.5)
55 , scale_(1.0f)
56 {
57- GtkSettings* settings = gtk_settings_get_default(); // not ref'ed
58- g_signal_connect(settings, "notify::gtk-font-name",
59- (GCallback)FontChanged, this);
60+ signals_.Add<void, GSettings*, const gchar*>(gnome_ui_settings_, "notify::" + GNOME_FONT_NAME, [this] (GSettings*, const gchar* t) {
61+ OnFontChanged();
62+ });
63
64 Settings::Instance().font_scaling.changed.connect(sigc::hide(sigc::mem_fun(this, &Impl::OnFontChanged)));
65 }
66
67 StaticCairoText::Impl::~Impl()
68 {
69- GtkSettings* settings = gtk_settings_get_default(); // not ref'ed
70- g_signal_handlers_disconnect_by_func(settings,
71- (void*)FontChanged,
72- this);
73 }
74
75 PangoEllipsizeMode StaticCairoText::Impl::GetPangoEllipsizeMode() const
76@@ -780,14 +786,6 @@
77 }
78 }
79
80-void StaticCairoText::Impl::FontChanged(GObject* gobject,
81- GParamSpec* pspec,
82- gpointer data)
83-{
84- StaticCairoText::Impl* self = static_cast<StaticCairoText::Impl*>(data);
85- self->OnFontChanged();
86-}
87-
88 void StaticCairoText::Impl::OnFontChanged()
89 {
90 need_new_extent_cache_ = true;