Merge lp:~canonical-dx-team/unity/unity.fix-685830 into lp:unity

Proposed by Mirco Müller
Status: Merged
Merged at revision: 867
Proposed branch: lp:~canonical-dx-team/unity/unity.fix-685830
Merge into: lp:unity
Diff against target: 243 lines (+192/-2)
5 files modified
src/PanelIndicatorObjectEntryView.cpp (+1/-0)
src/PanelStyle.cpp (+122/-0)
src/PanelStyle.h (+47/-0)
src/PanelView.cpp (+20/-2)
tests/CMakeLists.txt (+2/-0)
To merge this branch: bzr merge lp:~canonical-dx-team/unity/unity.fix-685830
Reviewer Review Type Date Requested Status
Neil J. Patel (community) Needs Fixing
Review via email: mp+43054@code.launchpad.net

Description of the change

New style-handling class for the panel implemented as a singleton.

To post a comment you must log in.
Revision history for this message
Neil J. Patel (njpatel) wrote :

It looks like you haven't yet hooked everything up so this merge proposal might be a bit premature. I'll comment on what I see anyway:

 - It should be _panel_style not _panelStyle, same goes for anything else like that

- GetDefault(), if _panelStyle == NULL, then create a new one and return it, but if one is already around: return _panelStyle->Reference(); On the other end, the objects should grab panel style object on construction and then ->UnReference it on destruction.

- To be more C++, you should probably have:

nux::Color& GetBackgroundTop ()

etc, so you can do nux::Color top = style->GetBackgroundTop ();, which is cleaner

- Probably can rid of printfs?

- I think the PanelIndicatorObjectEntryView bits still need to be hooked up

- Missing a "changed" signal so we can react to theme changes?

review: Needs Fixing
Revision history for this message
Mirco Müller (macslow) wrote :

> - It should be _panel_style not _panelStyle, same goes for anything else like
> that

Changed.
>
> - GetDefault(), if _panelStyle == NULL, then create a new one and return it,
> but if one is already around: return _panelStyle->Reference(); On the other
> end, the objects should grab panel style object on construction and then
> ->UnReference it on destruction.

But why should reference-counting be used, when PanelStyle is meant to be a singleton. I think these two concepts are mutual exclusive imo.

> - To be more C++, you should probably have:
>
> nux::Color& GetBackgroundTop ()

Initially you asked me to use ANSI-C-ish call-by-reference for this, but I've changed it now to the C++-reference return-type.

> - Probably can rid of printfs?

Yeah, was just for debugging.

> - I think the PanelIndicatorObjectEntryView bits still need to be hooked up
>
> - Missing a "changed" signal so we can react to theme changes?

