Merge lp:~azzar1/unity/fix-767075 into lp:unity

Proposed by Andrea Azzarone on 2011-06-08
Status: Merged
Merged at revision: 1240
Proposed branch: lp:~azzar1/unity/fix-767075
Merge into: lp:unity
Diff against target: 199 lines (+52/-71)
2 files modified
src/Launcher.cpp (+51/-70)
src/Launcher.h (+1/-1)
To merge this branch: bzr merge lp:~azzar1/unity/fix-767075
Reviewer Review Type Date Requested Status
Jason Smith (community) 2011-06-09 Approve on 2011-06-15
Tim Penhey (community) 2011-06-08 Approve on 2011-06-09
Review via email: mp+63852@code.launchpad.net
To post a comment you must log in.
Tim Penhey (thumper) wrote :

Great work.

Even though you didn't change the geometry param, can I get you to fix it please?

  SetDndDelta (0, center.y, nux::Geometry (geo.x, geo.y, geo.width, geo.height), current);

should be simplified to

  SetDndDelta (0, center.y, geo, current);

We don't need to have yet another constructor.

For bonus points you can change the signature of the SetDndDelta from:

void Launcher::SetDndDelta (float x, float y, nux::Geometry geo, struct timespec const &current)

to

void Launcher::SetDndDelta(float x, float y, nux::Geometry const& geo, timespec const &current)

The const& nux::Geometry parameter means that we don't make an unnecessary copy, and the "struct timespec" can be simplified to just "timespec".

review: Needs Fixing
lp:~azzar1/unity/fix-767075 updated on 2011-06-09
1226. By Andrea Azzarone on 2011-06-09

Applied changes as requested by Tim Penhey.

Tim Penhey (thumper) wrote :

On Thu, 09 Jun 2011 19:26:57 you wrote:
> You have been requested to review the proposed merge of
> lp:~andyrock/unity/fix-767075 into lp:unity.
>
> For more details, see:
> https://code.launchpad.net/~andyrock/unity/fix-767075/+merge/63852

  review approve
  reviewer njpatel

review: Approve
Jason Smith (jassmith) wrote :

Setting 0 is not entirely appropriate. This wont get top edge of a folded icon (which may be obscured anyway?) but is still incorrect. Instead of 0 it needs to track the horizontal offset of the launcher.

review: Needs Fixing
lp:~azzar1/unity/fix-767075 updated on 2011-06-10
1227. By Andrea Azzarone on 2011-06-09

Use the the horizontal offset of the launcher instead of 0.

