Merge lp:~dandrader/frame/smarter_backend into lp:frame
- smarter_backend
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 92 |
Proposed branch: | lp:~dandrader/frame/smarter_backend |
Merge into: | lp:frame |
Diff against target: |
691 lines (+301/-99) 10 files modified
include/oif/frame.h (+1/-0) include/oif/frame_backend.h (+49/-12) src/frame.cpp (+70/-11) src/frame.h (+4/-4) src/libframe.ver (+4/-3) src/touch.cpp (+75/-59) src/touch.h (+7/-2) src/window.h (+1/-1) src/x11/window_x11.cpp (+2/-2) test/regular/backend.cpp (+88/-5) |
To merge this branch: | bzr merge lp:~dandrader/frame/smarter_backend |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Chase Douglas (community) | Approve | ||
Review via email: mp+122092@code.launchpad.net |
Commit message
Description of the change
Add some helper functions to the backend API and lazy copy for touches.
The new functions are:
- frame_backend_
- frame_backend_
With those functions, backend implementors can more easily write efficient and error-free implementations.
Daniel d'Andrada (dandrader) wrote : | # |
> * Any functions that may fail need to return a UFStatus and provide the return
> result through a referenced parameter. All of the new functions here should do
> this. I forgot about this when I approved the previous merge proposal as well.
> We should go back and fix these up.
Isn't that extra status variable redundant in those cases? What's the benefit?
Chase Douglas (chasedouglas) wrote : | # |
On 08/30/2012 11:01 AM, Daniel d'Andrada wrote:
>> * Any functions that may fail need to return a UFStatus and provide the return
>> result through a referenced parameter. All of the new functions here should do
>> this. I forgot about this when I approved the previous merge proposal as well.
>> We should go back and fix these up.
>
> Isn't that extra status variable redundant in those cases? What's the benefit?
Let's take a function like:
UFBackendTouch frame_backend_
UFBackendFrame frame, UFTouchId id)
Without knowing exactly how this is implemented, there are two possible
recoverable error conditions:
1. The touch ID is invalid (UFStatusErrorI
2. The allocation and initialization of UFBackendTouch could fail
(UFStatusErrorR
The current implementation doesn't actually catch allocation and
initialization exceptions, but it could if we found that to be a problem.
The client could use this information to determine how to proceed. It's
almost impossible to recover from allocation errors, so the client could
abort. However, if the touch ID is invalid, it could stop processing
gestures for that frame and continue on. At the very least, it could log
a more meaningful error message, which has been helpful in debugging
issues in the past.
The other main reason is that it maintains the cohesiveness of the API.
The entire grail and frame API behaves like this. It makes sense to
continue using the same mechanism.
I'm not a huge fan of the API, tbh. I much prefer an exception based
approach. However, this is C, and there's only so much you can do with a
C API. There is one benefit to the API, however, and it becomes when you
actively check the return value.
UFBackendTouch be_touch;
assert(
As opposed to:
UFBackendTouch be_touch;
assert((be_touch = frame_backend_
nullptr);
My personal opinion is that the second approach is messy because you are
both assigning a value and using C's () operator to check the assigned
value is correct, all in one statement of code. You could split it up,
but then you're using more lines of code and it isn't any more
comprehensible versus the original approach, imo.
Daniel d'Andrada (dandrader) wrote : | # |
> On 08/30/2012 11:01 AM, Daniel d'Andrada wrote:
> >> * Any functions that may fail need to return a UFStatus and provide the
> return
> >> result through a referenced parameter. All of the new functions here should
> do
> >> this. I forgot about this when I approved the previous merge proposal as
> well.
> >> We should go back and fix these up.
> >
> > Isn't that extra status variable redundant in those cases? What's the
> benefit?
>
> Let's take a function like:
>
> UFBackendTouch frame_backend_
> UFBackendFrame frame, UFTouchId id)
>
> Without knowing exactly how this is implemented, there are two possible
> recoverable error conditions:
>
> 1. The touch ID is invalid (UFStatusErrorI
> 2. The allocation and initialization of UFBackendTouch could fail
> (UFStatusErrorR
>
> The current implementation doesn't actually catch allocation and
> initialization exceptions, but it could if we found that to be a problem.
>
> The client could use this information to determine how to proceed. It's
> almost impossible to recover from allocation errors, so the client could
> abort. However, if the touch ID is invalid, it could stop processing
> gestures for that frame and continue on. At the very least, it could log
> a more meaningful error message, which has been helpful in debugging
> issues in the past.
You're right.
Thanks for the explanation!
Daniel d'Andrada (dandrader) wrote : | # |
Updated
- 96. By Daniel d'Andrada
-
Plug a memory leak in test Backend:
:FrameCreateNex t
Chase Douglas (chasedouglas) wrote : | # |
* In frame_backend_
* In frame_backend_
Otherwise, I'm ok with everything. It's not exactly how I would do it, but I don't know of a 100% best alternative either.
- 97. By Daniel d'Andrada
-
Minor tweaks
- 98. By Daniel d'Andrada
-
A bit more tests
Preview Diff
1 | === modified file 'include/oif/frame.h' |
2 | --- include/oif/frame.h 2012-07-24 21:36:05 +0000 |
3 | +++ include/oif/frame.h 2012-08-31 19:29:21 +0000 |
4 | @@ -89,6 +89,7 @@ |
5 | UFStatusErrorInvalidType, /**< The variable type passed as a void pointer into |
6 | a property getter is invalid for the property |
7 | */ |
8 | + UFStatusErrorTouchIdExists, /**< A touch with the same ID already exists */ |
9 | } UFStatus; |
10 | |
11 | /** Properties of a device */ |
12 | |
13 | === modified file 'include/oif/frame_backend.h' |
14 | --- include/oif/frame_backend.h 2012-08-28 19:54:18 +0000 |
15 | +++ include/oif/frame_backend.h 2012-08-31 19:29:21 +0000 |
16 | @@ -132,18 +132,53 @@ |
17 | ********************************************************************/ |
18 | |
19 | /** |
20 | - * Creates a new UFFrame and returns its backend handle. |
21 | + * Creates a new, empty, UFFrame and returns its backend handle. |
22 | + * |
23 | + * Usually you will use this method only for the very first frame. For all |
24 | + * subsequent ones it will be safer and more convinent to use |
25 | + * frame_backend_frame_create_next(). |
26 | */ |
27 | FRAME_PUBLIC |
28 | UFBackendFrame frame_backend_frame_new(); |
29 | |
30 | /** |
31 | + * Creates a new UFFrame that is a continuation of the given one. |
32 | + * |
33 | + * Touches that had a "begin" state on the given frame will be hard-copied and |
34 | + * have an "update" state on the new frame. |
35 | + * |
36 | + * Touches that had an "update" state will be lazily copied to the new frame. |
37 | + * |
38 | + * Touches that had a "end" state on the given frame won't be present |
39 | + * on the new frame. |
40 | + */ |
41 | +FRAME_PUBLIC |
42 | +UFBackendFrame frame_backend_frame_create_next(UFBackendFrame frame); |
43 | + |
44 | +/** |
45 | * Returns a UFFrame instance given its backend handle. |
46 | */ |
47 | FRAME_PUBLIC |
48 | UFFrame frame_backend_frame_get_frame(UFBackendFrame frame); |
49 | |
50 | /** |
51 | + * Gets a UFBackendTouch for the UFTouch that has the given id. |
52 | + * |
53 | + * The underlying UFTouch is moved from the given frame to the returned UFBackendTouch. |
54 | + * Once done modifying the touch you're expected to return it to the frame via |
55 | + * frame_backend_frame_give_touch(). |
56 | + * |
57 | + * If the underlying UFTouch is a lazy copy (likely from a touch in the previous frame), a hard copy |
58 | + * will be made upon the first change made to it. |
59 | + * |
60 | + * Possible errors: UFStatusErrorInvalidTouch |
61 | + */ |
62 | +FRAME_PUBLIC |
63 | +UFStatus frame_backend_frame_borrow_touch_by_id(UFBackendFrame frame, |
64 | + UFTouchId id, |
65 | + UFBackendTouch *touch); |
66 | + |
67 | +/** |
68 | * Sets the "Device" property of the given frame |
69 | */ |
70 | FRAME_PUBLIC |
71 | @@ -164,10 +199,15 @@ |
72 | void frame_backend_frame_set_active_touches(UFBackendFrame frame, unsigned int active_touches); |
73 | |
74 | /** |
75 | - * Adds a UFTouch to the given frame. |
76 | + * Gives a UFTouch to the specified frame. |
77 | + * |
78 | + * Gives the underlying UFTouch to the specified frame. The UFBackendTouch |
79 | + * is deleted and no longer valid after this call. |
80 | + * |
81 | + * Possible errors: UFStatusErrorTouchIdExists |
82 | */ |
83 | FRAME_PUBLIC |
84 | -void frame_backend_frame_add_touch(UFBackendFrame frame, UFBackendTouch touch); |
85 | +UFStatus frame_backend_frame_give_touch(UFBackendFrame frame, UFBackendTouch *touch); |
86 | |
87 | /** |
88 | * Deletes the backend handle of a UFFrame, |
89 | @@ -182,6 +222,10 @@ |
90 | |
91 | /** |
92 | * Creates a new UFTouch and returns its backend handle. |
93 | + * |
94 | + * Its state will be set to "Begin". |
95 | + * |
96 | + * After filled out, it should be given to a frame via frame_backend_frame_give_touch() |
97 | */ |
98 | FRAME_PUBLIC |
99 | UFBackendTouch frame_backend_touch_new(); |
100 | @@ -199,10 +243,10 @@ |
101 | void frame_backend_touch_set_id(UFBackendTouch touch, UFTouchId id); |
102 | |
103 | /** |
104 | - * Sets the "State" property of the given touch |
105 | + * Sets the "State" property of the given touch to "End" |
106 | */ |
107 | FRAME_PUBLIC |
108 | -void frame_backend_touch_set_state(UFBackendTouch touch, UFTouchState state); |
109 | +void frame_backend_touch_set_ended(UFBackendTouch touch); |
110 | |
111 | /** |
112 | * Sets the "WindowX" and "WindowY" properties of the given touch |
113 | @@ -240,13 +284,6 @@ |
114 | FRAME_PUBLIC |
115 | void frame_backend_touch_set_value(UFBackendTouch touch, UFAxisType type, float value); |
116 | |
117 | -/** |
118 | - * Deletes the backend handle of a UFTouch, |
119 | - * decreasing its reference count by one. |
120 | - */ |
121 | -FRAME_PUBLIC |
122 | -void frame_backend_touch_delete(UFBackendTouch touch); |
123 | - |
124 | #ifdef __cplusplus |
125 | } |
126 | #endif |
127 | |
128 | === modified file 'src/frame.cpp' |
129 | --- src/frame.cpp 2012-08-29 18:54:16 +0000 |
130 | +++ src/frame.cpp 2012-08-31 19:29:21 +0000 |
131 | @@ -25,16 +25,14 @@ |
132 | |
133 | #include "device.h" |
134 | #include "touch.h" |
135 | -#include "window.h" |
136 | |
137 | #include <oif/frame_backend.h> |
138 | |
139 | namespace oif { |
140 | namespace frame { |
141 | |
142 | -UFFrame::UFFrame(const SharedWindow& window, const SharedUFFrame& prev) |
143 | +UFFrame::UFFrame(const SharedUFFrame& prev) |
144 | : prev_(prev), |
145 | - window_(window), |
146 | touches_array_(), |
147 | touches_map_() { |
148 | for (const SharedUFTouch& prev_touch : prev_->touches_array_) { |
149 | @@ -96,10 +94,28 @@ |
150 | |
151 | } // namespace |
152 | |
153 | -void UFFrame::AddTouch(const SharedUFTouch& touch) |
154 | -{ |
155 | - touches_map_[touch->id()] = touches_array_.size(); |
156 | - touches_array_.push_back(touch); |
157 | +UFStatus UFFrame::GiveTouch(SharedUFTouch& touch) { |
158 | + auto map_it = touches_map_.find(touch->id()); |
159 | + if (map_it != touches_map_.end()) { |
160 | + /* A loan is being returned */ |
161 | + |
162 | + SharedUFTouch &our_touch = touches_array_[map_it->second]; |
163 | + |
164 | + /* we shouldn't be referencing to any UFTouch here. |
165 | + If we are, it's a programming error by the API user. */ |
166 | + if (our_touch.get() != NULL) { |
167 | + return UFStatusErrorTouchIdExists; |
168 | + } |
169 | + |
170 | + /* take back our UFtouch */ |
171 | + our_touch.swap(touch); |
172 | + } else { |
173 | + touches_map_[touch->id()] = touches_array_.size(); |
174 | + touches_array_.push_back(touch); |
175 | + touch.reset(); |
176 | + } |
177 | + |
178 | + return UFStatusSuccess; |
179 | } |
180 | |
181 | void UFFrame::UpdateTouch(const SharedUFTouch& touch) { |
182 | @@ -111,7 +127,8 @@ |
183 | CopyStartTime(touches_array_[map_it->second], touch); |
184 | touches_array_[map_it->second] = touch; |
185 | } else { |
186 | - AddTouch(touch); |
187 | + touches_map_[touch->id()] = touches_array_.size(); |
188 | + touches_array_.push_back(touch); |
189 | } |
190 | break; |
191 | |
192 | @@ -171,6 +188,14 @@ |
193 | return UFStatusSuccess; |
194 | } |
195 | |
196 | +SharedUFTouch* UFFrame::GetSharedTouchById(UFTouchId touch_id) { |
197 | + auto it = touches_map_.find(touch_id); |
198 | + if (it == touches_map_.end()) |
199 | + return nullptr; |
200 | + else |
201 | + return &(touches_array_[it->second]); |
202 | +} |
203 | + |
204 | UFStatus UFFrame::GetTouchById(UFTouchId touch_id, ::UFTouch* touch) const { |
205 | auto it = touches_map_.find(touch_id); |
206 | if (it == touches_map_.end()) |
207 | @@ -316,11 +341,32 @@ |
208 | return new UFBackendFrame_(new oif::frame::UFFrame); |
209 | } |
210 | |
211 | +UFBackendFrame frame_backend_frame_create_next(UFBackendFrame frame) |
212 | +{ |
213 | + return new UFBackendFrame_(new oif::frame::UFFrame(frame->shared_ptr)); |
214 | +} |
215 | + |
216 | UFFrame frame_backend_frame_get_frame(UFBackendFrame frame) |
217 | { |
218 | return frame->shared_ptr.get(); |
219 | } |
220 | |
221 | +UFStatus frame_backend_frame_borrow_touch_by_id(UFBackendFrame frame, |
222 | + UFTouchId id, |
223 | + UFBackendTouch *touch) |
224 | +{ |
225 | + oif::frame::UFFrame* ufframe = |
226 | + static_cast<oif::frame::UFFrame*>(frame->shared_ptr.get()); |
227 | + |
228 | + oif::frame::SharedUFTouch *shared_touch = ufframe->GetSharedTouchById(id); |
229 | + if (shared_touch) { |
230 | + *touch = new UFBackendTouch_(*shared_touch); |
231 | + return UFStatusSuccess; |
232 | + } else { |
233 | + return UFStatusErrorInvalidTouch; |
234 | + } |
235 | +} |
236 | + |
237 | void frame_backend_frame_set_device(UFBackendFrame frame, UFBackendDevice device) |
238 | { |
239 | static_cast<oif::frame::UFFrame*>(frame->shared_ptr.get())-> |
240 | @@ -342,10 +388,23 @@ |
241 | new oif::frame::Value(active_touches)); |
242 | } |
243 | |
244 | -void frame_backend_frame_add_touch(UFBackendFrame frame, UFBackendTouch touch) |
245 | +UFStatus frame_backend_frame_give_touch(UFBackendFrame frame, UFBackendTouch *touch) |
246 | { |
247 | - static_cast<oif::frame::UFFrame*>(frame->shared_ptr.get())-> |
248 | - AddTouch(touch->shared_ptr); |
249 | + UFStatus status; |
250 | + |
251 | + /* the touch backend must be carrying a UFtouch */ |
252 | + assert((*touch)->shared_ptr); |
253 | + |
254 | + status = static_cast<oif::frame::UFFrame*>(frame->shared_ptr.get())-> |
255 | + GiveTouch((*touch)->shared_ptr); |
256 | + |
257 | + /* frame must have taken the touch */ |
258 | + assert(!(*touch)->shared_ptr); |
259 | + |
260 | + delete (*touch); |
261 | + *touch = nullptr; |
262 | + |
263 | + return status; |
264 | } |
265 | |
266 | void frame_backend_frame_delete(UFBackendFrame frame) |
267 | |
268 | === modified file 'src/frame.h' |
269 | --- src/frame.h 2012-08-28 19:51:37 +0000 |
270 | +++ src/frame.h 2012-08-31 19:29:21 +0000 |
271 | @@ -45,12 +45,12 @@ |
272 | |
273 | class UFFrame : public UFFrame_, public Property<UFFrameProperty> { |
274 | public: |
275 | - UFFrame() : prev_(), window_(), touches_array_(), touches_map_() {} |
276 | - UFFrame(const SharedWindow& window, const SharedUFFrame& prev); |
277 | + UFFrame() : prev_(), touches_array_(), touches_map_() {} |
278 | + UFFrame(const SharedUFFrame& prev); |
279 | |
280 | UFTouch* CopyTouch(UFTouchId touchid, UFTouchState new_state) const; |
281 | bool IsTouchOwned(UFTouchId touchid); |
282 | - void AddTouch(const SharedUFTouch& touch); |
283 | + UFStatus GiveTouch(SharedUFTouch& touch); |
284 | void UpdateTouch(const SharedUFTouch& touch); |
285 | bool IsEnded() const; |
286 | unsigned int GetNumTouches() const { return touches_array_.size(); } |
287 | @@ -59,6 +59,7 @@ |
288 | UFStatus GetPreviousTouchProperty(const UFTouch* touch, |
289 | UFTouchProperty property, void* value) const; |
290 | UFStatus GetTouchByIndex(unsigned int index, ::UFTouch* touch) const; |
291 | + SharedUFTouch* GetSharedTouchById(UFTouchId touch_id); |
292 | UFStatus GetTouchById(UFTouchId touch_id, ::UFTouch* touch) const; |
293 | void ReleasePreviousFrame(); |
294 | |
295 | @@ -67,7 +68,6 @@ |
296 | |
297 | private: |
298 | SharedUFFrame prev_; |
299 | - SharedWindow window_; |
300 | std::vector<SharedUFTouch> touches_array_; |
301 | std::map<UFTouchId, unsigned int> touches_map_; |
302 | }; |
303 | |
304 | === modified file 'src/libframe.ver' |
305 | --- src/libframe.ver 2012-08-28 19:51:37 +0000 |
306 | +++ src/libframe.ver 2012-08-31 19:29:21 +0000 |
307 | @@ -77,23 +77,24 @@ |
308 | frame_backend_device_set_window_resolution; |
309 | frame_backend_device_add_axis; |
310 | frame_backend_frame_new; |
311 | + frame_backend_frame_create_next; |
312 | frame_backend_frame_get_frame; |
313 | + frame_backend_frame_borrow_touch_by_id; |
314 | frame_backend_frame_set_device; |
315 | frame_backend_frame_set_window_id; |
316 | frame_backend_frame_set_active_touches; |
317 | - frame_backend_frame_add_touch; |
318 | + frame_backend_frame_give_touch; |
319 | frame_backend_frame_delete; |
320 | frame_backend_touch_new; |
321 | frame_backend_touch_get_touch; |
322 | frame_backend_touch_set_id; |
323 | - frame_backend_touch_set_state; |
324 | + frame_backend_touch_set_ended; |
325 | frame_backend_touch_set_window_pos; |
326 | frame_backend_touch_set_time; |
327 | frame_backend_touch_set_start_time; |
328 | frame_backend_touch_set_owned; |
329 | frame_backend_touch_set_pending_end; |
330 | frame_backend_touch_set_value; |
331 | - frame_backend_touch_delete; |
332 | |
333 | local: |
334 | *; |
335 | |
336 | === modified file 'src/touch.cpp' |
337 | --- src/touch.cpp 2012-08-28 18:20:51 +0000 |
338 | +++ src/touch.cpp 2012-08-31 19:29:21 +0000 |
339 | @@ -26,9 +26,32 @@ |
340 | |
341 | #include <oif/frame_backend.h> |
342 | |
343 | +oif::frame::UFTouch* UFBackendTouch_::GetModifiableTouch() { |
344 | + if (shared_ptr.unique()) { |
345 | + return static_cast<oif::frame::UFTouch*>(shared_ptr.get()); |
346 | + } else { |
347 | + /* Make a hard-copy. We don't want other holders of that UFTouch (like frames |
348 | + from previous but still existing events) to get the changes about to be |
349 | + made through this UFBackendTouch. */ |
350 | + oif::frame::UFTouch *old_touch = |
351 | + static_cast<oif::frame::UFTouch*>(shared_ptr.get()); |
352 | + oif::frame::UFTouch *new_touch = new oif::frame::UFTouch(*old_touch); |
353 | + shared_ptr.reset(new_touch); |
354 | + return new_touch; |
355 | + } |
356 | +} |
357 | + |
358 | namespace oif { |
359 | namespace frame { |
360 | |
361 | +UFTouch::UFTouch() |
362 | + : state_(UFTouchStateBegin) { |
363 | + const Value* value; |
364 | + |
365 | + value = new Value(state_); |
366 | + InsertProperty(UFTouchPropertyState, value); |
367 | +} |
368 | + |
369 | UFTouch::UFTouch(UFTouchState state, UFTouchId id, float x, float y, |
370 | uint64_t time) |
371 | : id_(id), |
372 | @@ -219,70 +242,63 @@ |
373 | |
374 | void frame_backend_touch_set_id(UFBackendTouch touch_backend, UFTouchId id) |
375 | { |
376 | - oif::frame::UFTouch *touch = |
377 | - static_cast<oif::frame::UFTouch*>(touch_backend->shared_ptr.get()); |
378 | + oif::frame::UFTouch *touch = touch_backend->GetModifiableTouch(); |
379 | |
380 | touch->InsertProperty(UFTouchPropertyId, new oif::frame::Value(id)); |
381 | touch->SetId(id); |
382 | } |
383 | |
384 | -void frame_backend_touch_set_state(UFBackendTouch touch_backend, UFTouchState state) |
385 | -{ |
386 | - oif::frame::UFTouch *touch = |
387 | - static_cast<oif::frame::UFTouch*>(touch_backend->shared_ptr.get()); |
388 | - |
389 | - touch->InsertProperty(UFTouchPropertyState, new oif::frame::Value(state)); |
390 | - touch->SetState(state); |
391 | -} |
392 | - |
393 | -void frame_backend_touch_set_window_pos(UFBackendTouch touch, float x, float y) |
394 | -{ |
395 | - static_cast<oif::frame::UFTouch*>(touch->shared_ptr.get())-> |
396 | - InsertProperty(UFTouchPropertyWindowX, |
397 | - new oif::frame::Value(x)); |
398 | - |
399 | - static_cast<oif::frame::UFTouch*>(touch->shared_ptr.get())-> |
400 | - InsertProperty(UFTouchPropertyWindowY, |
401 | - new oif::frame::Value(y)); |
402 | -} |
403 | - |
404 | -void frame_backend_touch_set_time(UFBackendTouch touch, uint64_t time) |
405 | -{ |
406 | - static_cast<oif::frame::UFTouch*>(touch->shared_ptr.get())-> |
407 | - InsertProperty(UFTouchPropertyTime, |
408 | - new oif::frame::Value(time)); |
409 | -} |
410 | - |
411 | -void frame_backend_touch_set_start_time(UFBackendTouch touch, uint64_t start_time) |
412 | -{ |
413 | - static_cast<oif::frame::UFTouch*>(touch->shared_ptr.get())-> |
414 | - InsertProperty(UFTouchPropertyStartTime, |
415 | - new oif::frame::Value(start_time)); |
416 | -} |
417 | - |
418 | -void frame_backend_touch_set_owned(UFBackendTouch touch, int owned) |
419 | -{ |
420 | - static_cast<oif::frame::UFTouch*>(touch->shared_ptr.get())-> |
421 | - InsertProperty(UFTouchPropertyOwned, |
422 | - new oif::frame::Value(owned)); |
423 | -} |
424 | - |
425 | -void frame_backend_touch_set_pending_end(UFBackendTouch touch, int pending_end) |
426 | -{ |
427 | - static_cast<oif::frame::UFTouch*>(touch->shared_ptr.get())-> |
428 | - InsertProperty(UFTouchPropertyPendingEnd, |
429 | - new oif::frame::Value(pending_end)); |
430 | -} |
431 | - |
432 | -void frame_backend_touch_set_value(UFBackendTouch touch, UFAxisType type, float value) |
433 | -{ |
434 | - static_cast<oif::frame::UFTouch*>(touch->shared_ptr.get())-> |
435 | - SetValue(type, value); |
436 | -} |
437 | - |
438 | -void frame_backend_touch_delete(UFBackendTouch touch) |
439 | -{ |
440 | - delete touch; |
441 | +void frame_backend_touch_set_ended(UFBackendTouch touch_backend) |
442 | +{ |
443 | + oif::frame::UFTouch *touch = touch_backend->GetModifiableTouch(); |
444 | + |
445 | + touch->InsertProperty(UFTouchPropertyState, new oif::frame::Value(UFTouchStateEnd)); |
446 | + touch->SetState(UFTouchStateEnd); |
447 | +} |
448 | + |
449 | +void frame_backend_touch_set_window_pos(UFBackendTouch touch_backend, float x, float y) |
450 | +{ |
451 | + oif::frame::UFTouch *touch = touch_backend->GetModifiableTouch(); |
452 | + |
453 | + touch->InsertProperty(UFTouchPropertyWindowX, new oif::frame::Value(x)); |
454 | + touch->InsertProperty(UFTouchPropertyWindowY, new oif::frame::Value(y)); |
455 | +} |
456 | + |
457 | +void frame_backend_touch_set_time(UFBackendTouch touch_backend, uint64_t time) |
458 | +{ |
459 | + oif::frame::UFTouch *touch = touch_backend->GetModifiableTouch(); |
460 | + |
461 | + touch->InsertProperty(UFTouchPropertyTime, new oif::frame::Value(time)); |
462 | +} |
463 | + |
464 | +void frame_backend_touch_set_start_time(UFBackendTouch touch_backend, |
465 | + uint64_t start_time) |
466 | +{ |
467 | + oif::frame::UFTouch *touch = touch_backend->GetModifiableTouch(); |
468 | + |
469 | + touch->InsertProperty(UFTouchPropertyStartTime, new oif::frame::Value(start_time)); |
470 | +} |
471 | + |
472 | +void frame_backend_touch_set_owned(UFBackendTouch touch_backend, int owned) |
473 | +{ |
474 | + oif::frame::UFTouch *touch = touch_backend->GetModifiableTouch(); |
475 | + |
476 | + touch->InsertProperty(UFTouchPropertyOwned, new oif::frame::Value(owned)); |
477 | +} |
478 | + |
479 | +void frame_backend_touch_set_pending_end(UFBackendTouch touch_backend, int pending_end) |
480 | +{ |
481 | + oif::frame::UFTouch *touch = touch_backend->GetModifiableTouch(); |
482 | + |
483 | + touch->InsertProperty(UFTouchPropertyPendingEnd, new oif::frame::Value(pending_end)); |
484 | +} |
485 | + |
486 | +void frame_backend_touch_set_value(UFBackendTouch touch_backend, |
487 | + UFAxisType type, float value) |
488 | +{ |
489 | + oif::frame::UFTouch *touch = touch_backend->GetModifiableTouch(); |
490 | + |
491 | + touch->SetValue(type, value); |
492 | } |
493 | |
494 | } // extern "C" |
495 | |
496 | === modified file 'src/touch.h' |
497 | --- src/touch.h 2012-08-28 18:20:51 +0000 |
498 | +++ src/touch.h 2012-08-31 19:29:21 +0000 |
499 | @@ -38,6 +38,12 @@ |
500 | UFBackendTouch_(oif::frame::UFTouch* touch) |
501 | : shared_ptr(touch) {} |
502 | |
503 | + UFBackendTouch_(oif::frame::SharedUFTouch &shared_touch) { |
504 | + shared_ptr.swap(shared_touch); |
505 | + } |
506 | + |
507 | + oif::frame::UFTouch* GetModifiableTouch(); |
508 | + |
509 | oif::frame::SharedUFTouch shared_ptr; |
510 | }; |
511 | |
512 | @@ -46,7 +52,7 @@ |
513 | |
514 | class UFTouch : public UFTouch_, public Property<UFTouchProperty> { |
515 | public: |
516 | - UFTouch() {} |
517 | + UFTouch(); |
518 | UFTouch(UFTouchState state, UFTouchId id, float x, float y, |
519 | uint64_t time); |
520 | UFTouch(const UFTouch& touch, UFTouchState new_state); |
521 | @@ -60,7 +66,6 @@ |
522 | void SetValue(UFAxisType type, float value); |
523 | UFStatus GetValue(UFAxisType type, float* value) const; |
524 | |
525 | - UFTouch(const UFTouch&) = delete; |
526 | UFTouch& operator=(const UFTouch&) = delete; |
527 | |
528 | private: |
529 | |
530 | === modified file 'src/window.h' |
531 | --- src/window.h 2012-06-21 19:41:40 +0000 |
532 | +++ src/window.h 2012-08-31 19:29:21 +0000 |
533 | @@ -31,7 +31,7 @@ |
534 | namespace oif{ |
535 | namespace frame { |
536 | |
537 | -class Window : public std::enable_shared_from_this<Window> { |
538 | +class Window { |
539 | public: |
540 | Window(); |
541 | virtual ~Window() {}; |
542 | |
543 | === modified file 'src/x11/window_x11.cpp' |
544 | --- src/x11/window_x11.cpp 2012-07-24 20:47:15 +0000 |
545 | +++ src/x11/window_x11.cpp 2012-08-31 19:29:21 +0000 |
546 | @@ -76,7 +76,7 @@ |
547 | return false; |
548 | } |
549 | |
550 | - *frame = SharedUFFrame(new UFFrame(shared_from_this(), current_frame_)); |
551 | + *frame = SharedUFFrame(new UFFrame(current_frame_)); |
552 | |
553 | const Value* value = new Value(frame_x11_create_window_id(window_)); |
554 | (*frame)->InsertProperty(UFFramePropertyWindowId, value); |
555 | @@ -145,7 +145,7 @@ |
556 | |
557 | bool WindowX11::HandleOwnershipEvent(const XITouchOwnershipEvent* event, |
558 | SharedUFFrame* frame) { |
559 | - *frame = SharedUFFrame(new UFFrame(shared_from_this(), current_frame_)); |
560 | + *frame = SharedUFFrame(new UFFrame(current_frame_)); |
561 | |
562 | const Value* value = new Value(frame_x11_create_window_id(window_)); |
563 | (*frame)->InsertProperty(UFFramePropertyWindowId, value); |
564 | |
565 | === modified file 'test/regular/backend.cpp' |
566 | --- test/regular/backend.cpp 2012-08-28 19:51:37 +0000 |
567 | +++ test/regular/backend.cpp 2012-08-31 19:29:21 +0000 |
568 | @@ -11,11 +11,13 @@ |
569 | UFTouch touch = frame_backend_touch_get_touch(touch_backend); |
570 | ASSERT_TRUE(touch != nullptr); |
571 | |
572 | + ASSERT_EQ(UFTouchStateBegin, frame_touch_get_state(touch)); |
573 | + |
574 | frame_backend_touch_set_id(touch_backend, 123); |
575 | ASSERT_EQ(123, frame_touch_get_id(touch)); |
576 | |
577 | - frame_backend_touch_set_state(touch_backend, UFTouchStateUpdate); |
578 | - ASSERT_EQ(UFTouchStateUpdate, frame_touch_get_state(touch)); |
579 | + frame_backend_touch_set_ended(touch_backend); |
580 | + ASSERT_EQ(UFTouchStateEnd, frame_touch_get_state(touch)); |
581 | |
582 | frame_backend_touch_set_window_pos(touch_backend, 1.2f, 3.4f); |
583 | ASSERT_EQ(1.2f, frame_touch_get_window_x(touch)); |
584 | @@ -45,7 +47,10 @@ |
585 | ASSERT_EQ(UFStatusSuccess, status); |
586 | ASSERT_EQ(987.6f, touch_major); |
587 | |
588 | - frame_backend_touch_delete(touch_backend); |
589 | + /* clean up */ |
590 | + UFBackendFrame frame = frame_backend_frame_new(); |
591 | + frame_backend_frame_give_touch(frame, &touch_backend); |
592 | + frame_backend_frame_delete(frame); |
593 | } |
594 | |
595 | TEST(Backend, Device) |
596 | @@ -152,7 +157,9 @@ |
597 | status = frame_frame_get_property(frame, UFFramePropertyActiveTouches, &active_touches); |
598 | ASSERT_EQ(UFStatusSuccess, status); |
599 | ASSERT_EQ(0, active_touches); |
600 | - frame_backend_frame_add_touch(frame_backend, touch_backend); |
601 | + status = frame_backend_frame_give_touch(frame_backend, &touch_backend); |
602 | + ASSERT_EQ(UFStatusSuccess, status); |
603 | + ASSERT_EQ(nullptr, touch_backend); |
604 | ASSERT_EQ(1, frame_frame_get_num_touches(frame)); |
605 | status = frame_frame_get_property(frame, UFFramePropertyActiveTouches, &active_touches); |
606 | ASSERT_EQ(UFStatusSuccess, status); |
607 | @@ -168,7 +175,83 @@ |
608 | |
609 | frame_backend_frame_delete(frame_backend); |
610 | frame_backend_device_delete(device_backend); |
611 | - frame_backend_touch_delete(touch_backend); |
612 | +} |
613 | + |
614 | +TEST(Backend, FrameBorrowInexistentTouch) |
615 | +{ |
616 | + UFStatus status; |
617 | + |
618 | + UFBackendFrame frame_backend = frame_backend_frame_new(); |
619 | + |
620 | + UFBackendTouch touch_backend; |
621 | + status = frame_backend_frame_borrow_touch_by_id(frame_backend, 123, &touch_backend); |
622 | + ASSERT_EQ(UFStatusErrorInvalidTouch, status); |
623 | + |
624 | + /* clean up */ |
625 | + frame_backend_frame_delete(frame_backend); |
626 | +} |
627 | + |
628 | +TEST(Backend, FrameCreateNext) |
629 | +{ |
630 | + UFStatus status; |
631 | + |
632 | + /* frame 1 */ |
633 | + UFBackendTouch touch_backend = frame_backend_touch_new(); |
634 | + frame_backend_touch_set_id(touch_backend, 1); |
635 | + UFTouch frame1_touch = frame_backend_touch_get_touch(touch_backend); |
636 | + |
637 | + UFBackendFrame frame1_backend = frame_backend_frame_new(); |
638 | + status = frame_backend_frame_give_touch(frame1_backend, &touch_backend); |
639 | + ASSERT_EQ(UFStatusSuccess, status); |
640 | + |
641 | + /* frame 2 */ |
642 | + UFBackendFrame frame2_backend = frame_backend_frame_create_next(frame1_backend); |
643 | + UFFrame frame2 = frame_backend_frame_get_frame(frame2_backend); |
644 | + |
645 | + UFTouch frame2_touch; |
646 | + status = frame_frame_get_touch_by_id(frame2, 1, &frame2_touch); |
647 | + ASSERT_EQ(UFStatusSuccess, status); |
648 | + |
649 | + /* frame 3 */ |
650 | + UFBackendFrame frame3_backend = frame_backend_frame_create_next(frame2_backend); |
651 | + UFFrame frame3 = frame_backend_frame_get_frame(frame3_backend); |
652 | + |
653 | + UFTouch frame3_touch; |
654 | + status = frame_frame_get_touch_by_id(frame3, 1, &frame3_touch); |
655 | + ASSERT_EQ(UFStatusSuccess, status); |
656 | + |
657 | + /* frame 4 */ |
658 | + UFBackendFrame frame4_backend = frame_backend_frame_create_next(frame3_backend); |
659 | + |
660 | + status = frame_backend_frame_borrow_touch_by_id(frame4_backend, 1, &touch_backend); |
661 | + ASSERT_EQ(UFStatusSuccess, status); |
662 | + frame_backend_touch_set_ended(touch_backend); |
663 | + status = frame_backend_frame_give_touch(frame4_backend, &touch_backend); |
664 | + ASSERT_EQ(UFStatusSuccess, status); |
665 | + |
666 | + UFFrame frame4 = frame_backend_frame_get_frame(frame4_backend); |
667 | + UFTouch frame4_touch; |
668 | + status = frame_frame_get_touch_by_id(frame4, 1, &frame4_touch); |
669 | + ASSERT_EQ(UFStatusSuccess, status); |
670 | + |
671 | + /* frame 5 */ |
672 | + UFBackendFrame frame5_backend = frame_backend_frame_create_next(frame4_backend); |
673 | + UFFrame frame5 = frame_backend_frame_get_frame(frame5_backend); |
674 | + |
675 | + /* Test the status of touch 1 throughout all frames */ |
676 | + ASSERT_EQ(UFTouchStateBegin, frame_touch_get_state(frame1_touch)); |
677 | + ASSERT_EQ(UFTouchStateUpdate, frame_touch_get_state(frame2_touch)); |
678 | + ASSERT_EQ(UFTouchStateUpdate, frame_touch_get_state(frame3_touch)); |
679 | + ASSERT_EQ(frame2_touch, frame3_touch); |
680 | + ASSERT_EQ(UFTouchStateEnd, frame_touch_get_state(frame4_touch)); |
681 | + ASSERT_EQ(0, frame_frame_get_num_touches(frame5)); |
682 | + |
683 | + /* clean up */ |
684 | + frame_backend_frame_delete(frame1_backend); |
685 | + frame_backend_frame_delete(frame2_backend); |
686 | + frame_backend_frame_delete(frame3_backend); |
687 | + frame_backend_frame_delete(frame4_backend); |
688 | + frame_backend_frame_delete(frame5_backend); |
689 | } |
690 | |
691 | TEST(Backend, Event) |
* The Window class no longer needs to subclass std::enable_ shared_ from_this< Window> .
* Any functions that may fail need to return a UFStatus and provide the return result through a referenced parameter. All of the new functions here should do this. I forgot about this when I approved the previous merge proposal as well. We should go back and fix these up.
* There's a stray newline at the end of UFFrame: :GetSharedTouch ById.
Everything else looks good :). This should really help backend implementations get things right the first time.