Doing that atm.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/PanelIndicatorObjectEntryView.cpp'
2--- src/PanelIndicatorObjectEntryView.cpp 2010-12-07 15:01:07 +0000
3+++ src/PanelIndicatorObjectEntryView.cpp 2010-12-09 16:50:42 +0000
4@@ -25,6 +25,7 @@
5 #include "Nux/WindowCompositor.h"
6
7 #include "PanelIndicatorObjectEntryView.h"
8+#include "PanelStyle.h"
9
10 #include <glib.h>
11 #include <pango/pangocairo.h>
12
13=== added file 'src/PanelStyle.cpp'
14--- src/PanelStyle.cpp 1970-01-01 00:00:00 +0000
15+++ src/PanelStyle.cpp 2010-12-09 16:50:42 +0000
16@@ -0,0 +1,122 @@
17+/*
18+ * Copyright (C) 2010 Canonical Ltd
19+ *
20+ * This program is free software: you can redistribute it and/or modify
21+ * it under the terms of the GNU General Public License version 3 as
22+ * published by the Free Software Foundation.
23+ *
24+ * This program is distributed in the hope that it will be useful,
25+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
26+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
27+ * GNU General Public License for more details.
28+ *
29+ * You should have received a copy of the GNU General Public License
30+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
31+ *
32+ * Authored by: Mirco Müller <mirco.mueller@canonical.com>
33+ */
34+
35+#include <gtk/gtk.h>
36+
37+#include "PanelStyle.h"
38+
39+PanelStyle* PanelStyle::_panel_style = NULL;
40+
41+void
42+OnStyleChanged (GObject* gobject,
43+ GParamSpec* pspec,
44+ gpointer data)
45+{
46+ PanelStyle* self = (PanelStyle*) data;
47+
48+ self->sigChanged.emit (*self);
49+}
50+
51+PanelStyle*
52+PanelStyle::GetDefault ()
53+{
54+ if (_panel_style == NULL)
55+ {
56+ _panel_style = new PanelStyle ();
57+
58+ g_signal_connect (gtk_settings_get_default (),
59+ "notify::gtk-theme-name",
60+ G_CALLBACK (OnStyleChanged),
61+ _panel_style);
62+ }
63+
64+ _panel_style->Reference ();
65+ return _panel_style;
66+}
67+
68+nux::Color&
69+PanelStyle::GetTextColor ()
70+{
71+ nux::Color& color = _text;
72+
73+ return color;
74+}
75+
76+nux::Color&
77+PanelStyle::GetBackgroundTop ()
78+{
79+ nux::Color& color = _bg_top;
80+
81+ return color;
82+}
83+
84+nux::Color&
85+PanelStyle::GetBackgroundBottom ()
86+{
87+ nux::Color& color = _bg_bottom;
88+
89+ return color;
90+}
91+
92+nux::Color&
93+PanelStyle::GetTextShadow ()
94+{
95+ nux::Color& color = _text_shadow;
96+
97+ return color;
98+}
99+
100+PanelStyle::PanelStyle ()
101+{
102+ GtkWidget* menu_bar = NULL;
103+ GtkStyle* style = NULL;
104+
105+ menu_bar = gtk_menu_bar_new ();
106+ style = gtk_widget_get_style (menu_bar);
107+
108+ _text.SetRed ((float) style->text[4].red / (float) 0xffff);
109+ _text.SetGreen ((float) style->text[4].green / (float) 0xffff);
110+ _text.SetBlue ((float) style->text[4].blue / (float) 0xffff);
111+ _text.SetAlpha (1.0f);
112+
113+ _bg_top.SetRed ((float) style->bg[4].red / (float) 0xffff);
114+ _bg_top.SetGreen ((float) style->bg[4].green / (float) 0xffff);
115+ _bg_top.SetBlue ((float) style->bg[4].blue / (float) 0xffff);
116+ _bg_top = 0.4f * _bg_top;
117+ _bg_top.SetAlpha (1.0f);
118+
119+ _bg_bottom.SetRed ((float) style->bg[4].red / (float) 0xffff);
120+ _bg_bottom.SetGreen ((float) style->bg[4].green / (float) 0xffff);
121+ _bg_bottom.SetBlue ((float) style->bg[4].blue / (float) 0xffff);
122+ _bg_bottom = 0.22f * _bg_bottom;
123+ _bg_bottom.SetAlpha (1.0f);
124+
125+ _text_shadow.SetRed ((float) style->text[2].red / (float) 0xffff);
126+ _text_shadow.SetGreen ((float) style->text[2].green / (float) 0xffff);
127+ _text_shadow.SetBlue ((float) style->text[2].blue / (float) 0xffff);
128+ _text_shadow.SetAlpha (1.0f);
129+
130+ g_object_unref (style);
131+ g_object_unref (menu_bar);
132+}
133+
134+PanelStyle::~PanelStyle ()
135+{
136+ if (GetReferenceCount () != 0)
137+ g_warning ("PanelStyle::~PanelStyle() - Reference-counter is not 0!");
138+}
139
140=== added file 'src/PanelStyle.h'
141--- src/PanelStyle.h 1970-01-01 00:00:00 +0000
142+++ src/PanelStyle.h 2010-12-09 16:50:42 +0000
143@@ -0,0 +1,47 @@
144+/*
145+ * Copyright (C) 2010 Canonical Ltd
146+ *
147+ * This program is free software: you can redistribute it and/or modify
148+ * it under the terms of the GNU General Public License version 3 as
149+ * published by the Free Software Foundation.
150+ *
151+ * This program is distributed in the hope that it will be useful,
152+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
153+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
154+ * GNU General Public License for more details.
155+ *
156+ * You should have received a copy of the GNU General Public License
157+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
158+ *
159+ * Authored by: Mirco Müller <mirco.mueller@canonical.com>
160+ */
161+
162+#ifndef PANEL_STYLE_H
163+#define PANEL_STYLE_H
164+
165+#include "Nux/Nux.h"
166+
167+class PanelStyle : public nux::Object
168+{
169+ public:
170+ PanelStyle ();
171+ ~PanelStyle ();
172+
173+ static PanelStyle* GetDefault ();
174+
175+ nux::Color& GetTextColor ();
176+ nux::Color& GetBackgroundTop ();
177+ nux::Color& GetBackgroundBottom ();
178+ nux::Color& GetTextShadow ();
179+
180+ sigc::signal<void, PanelStyle&> sigChanged;
181+
182+ private:
183+ static PanelStyle* _panel_style;
184+ nux::Color _text;
185+ nux::Color _bg_top;
186+ nux::Color _bg_bottom;
187+ nux::Color _text_shadow;
188+};
189+
190+#endif // PANEL_STYLE_H
191
192=== modified file 'src/PanelView.cpp'
193--- src/PanelView.cpp 2010-12-09 15:52:53 +0000
194+++ src/PanelView.cpp 2010-12-09 16:50:42 +0000
195@@ -31,6 +31,7 @@
196 #include <glib.h>
197
198 #include "PanelView.h"
199+#include "PanelStyle.h"
200
201 #include "IndicatorObjectFactoryRemote.h"
202 #include "PanelIndicatorObjectView.h"
203@@ -156,8 +157,25 @@
204 cairo_set_line_width (cr, 1);
205
206 cairo_pattern_t *pat = cairo_pattern_create_linear (0, 0, 0, _last_height);
207- cairo_pattern_add_color_stop_rgb (pat, 0.0f, 89/255.0f, 88/255.0f, 83/255.0f);
208- cairo_pattern_add_color_stop_rgb (pat, 1.0f, 50/255.0f, 50/255.0f, 45/255.0f);
209+
210+ PanelStyle* style = PanelStyle::GetDefault ();
211+ nux::Color start;
212+ nux::Color end;
213+
214+ start = style->GetBackgroundTop ();
215+ end = style->GetBackgroundBottom ();
216+
217+ cairo_pattern_add_color_stop_rgb (pat,
218+ 0.0f,
219+ start.GetRed (),
220+ start.GetGreen (),
221+ start.GetBlue ());
222+ cairo_pattern_add_color_stop_rgb (pat,
223+ 1.0f,
224+ end.GetRed (),
225+ end.GetGreen (),
226+ end.GetBlue ());
227+
228 cairo_set_source (cr, pat);
229 cairo_rectangle (cr, 0, 0, _last_width, _last_height);
230 cairo_fill (cr);
231
232=== modified file 'tests/CMakeLists.txt'
233--- tests/CMakeLists.txt 2010-12-09 11:04:20 +0000
234+++ tests/CMakeLists.txt 2010-12-09 16:50:42 +0000
235@@ -62,6 +62,8 @@
236
237 add_executable (test-panel
238 TestPanel.cpp
239+ ../src/PanelStyle.cpp
240+ ../src/PanelStyle.h
241 ../src/PanelView.cpp
242 ../src/PanelView.h
243 ../src/PanelIndicatorObjectView.cpp