1228. By Andrea Azzarone on 2011-06-10
Jason Smith (jassmith) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/Launcher.cpp'
2--- src/Launcher.cpp 2011-06-09 17:07:15 +0000
3+++ src/Launcher.cpp 2011-06-10 14:39:15 +0000
4@@ -1109,7 +1109,7 @@
5 }
6 }
7
8-void Launcher::SetDndDelta (float x, float y, nux::Geometry geo, struct timespec const &current)
9+void Launcher::SetDndDelta (float x, float y, nux::Geometry const &geo, timespec const &current)
10 {
11 LauncherIcon *anchor = 0;
12 LauncherModel::iterator it;
13@@ -1508,10 +1508,55 @@
14 if (sum - _space_between_icons <= launcher_height)
15 folding_threshold = launcher_height;
16
17+ float autohide_offset = 0.0f;
18+ *launcher_alpha = 1.0f;
19+ if (_hidemode != LAUNCHER_HIDE_NEVER)
20+ {
21+
22+ float autohide_progress = AutohideProgress (current) * (1.0f - DragOutProgress (current));
23+ if (_autohide_animation == FADE_ONLY
24+ || (_autohide_animation == FADE_OR_SLIDE && _hide_machine->GetQuirk (LauncherHideMachine::MOUSE_OVER_BFB)))
25+ *launcher_alpha = 1.0f - autohide_progress;
26+ else
27+ {
28+ if (autohide_progress > 0.0f)
29+ {
30+ autohide_offset -= geo.width * autohide_progress;
31+ if (_autohide_animation == FADE_AND_SLIDE)
32+ *launcher_alpha = 1.0f - 0.5f * autohide_progress;
33+ }
34+ }
35+ }
36+
37+ float drag_hide_progress = DragHideProgress (current);
38+ if (_hidemode != LAUNCHER_HIDE_NEVER && drag_hide_progress > 0.0f)
39+ {
40+ autohide_offset -= geo.width * 0.25f * drag_hide_progress;
41+
42+ if (drag_hide_progress >= 1.0f)
43+ _hide_machine->SetQuirk (LauncherHideMachine::DND_PUSHED_OFF, true);
44+ }
45+
46+ // Inform the painter where to paint the box
47+ box_geo = geo;
48+
49+ if (_hidemode != LAUNCHER_HIDE_NEVER)
50+ box_geo.x += autohide_offset;
51+
52+ /* Why we need last_geo? It stores the last box_geo (note: as it is a static variable,
53+ * it is initialized only first time). Infact we call SetDndDelta that calls MouseIconIntersection
54+ * that uses values (HitArea) that are computed in UpdateIconXForm.
55+ * The problem is that in DrawContent we calls first RenderArgs, then UpdateIconXForm. Just
56+ * use last_geo to hack this problem.
57+ */
58+ static nux::Geometry last_geo = box_geo;
59+
60 // this happens on hover, basically its a flag and a value in one, we translate this into a dnd offset
61 if (_enter_y != 0 && _enter_y + _icon_size / 2 > folding_threshold)
62- SetDndDelta (center.x, center.y, nux::Geometry (geo.x, geo.y, geo.width, geo.height), current);
63+ SetDndDelta (last_geo.x + last_geo.width / 2, center.y, geo, current);
64
65+ // Update the last_geo value.
66+ last_geo = box_geo;
67 _enter_y = 0;
68
69 if (hover_progress > 0.0f && _launcher_drag_delta != 0)
70@@ -1550,41 +1595,6 @@
71 _launcher_drag_delta = 0;
72 }
73
74- float autohide_offset = 0.0f;
75- *launcher_alpha = 1.0f;
76- if (_hidemode != LAUNCHER_HIDE_NEVER)
77- {
78-
79- float autohide_progress = AutohideProgress (current) * (1.0f - DragOutProgress (current));
80- if (_autohide_animation == FADE_ONLY
81- || (_autohide_animation == FADE_OR_SLIDE && _hide_machine->GetQuirk (LauncherHideMachine::MOUSE_OVER_BFB)))
82- *launcher_alpha = 1.0f - autohide_progress;
83- else
84- {
85- if (autohide_progress > 0.0f)
86- {
87- autohide_offset -= geo.width * autohide_progress;
88- if (_autohide_animation == FADE_AND_SLIDE)
89- *launcher_alpha = 1.0f - 0.5f * autohide_progress;
90- }
91- }
92- }
93-
94- float drag_hide_progress = DragHideProgress (current);
95- if (_hidemode != LAUNCHER_HIDE_NEVER && drag_hide_progress > 0.0f)
96- {
97- autohide_offset -= geo.width * 0.25f * drag_hide_progress;
98-
99- if (drag_hide_progress >= 1.0f)
100- _hide_machine->SetQuirk (LauncherHideMachine::DND_PUSHED_OFF, true);
101- }
102-
103- // Inform the painter where to paint the box
104- box_geo = geo;
105-
106- if (_hidemode != LAUNCHER_HIDE_NEVER)
107- box_geo.x += autohide_offset;
108-
109 // The functional position we wish to represent for these icons is not smooth. Rather than introducing
110 // special casing to represent this, we use MIN/MAX functions. This helps ensure that even though our
111 // function is not smooth it is continuous, which is more important for our visual representation (icons
112@@ -2786,7 +2796,6 @@
113 nux::Geometry bkg_box;
114 std::list<Launcher::RenderArg> args;
115 std::list<Launcher::RenderArg>::reverse_iterator rev_it;
116- std::list<Launcher::RenderArg>::iterator it;
117 float launcher_alpha = 1.0f;
118
119 // rely on the compiz event loop to come back to us in a nice throttling
120@@ -2829,23 +2838,11 @@
121 nux::Geometry (bkg_box.x, (*rev_it).render_center.y - 3, bkg_box.width, 2),
122 nux::Color(0xAAAAAAAA));
123
124- if ((*rev_it).x_rotation >= 0.0f || (*rev_it).skip)
125+ if ((*rev_it).skip)
126 continue;
127
128 DrawRenderArg (GfxContext, *rev_it, bkg_box);
129 }
130-
131- for (it = args.begin(); it != args.end(); it++)
132- {
133- if ((*it).stick_thingy)
134- gPainter.Paint2DQuadColor (GfxContext,
135- nux::Geometry (bkg_box.x, (*it).render_center.y - 3, bkg_box.width, 2),
136- nux::Color(0xAAAAAAAA));
137- if ((*it).x_rotation < 0.0f || (*it).skip)
138- continue;
139-
140- DrawRenderArg (GfxContext, *it, bkg_box);
141- }
142
143 gPainter.Paint2DQuadColor(GfxContext,
144 nux::Geometry(bkg_box.x + bkg_box.width - 1,
145@@ -3256,6 +3253,8 @@
146 void
147 Launcher::EdgeRevealTriggered ()
148 {
149+ SetMousePosition (pointerX, pointerY - GetAbsoluteGeometry ().y);
150+
151 _hide_machine->SetQuirk (LauncherHideMachine::MOUSE_OVER_ACTIVE_EDGE, true);
152 _hide_machine->SetQuirk (LauncherHideMachine::MOUSE_MOVE_POST_REVEAL, true);
153 }
154@@ -3475,31 +3474,13 @@
155 LauncherIcon* Launcher::MouseIconIntersection (int x, int y)
156 {
157 LauncherModel::iterator it;
158- LauncherModel::reverse_iterator rev_it;
159 // We are looking for the icon at screen coordinates x, y;
160 nux::Point2 mouse_position(x, y);
161 int inside = 0;
162
163- // Because of the way icons fold and stack on one another, we must proceed in 2 steps.
164- for (rev_it = _model->rbegin (); rev_it != _model->rend (); rev_it++)
165- {
166- if ((*rev_it)->_folding_angle < 0.0f || !(*rev_it)->GetQuirk (LauncherIcon::QUIRK_VISIBLE))
167- continue;
168-
169- nux::Point2 screen_coord [4];
170- for (int i = 0; i < 4; i++)
171- {
172- screen_coord [i].x = (*rev_it)->_xform_coords["HitArea"] [i].x;
173- screen_coord [i].y = (*rev_it)->_xform_coords["HitArea"] [i].y;
174- }
175- inside = PointInside2DPolygon (screen_coord, 4, mouse_position, 1);
176- if (inside)
177- return (*rev_it);
178- }
179-
180 for (it = _model->begin(); it != _model->end (); it++)
181 {
182- if ((*it)->_folding_angle >= 0.0f || !(*it)->GetQuirk (LauncherIcon::QUIRK_VISIBLE))
183+ if (!(*it)->GetQuirk (LauncherIcon::QUIRK_VISIBLE))
184 continue;
185
186 nux::Point2 screen_coord [4];
187
188=== modified file 'src/Launcher.h'
189--- src/Launcher.h 2011-04-20 03:59:36 +0000
190+++ src/Launcher.h 2011-06-10 14:39:15 +0000
191@@ -308,7 +308,7 @@
192 void SetHover (bool hovered);
193 void SetHidden (bool hidden);
194
195- void SetDndDelta (float x, float y, nux::Geometry geo, struct timespec const &current);
196+ void SetDndDelta (float x, float y, nux::Geometry const &geo, timespec const &current);
197 float DragLimiter (float x);
198
199 void SetupRenderArg (LauncherIcon *icon, struct timespec const &current, RenderArg &arg);