Merge lp:~mhr3/unity/respect-icon-size-hint into lp:unity

Proposed by Michal Hruby on 2012-10-10
Status: Merged
Approved by: Michal Hruby on 2012-10-10
Approved revision: 2818
Merged at revision: 2820
Proposed branch: lp:~mhr3/unity/respect-icon-size-hint
Merge into: lp:unity
Diff against target: 82 lines (+38/-10)
1 file modified
unity-shared/IconLoader.cpp (+38/-10)
To merge this branch: bzr merge lp:~mhr3/unity/respect-icon-size-hint
Reviewer Review Type Date Requested Status
Nick Dedekind (community) 2012-10-10 Approve on 2012-10-10
jenkins continuous-integration Pending
Review via email: mp+128899@code.launchpad.net

Commit message

Respect use-small-icon hint set on UnityProtocolAnnotatedIcon

Description of the change

Respect icon size hint set on UnityProtocolAnnotatedIcon - if the hint is set, the icon will be rendered at 2/3 of the requested size, yet the ribbon will be painted at full size of the icon.

To post a comment you must log in.
Michal Hruby (mhr3) wrote :

Screenshot of how apps lens' "More suggestions" will look like with this: http://ubuntuone.com/2fUNZHBaOlK2eoJ22porzK

Nick Dedekind (nick-dedekind) wrote :

Code-wise, looks ok; and even though it's a pretty nasty hack, I don't see any other way around it.

But on a side note:

I was under the impression that using libunity private is strictly limited to UnityCore? In which case this class should be moved there.

struct IconLoaderTask

This should be a class, not a struct. And it's starting to get confusing with members vs locals. From the diff it doesn't look like this should compile because you changed the max_width to base_max_width, but are still using max_width in the function. Members should have trailing '_'. I realise it's not part of the bug, but it could definitely use an code refactor.

review: Approve
Michal Hruby (mhr3) wrote :

+1 on the need of refactoring IconLoader*, but since this is intended for 6.0 series too, let's keep it simple so far.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'unity-shared/IconLoader.cpp'
2--- unity-shared/IconLoader.cpp 2012-09-28 08:30:15 +0000
3+++ unity-shared/IconLoader.cpp 2012-10-10 10:30:27 +0000
4@@ -212,15 +212,27 @@
5
6 if (icon.IsType(UNITY_PROTOCOL_TYPE_ANNOTATED_ICON))
7 {
8- UnityProtocolAnnotatedIcon *anno;
9- anno = UNITY_PROTOCOL_ANNOTATED_ICON(icon.RawPtr());
10+ glib::Object<UnityProtocolAnnotatedIcon> anno(glib::object_cast<UnityProtocolAnnotatedIcon>(icon));
11 GIcon* base_icon = unity_protocol_annotated_icon_get_icon(anno);
12 glib::String gicon_string(g_icon_to_string(base_icon));
13
14+ // ensure that annotated icons aren't cached by the IconLoader
15 no_cache = true;
16- auto helper_slot = sigc::bind(sigc::mem_fun(this, &IconLoaderTask::BaseIconLoaded), glib::object_cast<UnityProtocolAnnotatedIcon>(icon));
17- int base_icon_width = max_width > 0 ? max_width - RIBBON_PADDING * 2 : -1;
18- int base_icon_height = base_icon_width < 0 ? max_height - RIBBON_PADDING *2 : max_height;
19+
20+ auto helper_slot = sigc::bind(sigc::mem_fun(this, &IconLoaderTask::BaseIconLoaded), anno);
21+ int base_icon_width, base_icon_height;
22+ if (unity_protocol_annotated_icon_get_use_small_icon(anno))
23+ {
24+ // FIXME: although this pretends to be generic, we're just making
25+ // sure that icons requested to have 96px will be 64
26+ base_icon_width = max_width > 0 ? max_width * 2 / 3 : max_width;
27+ base_icon_height = max_height > 0 ? max_height * 2 / 3 : max_height;
28+ }
29+ else
30+ {
31+ base_icon_width = max_width > 0 ? max_width - RIBBON_PADDING * 2 : -1;
32+ base_icon_height = base_icon_width < 0 ? max_height - RIBBON_PADDING *2 : max_height;
33+ }
34 helper_handle = impl->LoadFromGIconString(gicon_string.Str(),
35 base_icon_width,
36 base_icon_height,
37@@ -518,7 +530,7 @@
38 }
39
40 void BaseIconLoaded(std::string const& base_icon_string,
41- int max_width, int max_height,
42+ int base_max_width, int base_max_height,
43 glib::Object<GdkPixbuf> const& base_pixbuf,
44 glib::Object<UnityProtocolAnnotatedIcon> const& anno_icon)
45 {
46@@ -529,16 +541,32 @@
47 "' size: " << gdk_pixbuf_get_width(base_pixbuf) << 'x' <<
48 gdk_pixbuf_get_height(base_pixbuf);
49
50+ int result_width, result_height, dest_x, dest_y;
51+ if (unity_protocol_annotated_icon_get_use_small_icon(anno_icon))
52+ {
53+ result_width = max_width > 0 ? max_width : MAX(max_height, gdk_pixbuf_get_width(base_pixbuf));
54+ result_height = max_height > 0 ? max_height : MAX(max_width, gdk_pixbuf_get_height(base_pixbuf));
55+
56+ dest_x = (result_width - gdk_pixbuf_get_width(base_pixbuf)) / 2;
57+ dest_y = (result_height - gdk_pixbuf_get_height(base_pixbuf)) / 2;
58+ }
59+ else
60+ {
61+ result_width = gdk_pixbuf_get_width(base_pixbuf) + RIBBON_PADDING * 2;
62+ result_height = gdk_pixbuf_get_height(base_pixbuf);
63+
64+ dest_x = RIBBON_PADDING;
65+ dest_y = 0;
66+ }
67 // we need extra space for the ribbon
68- result = gdk_pixbuf_new(GDK_COLORSPACE_RGB, TRUE, 8,
69- gdk_pixbuf_get_width(base_pixbuf) + RIBBON_PADDING * 2,
70- gdk_pixbuf_get_height(base_pixbuf));
71+ result = gdk_pixbuf_new(GDK_COLORSPACE_RGB, TRUE, 8,
72+ result_width, result_height);
73 gdk_pixbuf_fill(result, 0x0);
74 gdk_pixbuf_copy_area(base_pixbuf, 0, 0,
75 gdk_pixbuf_get_width(base_pixbuf),
76 gdk_pixbuf_get_height(base_pixbuf),
77 result,
78- RIBBON_PADDING, 0);
79+ dest_x, dest_y);
80 // FIXME: can we composite the pixbuf in helper thread?
81 UnityProtocolCategoryType category = unity_protocol_annotated_icon_get_category(anno_icon);
82 auto helper_slot = sigc::bind(sigc::mem_fun(this, &IconLoaderTask::CategoryIconLoaded), anno_icon);