Merge lp:~3v1n0/unity/fix-load-icon-crash-926658 into lp:unity
- fix-load-icon-crash-926658
- Merge into trunk
Status: | Rejected |
---|---|
Rejected by: | Marco Trevisan (Treviño) |
Proposed branch: | lp:~3v1n0/unity/fix-load-icon-crash-926658 |
Merge into: | lp:unity |
Prerequisite: | lp:~3v1n0/unity/barrier-timeout |
Diff against target: |
641 lines (+150/-144) 7 files modified
UnityCore/ModelRowAdaptor.h (+1/-1) dash/LensView.cpp (+40/-5) dash/LensView.h (+4/-0) dash/ResultRendererTile.cpp (+80/-98) dash/ResultRendererTile.h (+1/-2) dash/ResultView.cpp (+21/-32) dash/ResultView.h (+3/-6) |
To merge this branch: | bzr merge lp:~3v1n0/unity/fix-load-icon-crash-926658 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Michal Hruby (community) | Needs Fixing | ||
Francis Ginther | Abstain | ||
jenkins (community) | continuous-integration | Needs Fixing | |
Review via email: mp+116091@code.launchpad.net |
This proposal supersedes a proposal from 2012-07-20.
Commit message
ResultRendererTile: don't crash in LoadIcon if row.renderer<
Description of the change
In ResultRendererT
The implementation of that depends on dee_model_
So, at this point, even if it would not the best solution, it's better to add a safety check to prevent crashes.
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
Yes, that's true... But according to the stacktraces I got (attached to the bug), the crash is not happening because dee_model_get_tag returns an unreferenced pointer, but because it's returning a null one...
I'm wondering if that could happen that could be happen this:
ResultRendererT
ResultRendererT
ResultRendererT
But it would be weird as all seems done in the same thread...
jenkins (martin-mrazik+qa) wrote : | # |
PASSED: Continuous integration, rev:2540
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Michal Hruby (mhr3) wrote : | # |
As you alone said in the comment, the renderer is set in Preload(), so the only explanation is that the model is already invalid at that point, but dee notices that and ignores the call (while logging a critical), doing dee_model_
Other times dee doesn't notice that the model is invalid, and crashes inside itself.
jenkins (martin-mrazik+qa) wrote : | # |
PASSED: Continuous integration, rev:2545
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
Well, it's the same I thought, but I'm not getting that on the stacktrace...
However, protecting us from possible invalid pointers is something we should do in any case.
jenkins (martin-mrazik+qa) wrote : | # |
PASSED: Continuous integration, rev:2547
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Marco Trevisan (Treviño) (3v1n0) wrote : | # |
I've reffed the model in ModelRowAdaptor... As you said it's not the full solution, but it's something we can safely do anyway. Let me know if this is better for you.
jenkins (martin-mrazik+qa) wrote : | # |
PASSED: Continuous integration, rev:2551
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
jenkins (martin-mrazik+qa) wrote : | # |
PASSED: Continuous integration, rev:2552
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Michal Hruby (mhr3) wrote : | # |
> I've reffed the model in ModelRowAdaptor... As you said it's not the full
> solution, but it's something we can safely do anyway. Let me know if this is
> better for you.
As mentioned on IRC, I'd rather see a *single* weak ref on the model in LensView? than this. (since adding a reference for each model row isn't a good idea)
jenkins (martin-mrazik+qa) wrote : | # |
FAILED: Continuous integration, rev:2556
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Francis Ginther (fginther) wrote : | # |
Review was claimed by accident, please ignore.
Michal Hruby (mhr3) wrote : | # |
Discussed on IRC, but so make sure it doesn't get lost:
1) the model can change, you need to listen to changed signal
2) in the weak notify callback you're basically iterating over the model, which you can't since the model is already dead
Preview Diff
1 | === modified file 'UnityCore/ModelRowAdaptor.h' |
2 | --- UnityCore/ModelRowAdaptor.h 2012-06-19 16:47:56 +0000 |
3 | +++ UnityCore/ModelRowAdaptor.h 2012-08-10 12:37:21 +0000 |
4 | @@ -21,8 +21,8 @@ |
5 | #define UNITY_MODEL_ROW_H |
6 | |
7 | #include <string> |
8 | - |
9 | #include <dee.h> |
10 | +#include "GLibWrapper.h" |
11 | |
12 | namespace unity |
13 | { |
14 | |
15 | === modified file 'dash/LensView.cpp' |
16 | --- dash/LensView.cpp 2012-06-18 02:57:23 +0000 |
17 | +++ dash/LensView.cpp 2012-08-10 12:37:21 +0000 |
18 | @@ -123,6 +123,7 @@ |
19 | , filters_expanded(false) |
20 | , can_refine_search(false) |
21 | , no_results_active_(false) |
22 | + , results_model_(nullptr) |
23 | {} |
24 | |
25 | LensView::LensView(Lens::Ptr lens, nux::Area* show_filters) |
26 | @@ -132,6 +133,7 @@ |
27 | , lens_(lens) |
28 | , initial_activation_(true) |
29 | , no_results_active_(false) |
30 | + , results_model_(nullptr) |
31 | { |
32 | SetupViews(show_filters); |
33 | SetupCategories(); |
34 | @@ -175,6 +177,14 @@ |
35 | |
36 | } |
37 | |
38 | +LensView::~LensView() |
39 | +{ |
40 | + if (DEE_IS_MODEL(results_model_)) |
41 | + { |
42 | + g_object_weak_unref(G_OBJECT(results_model_), (GWeakNotify)LensView::OnModelDestroyed, this); |
43 | + } |
44 | +} |
45 | + |
46 | void LensView::SetupViews(nux::Area* show_filters) |
47 | { |
48 | dash::Style& style = dash::Style::Instance(); |
49 | @@ -231,12 +241,39 @@ |
50 | OnCategoryAdded(categories->RowAtIndex(i)); |
51 | } |
52 | |
53 | +void LensView::OnModelDestroyed(LensView* self, DeeModel* was_here) |
54 | +{ |
55 | + for (auto group : self->categories_) |
56 | + { |
57 | + auto grid = static_cast<ResultViewGrid*>(group->GetChildView()); |
58 | + self->counts_[group] -= grid->GetResultList().size(); |
59 | + grid->ClearResults(); |
60 | + self->UpdateCounts(group); |
61 | + } |
62 | +} |
63 | + |
64 | void LensView::SetupResults() |
65 | { |
66 | Results::Ptr results = lens_->results; |
67 | results->result_added.connect(sigc::mem_fun(this, &LensView::OnResultAdded)); |
68 | results->result_removed.connect(sigc::mem_fun(this, &LensView::OnResultRemoved)); |
69 | |
70 | + results->model.changed.connect([this] (glib::Object<DeeModel> model) |
71 | + { |
72 | + if (DEE_IS_MODEL(results_model_)) |
73 | + { |
74 | + g_object_weak_unref(G_OBJECT(results_model_), (GWeakNotify)LensView::OnModelDestroyed, this); |
75 | + } |
76 | + |
77 | + results_model_ = nullptr; |
78 | + |
79 | + if (model.IsType(DEE_TYPE_MODEL)) |
80 | + { |
81 | + results_model_ = model; |
82 | + g_object_weak_ref(glib::object_cast<GObject>(model), (GWeakNotify)LensView::OnModelDestroyed, this); |
83 | + } |
84 | + }); |
85 | + |
86 | for (unsigned int i = 0; i < results->count(); ++i) |
87 | OnResultAdded(results->RowAtIndex(i)); |
88 | } |
89 | @@ -285,7 +322,7 @@ |
90 | grid->SetModelRenderer(new ResultRendererHorizontalTile(NUX_TRACKER_LOCATION)); |
91 | grid->horizontal_spacing = CARD_VIEW_GAP_HORIZ; |
92 | grid->vertical_spacing = CARD_VIEW_GAP_VERT; |
93 | - } |
94 | + } |
95 | else |
96 | grid->SetModelRenderer(new ResultRendererTile(NUX_TRACKER_LOCATION)); |
97 | |
98 | @@ -305,8 +342,7 @@ |
99 | PlacesGroup* group = categories_.at(result.category_index); |
100 | ResultViewGrid* grid = static_cast<ResultViewGrid*>(group->GetChildView()); |
101 | |
102 | - std::string uri = result.uri; |
103 | - LOG_TRACE(logger) << "Result added: " << uri; |
104 | + LOG_TRACE(logger) << "Result added: " << result.uri(); |
105 | |
106 | grid->AddResult(const_cast<Result&>(result)); |
107 | counts_[group]++; |
108 | @@ -329,8 +365,7 @@ |
109 | PlacesGroup* group = categories_.at(result.category_index); |
110 | ResultViewGrid* grid = static_cast<ResultViewGrid*>(group->GetChildView()); |
111 | |
112 | - std::string uri = result.uri; |
113 | - LOG_TRACE(logger) << "Result removed: " << uri; |
114 | + LOG_TRACE(logger) << "Result removed: " << result.uri(); |
115 | |
116 | grid->RemoveResult(const_cast<Result&>(result)); |
117 | counts_[group]--; |
118 | |
119 | === modified file 'dash/LensView.h' |
120 | --- dash/LensView.h 2012-06-18 02:57:23 +0000 |
121 | +++ dash/LensView.h 2012-08-10 12:37:21 +0000 |
122 | @@ -52,6 +52,7 @@ |
123 | public: |
124 | LensView(); |
125 | LensView(Lens::Ptr lens, nux::Area* show_filters); |
126 | + ~LensView(); |
127 | |
128 | CategoryGroups& categories() { return categories_; } |
129 | FilterBar* filter_bar() const { return filter_bar_; } |
130 | @@ -101,6 +102,8 @@ |
131 | |
132 | std::string get_search_string() const; |
133 | |
134 | + static void OnModelDestroyed(LensView* self, DeeModel* was_here); |
135 | + |
136 | private: |
137 | Lens::Ptr lens_; |
138 | CategoryGroups categories_; |
139 | @@ -116,6 +119,7 @@ |
140 | nux::VLayout* fscroll_layout_; |
141 | FilterBar* filter_bar_; |
142 | nux::StaticCairoText* no_results_; |
143 | + DeeModel* results_model_; |
144 | |
145 | UBusManager ubus_manager_; |
146 | glib::Source::UniquePtr fix_rendering_idle_; |
147 | |
148 | === modified file 'dash/ResultRendererTile.cpp' |
149 | --- dash/ResultRendererTile.cpp 2012-06-18 02:57:23 +0000 |
150 | +++ dash/ResultRendererTile.cpp 2012-08-10 12:37:21 +0000 |
151 | @@ -21,14 +21,10 @@ |
152 | */ |
153 | |
154 | |
155 | -#include <sstream> // for ostringstream |
156 | #include "ResultRendererTile.h" |
157 | |
158 | -#include <boost/algorithm/string.hpp> |
159 | - |
160 | #include <pango/pango.h> |
161 | #include <pango/pangocairo.h> |
162 | -#include <gdk/gdk.h> |
163 | #include <gtk/gtk.h> |
164 | |
165 | #include <NuxCore/Logger.h> |
166 | @@ -40,19 +36,18 @@ |
167 | #include "unity-shared/IconTexture.h" |
168 | #include "unity-shared/TextureCache.h" |
169 | |
170 | - |
171 | -namespace |
172 | -{ |
173 | - bool neko; |
174 | -} |
175 | - |
176 | namespace unity |
177 | { |
178 | namespace |
179 | { |
180 | nux::logging::Logger logger("unity.dash.results"); |
181 | +const std::string DEFAULT_GICON = ". GThemedIcon text-x-preview"; |
182 | |
183 | const int FONT_SIZE = 10; |
184 | +bool neko = false; |
185 | +const std::string neko_env = {0x55, 0x4e, 0x49, 0x54, 0x59, 0x5f, 0x4e, 0x45, 0x4b, 0x4f}; |
186 | +const std::string neko_src = {0x68, 0x74, 0x74, 0x70, 0x3a, 0x2f, 0x2f, 0x70, 0x6c, 0x61, 0x63, |
187 | + 0x65, 0x6b, 0x69, 0x74, 0x74, 0x65, 0x6e, 0x2e, 0x63, 0x6f, 0x6d}; |
188 | } |
189 | |
190 | namespace dash |
191 | @@ -69,11 +64,7 @@ |
192 | dash::Style& style = dash::Style::Instance(); |
193 | width = style.GetTileWidth(); |
194 | height = style.GetTileHeight(); |
195 | - |
196 | - gsize tmp; |
197 | - gchar* tmp1 = (gchar*)g_base64_decode("VU5JVFlfTkVLTw==", &tmp); |
198 | - neko = (g_getenv(tmp1)); |
199 | - g_free (tmp1); |
200 | + neko = g_getenv(neko_env.c_str()); |
201 | |
202 | // pre-load the highlight texture |
203 | // try and get a texture from the texture cache |
204 | @@ -84,10 +75,6 @@ |
205 | sigc::mem_fun(this, &ResultRendererTile::DrawHighlight)); |
206 | } |
207 | |
208 | -ResultRendererTile::~ResultRendererTile() |
209 | -{ |
210 | -} |
211 | - |
212 | void ResultRendererTile::Render(nux::GraphicsEngine& GfxContext, |
213 | Result& row, |
214 | ResultRendererState state, |
215 | @@ -95,7 +82,8 @@ |
216 | int x_offset, int y_offset) |
217 | { |
218 | TextureContainer* container = row.renderer<TextureContainer*>(); |
219 | - if (container == nullptr) |
220 | + |
221 | + if (!container) |
222 | return; |
223 | |
224 | dash::Style& style = dash::Style::Instance(); |
225 | @@ -105,7 +93,7 @@ |
226 | nux::TexCoordXForm texxform; |
227 | |
228 | int icon_width, icon_height; |
229 | - if (container->icon == nullptr) |
230 | + if (!container->icon) |
231 | { |
232 | icon_width = icon_height = tile_icon_size; |
233 | } |
234 | @@ -230,7 +218,7 @@ |
235 | |
236 | void ResultRendererTile::Preload(Result& row) |
237 | { |
238 | - if (row.renderer<TextureContainer*>() == nullptr) |
239 | + if (!row.renderer<TextureContainer*>()) |
240 | { |
241 | row.set_renderer(new TextureContainer()); |
242 | LoadIcon(row); |
243 | @@ -240,52 +228,53 @@ |
244 | |
245 | void ResultRendererTile::Unload(Result& row) |
246 | { |
247 | - TextureContainer *container = row.renderer<TextureContainer*>(); |
248 | - delete container; |
249 | + delete row.renderer<TextureContainer*>(); |
250 | row.set_renderer<TextureContainer*>(nullptr); |
251 | } |
252 | |
253 | void ResultRendererTile::LoadIcon(Result& row) |
254 | { |
255 | - Style& style = Style::Instance(); |
256 | - std::string const& icon_hint = row.icon_hint; |
257 | -#define DEFAULT_GICON ". GThemedIcon text-x-preview" |
258 | - std::string icon_name; |
259 | - if (G_UNLIKELY(neko)) |
260 | - { |
261 | - int tmp1 = style.GetTileIconSize() + (rand() % 16) - 8; |
262 | - gsize tmp3; |
263 | - gchar* tmp2 = (gchar*)g_base64_decode("aHR0cDovL3BsYWNla2l0dGVuLmNvbS8laS8laS8=", &tmp3); |
264 | - gchar* tmp4 = g_strdup_printf(tmp2, tmp1, tmp1); |
265 | - icon_name = tmp4; |
266 | - g_free(tmp4); |
267 | - g_free(tmp2); |
268 | - } |
269 | - else |
270 | - { |
271 | - icon_name = !icon_hint.empty() ? icon_hint : DEFAULT_GICON; |
272 | - } |
273 | - |
274 | - GIcon* icon = g_icon_new_for_string(icon_name.c_str(), NULL); |
275 | - TextureContainer* container = row.renderer<TextureContainer*>(); |
276 | - |
277 | - IconLoader::IconLoaderCallback slot = sigc::bind(sigc::mem_fun(this, &ResultRendererTile::IconLoaded), icon_hint, row); |
278 | - |
279 | - if (g_strrstr(icon_name.c_str(), "://")) |
280 | - { |
281 | - container->slot_handle = IconLoader::GetDefault().LoadFromURI(icon_name, style.GetTileIconSize(), slot); |
282 | - } |
283 | - else if (G_IS_ICON(icon)) |
284 | - { |
285 | - container->slot_handle = IconLoader::GetDefault().LoadFromGIconString(icon_name, style.GetTileIconSize(), slot); |
286 | - } |
287 | - else |
288 | - { |
289 | - container->slot_handle = IconLoader::GetDefault().LoadFromIconName(icon_name, style.GetTileIconSize(), slot); |
290 | - } |
291 | - |
292 | - if (icon != NULL) |
293 | - g_object_unref(icon); |
294 | + auto container = row.renderer<TextureContainer*>(); |
295 | + |
296 | + if (!container) |
297 | + { |
298 | + LOG_ERROR(logger) << "No valid container for Result " << row.name() << " with URI " |
299 | + << row.uri(); |
300 | + return; |
301 | + } |
302 | + |
303 | + int icon_size = Style::Instance().GetTileIconSize(); |
304 | + std::string icon_name = row.icon_hint(); |
305 | + |
306 | + if (neko) |
307 | + { |
308 | + auto tmp = std::to_string(icon_size + (rand() % 16) - 8); |
309 | + icon_name = neko_src + '/' + tmp + '/' + tmp; |
310 | + } |
311 | + else if (icon_name.empty()) |
312 | + { |
313 | + icon_name = DEFAULT_GICON; |
314 | + } |
315 | + |
316 | + auto slot = sigc::bind(sigc::mem_fun(this, &ResultRendererTile::IconLoaded), row); |
317 | + |
318 | + if (icon_name.find("://") != std::string::npos) |
319 | + { |
320 | + container->slot_handle = IconLoader::GetDefault().LoadFromURI(icon_name, icon_size, slot); |
321 | + } |
322 | + else |
323 | + { |
324 | + glib::Object<GIcon> icon(g_icon_new_for_string(icon_name.c_str(), nullptr)); |
325 | + |
326 | + if (icon.IsType(G_TYPE_ICON)) |
327 | + { |
328 | + container->slot_handle = IconLoader::GetDefault().LoadFromGIconString(icon_name, icon_size, slot); |
329 | + } |
330 | + else |
331 | + { |
332 | + container->slot_handle = IconLoader::GetDefault().LoadFromIconName(icon_name, icon_size, slot); |
333 | + } |
334 | + } |
335 | } |
336 | |
337 | nux::BaseTexture* ResultRendererTile::CreateTextureCallback(std::string const& texid, |
338 | @@ -405,20 +394,20 @@ |
339 | void ResultRendererTile::IconLoaded(std::string const& texid, |
340 | unsigned size, |
341 | glib::Object<GdkPixbuf> const& pixbuf, |
342 | - std::string icon_name, |
343 | Result& row) |
344 | { |
345 | TextureContainer *container = row.renderer<TextureContainer*>(); |
346 | - Style& style = Style::Instance(); |
347 | + int icon_size = Style::Instance().GetTileIconSize(); |
348 | |
349 | if (pixbuf && container) |
350 | { |
351 | + std::string const& icon_name = row.icon_hint(); |
352 | TextureCache& cache = TextureCache::GetDefault(); |
353 | - BaseTexturePtr texture(cache.FindTexture(icon_name, style.GetTileIconSize(), style.GetTileIconSize(), |
354 | + BaseTexturePtr texture(cache.FindTexture(icon_name, icon_size, icon_size, |
355 | sigc::bind(sigc::mem_fun(this, &ResultRendererTile::CreateTextureCallback), pixbuf))); |
356 | |
357 | std::string blur_texid = icon_name + "_blurred"; |
358 | - BaseTexturePtr texture_blurred(cache.FindTexture(blur_texid, style.GetTileIconSize(), style.GetTileIconSize(), |
359 | + BaseTexturePtr texture_blurred(cache.FindTexture(blur_texid, icon_size, icon_size, |
360 | sigc::bind(sigc::mem_fun(this, &ResultRendererTile::CreateBlurredTextureCallback), pixbuf))); |
361 | |
362 | BaseTexturePtr texture_prelight(cache.FindTexture("resultview_prelight", texture->GetWidth() + highlight_padding*2, texture->GetHeight() + highlight_padding*2, sigc::mem_fun(this, &ResultRendererTile::DrawHighlight))); |
363 | @@ -426,17 +415,15 @@ |
364 | container->icon = texture; |
365 | container->blurred_icon = texture_blurred; |
366 | container->prelight = texture_prelight; |
367 | + container->slot_handle = 0; |
368 | |
369 | NeedsRedraw.emit(); |
370 | - |
371 | - if (container) |
372 | - container->slot_handle = 0; |
373 | } |
374 | else if (container) |
375 | { |
376 | // we need to load a missing icon |
377 | - IconLoader::IconLoaderCallback slot = sigc::bind(sigc::mem_fun(this, &ResultRendererTile::IconLoaded), icon_name, row); |
378 | - container->slot_handle = IconLoader::GetDefault().LoadFromGIconString(". GThemedIcon text-x-preview", style.GetTileIconSize(), slot); |
379 | + IconLoader::IconLoaderCallback slot = sigc::bind(sigc::mem_fun(this, &ResultRendererTile::IconLoaded), row); |
380 | + container->slot_handle = IconLoader::GetDefault().LoadFromGIconString(DEFAULT_GICON, icon_size, slot); |
381 | } |
382 | |
383 | } |
384 | @@ -444,29 +431,31 @@ |
385 | |
386 | void ResultRendererTile::LoadText(Result& row) |
387 | { |
388 | + TextureContainer *container = row.renderer<TextureContainer*>(); |
389 | + |
390 | + if (!container) |
391 | + return; |
392 | + |
393 | + GdkScreen* screen = gdk_screen_get_default(); |
394 | Style& style = Style::Instance(); |
395 | nux::CairoGraphics _cairoGraphics(CAIRO_FORMAT_ARGB32, |
396 | style.GetTileWidth() - (padding * 2), |
397 | style.GetTileHeight() - style.GetTileIconSize() - spacing); |
398 | |
399 | + glib::String font; |
400 | + g_object_get(gtk_settings_get_default(), "gtk-font-name", &font, nullptr); |
401 | + |
402 | + int dpi = -1; |
403 | + g_object_get(gtk_settings_get_default(), "gtk-xft-dpi", &dpi, nullptr); |
404 | + |
405 | cairo_t* cr = _cairoGraphics.GetContext(); |
406 | - |
407 | - PangoLayout* layout = NULL; |
408 | - PangoFontDescription* desc = NULL; |
409 | - PangoContext* pango_context = NULL; |
410 | - GdkScreen* screen = gdk_screen_get_default(); // not ref'ed |
411 | - glib::String font; |
412 | - int dpi = -1; |
413 | - |
414 | - g_object_get(gtk_settings_get_default(), "gtk-font-name", &font, NULL); |
415 | - g_object_get(gtk_settings_get_default(), "gtk-xft-dpi", &dpi, NULL); |
416 | - |
417 | cairo_set_font_options(cr, gdk_screen_get_font_options(screen)); |
418 | - layout = pango_cairo_create_layout(cr); |
419 | - desc = pango_font_description_from_string(font.Value()); |
420 | - pango_font_description_set_size (desc, FONT_SIZE * PANGO_SCALE); |
421 | + glib::Object<PangoLayout> layout(pango_cairo_create_layout(cr)); |
422 | + std::shared_ptr<PangoFontDescription> desc(pango_font_description_from_string(font), |
423 | + pango_font_description_free); |
424 | + pango_font_description_set_size (desc.get(), FONT_SIZE * PANGO_SCALE); |
425 | |
426 | - pango_layout_set_font_description(layout, desc); |
427 | + pango_layout_set_font_description(layout, desc.get()); |
428 | pango_layout_set_alignment(layout, PANGO_ALIGN_CENTER); |
429 | |
430 | pango_layout_set_wrap(layout, PANGO_WRAP_WORD_CHAR); |
431 | @@ -474,17 +463,14 @@ |
432 | pango_layout_set_width(layout, (style.GetTileWidth() - (padding * 2))* PANGO_SCALE); |
433 | pango_layout_set_height(layout, -2); |
434 | |
435 | - char *escaped_text = g_markup_escape_text(row.name().c_str() , -1); |
436 | - |
437 | + glib::String escaped_text(g_markup_escape_text(row.name().c_str(), -1)); |
438 | pango_layout_set_markup(layout, escaped_text, -1); |
439 | |
440 | - g_free (escaped_text); |
441 | - |
442 | - pango_context = pango_layout_get_context(layout); // is not ref'ed |
443 | + PangoContext* pango_context = pango_layout_get_context(layout); // is not ref'ed |
444 | pango_cairo_context_set_font_options(pango_context, |
445 | gdk_screen_get_font_options(screen)); |
446 | pango_cairo_context_set_resolution(pango_context, |
447 | - dpi == -1 ? 96.0f : dpi/(float) PANGO_SCALE); |
448 | + dpi == -1 ? 96.0f : dpi/static_cast<float>(PANGO_SCALE)); |
449 | pango_layout_context_changed(layout); |
450 | |
451 | cairo_set_operator(cr, CAIRO_OPERATOR_CLEAR); |
452 | @@ -497,13 +483,9 @@ |
453 | pango_cairo_show_layout(cr, layout); |
454 | |
455 | // clean up |
456 | - pango_font_description_free(desc); |
457 | - g_object_unref(layout); |
458 | cairo_destroy(cr); |
459 | |
460 | - TextureContainer *container = row.renderer<TextureContainer*>(); |
461 | - if (container) |
462 | - container->text = texture_ptr_from_cairo_graphics(_cairoGraphics); |
463 | + container->text = texture_ptr_from_cairo_graphics(_cairoGraphics); |
464 | } |
465 | |
466 | |
467 | |
468 | === modified file 'dash/ResultRendererTile.h' |
469 | --- dash/ResultRendererTile.h 2012-07-04 02:37:23 +0000 |
470 | +++ dash/ResultRendererTile.h 2012-08-10 12:37:21 +0000 |
471 | @@ -67,7 +67,6 @@ |
472 | NUX_DECLARE_OBJECT_TYPE(ResultRendererTile, ResultRenderer); |
473 | |
474 | ResultRendererTile(NUX_FILE_LINE_PROTO); |
475 | - ~ResultRendererTile(); |
476 | |
477 | virtual void Render(nux::GraphicsEngine& GfxContext, |
478 | Result& row, |
479 | @@ -93,7 +92,7 @@ |
480 | //icon loading callbacks |
481 | void IconLoaded(std::string const& texid, unsigned size, |
482 | glib::Object<GdkPixbuf> const& pixbuf, |
483 | - std::string icon_name, Result& row); |
484 | + Result& row); |
485 | nux::BaseTexture* CreateTextureCallback(std::string const& texid, |
486 | int width, int height, |
487 | glib::Object<GdkPixbuf> const& pixbuf); |
488 | |
489 | === modified file 'dash/ResultView.cpp' |
490 | --- dash/ResultView.cpp 2012-07-30 16:22:53 +0000 |
491 | +++ dash/ResultView.cpp 2012-08-10 12:37:21 +0000 |
492 | @@ -24,9 +24,7 @@ |
493 | #include "ResultView.h" |
494 | #include "unity-shared/IntrospectableWrappers.h" |
495 | #include <UnityCore/Variant.h> |
496 | -#include <Nux/HLayout.h> |
497 | -#include <Nux/VLayout.h> |
498 | -#include <Nux/Button.h> |
499 | +#include <Nux/Layout.h> |
500 | #include <NuxCore/Logger.h> |
501 | |
502 | namespace unity |
503 | @@ -43,12 +41,10 @@ |
504 | ResultView::ResultView(NUX_FILE_LINE_DECL) |
505 | : View(NUX_FILE_LINE_PARAM) |
506 | , expanded(true) |
507 | - , renderer_(NULL) |
508 | { |
509 | expanded.changed.connect([&](bool value) |
510 | { |
511 | QueueRelayout(); |
512 | - NeedRedraw(); |
513 | }); |
514 | } |
515 | |
516 | @@ -60,28 +56,17 @@ |
517 | { |
518 | renderer_->Unload(result); |
519 | } |
520 | - |
521 | - renderer_->UnReference(); |
522 | } |
523 | |
524 | void ResultView::Draw(nux::GraphicsEngine& GfxContext, bool force_draw) |
525 | -{ |
526 | - |
527 | -} |
528 | +{} |
529 | |
530 | void ResultView::SetModelRenderer(ResultRenderer* renderer) |
531 | { |
532 | - if (renderer_ != NULL) |
533 | - renderer_->UnReference(); |
534 | - |
535 | renderer_ = renderer; |
536 | - renderer->NeedsRedraw.connect([&]() |
537 | - { |
538 | - NeedRedraw(); |
539 | - }); |
540 | - renderer_->SinkReference(); |
541 | + renderer_->NeedsRedraw.connect(sigc::mem_fun(this, &ResultView::QueueDraw)); |
542 | |
543 | - NeedRedraw(); |
544 | + QueueDraw(); |
545 | } |
546 | |
547 | void ResultView::AddResult(Result& result) |
548 | @@ -89,40 +74,44 @@ |
549 | results_.push_back(result); |
550 | renderer_->Preload(result); |
551 | |
552 | - NeedRedraw(); |
553 | + QueueDraw(); |
554 | } |
555 | |
556 | void ResultView::RemoveResult(Result& result) |
557 | { |
558 | - ResultList::iterator it; |
559 | - std::string uri = result.uri; |
560 | + std::string const& uri = result.uri; |
561 | |
562 | - for (it = results_.begin(); it != results_.end(); ++it) |
563 | + for (auto it = results_.begin(); it != results_.end(); ++it) |
564 | { |
565 | - if (result.uri == (*it).uri) |
566 | + if (it->uri == uri) |
567 | { |
568 | results_.erase(it); |
569 | break; |
570 | } |
571 | } |
572 | + |
573 | renderer_->Unload(result); |
574 | } |
575 | |
576 | -ResultView::ResultList ResultView::GetResultList() |
577 | +void ResultView::ClearResults() |
578 | +{ |
579 | + for (Result& result : results_) |
580 | + { |
581 | + renderer_->Unload(result); |
582 | + } |
583 | + |
584 | + results_.clear(); |
585 | +} |
586 | + |
587 | +ResultView::ResultList const& ResultView::GetResultList() const |
588 | { |
589 | return results_; |
590 | } |
591 | |
592 | |
593 | -long ResultView::ComputeContentSize() |
594 | -{ |
595 | - return View::ComputeContentSize(); |
596 | -} |
597 | - |
598 | - |
599 | void ResultView::DrawContent(nux::GraphicsEngine& GfxContent, bool force_draw) |
600 | { |
601 | - nux::Geometry base = GetGeometry(); |
602 | + nux::Geometry const& base = GetGeometry(); |
603 | GfxContent.PushClippingRectangle(base); |
604 | |
605 | if (GetCompositionLayout()) |
606 | |
607 | === modified file 'dash/ResultView.h' |
608 | --- dash/ResultView.h 2012-07-04 02:37:23 +0000 |
609 | +++ dash/ResultView.h 2012-08-10 12:37:21 +0000 |
610 | @@ -25,9 +25,6 @@ |
611 | |
612 | #include <Nux/Nux.h> |
613 | #include <Nux/View.h> |
614 | -#include <dee.h> |
615 | - |
616 | -#include <UnityCore/GLibSignal.h> |
617 | #include <UnityCore/Results.h> |
618 | |
619 | #include "unity-shared/Introspectable.h" |
620 | @@ -51,8 +48,9 @@ |
621 | |
622 | void AddResult(Result& result); |
623 | void RemoveResult(Result& result); |
624 | + void ClearResults(); |
625 | |
626 | - ResultList GetResultList (); |
627 | + ResultList const& GetResultList() const; |
628 | |
629 | nux::Property<bool> expanded; |
630 | nux::Property<int> results_per_row; |
631 | @@ -66,10 +64,9 @@ |
632 | protected: |
633 | virtual void Draw(nux::GraphicsEngine& GfxContext, bool force_draw); |
634 | virtual void DrawContent(nux::GraphicsEngine& GfxContext, bool force_draw); |
635 | - virtual long ComputeContentSize(); |
636 | |
637 | // properties |
638 | - ResultRenderer* renderer_; |
639 | + nux::ObjectPtr<ResultRenderer> renderer_; |
640 | ResultList results_; |
641 | IntrospectableList introspectable_children_; |
642 |
Sorry, but I don't like this one bit, dee_model_get_tag can crash if the passed model is not valid (already unreferenced). IMO guarding against null is not going to help, you'll just push the crash to a different place (in the better case, in worse you'll mask the underlying problem completely and there'll be even more random crashes).