Code review comment for lp:~oif-team/grail/grail2

Henrik,

I'm feeling really good about the architecture of the code. I still have questions about some of the math, but I'll leave that aside for now ;-)

I like the new transform tool quite a bit though; it really helps visualize what is going on.

Initially, I had lots more questions, but my investigations have answered most of those. Here are the remaining comments/questions I have:

* What would you think about passing around complete 3x3 matrices instead of the 3x2 simplified matrices that grail 2 emits right now? If we did something like this, a 3x3 matrix could be thrown into many transformation libraries, like pixman, without any conversion.

* In set_slot_multi(), the touches are deep copied into the grail_element slot. In the other set_slot_*() functions, the touches are shallow copied from the passed in utouch frame. It seems that it would be best for the usage model to be consistent between all the functions... is this left-over from a change, or is there a definite rationale for the difference?

* grail_pump_frame() returns a reference to a new grail frame. The utouch frame may be retrieved by calling grail_get_contact_frame(). How do we know that the utouch contact prev pointer in the returned utouch frame is still valid? It may become invalid as soon as utouch_frame is
pumped. I don't think this could cause a bug if the code calls the frame and grail methods appropriately. However, we could run into issues with folks doing things in odd ways, since the code is non-re-entrant. For instance, if anyone needs to do parallel programming with this interface
it could get very dicey. At the very least, we should add a note in grail.h that the utouch frame data is only valid until the utouch frame is pumped again.

One thing that may help is to add a utouch_frame function that can retrieve the previous frame, if available, instead of giving the client a pointer to memory that may end up modified over time. An even better alternative would be to have reference counting of frames, but that would be a larger architecture change to utouch frame. Do you have any thoughts here?

* Touch gestures seem to be missing from grail 2.

Overall, I'm feeling pretty good about the changes. As I now understand it, the extra freedom of choosing an appropriate pivot point may allow for better touch mapping as well. I really like the work overall!

As a side note, when we go to merge this Duncan has asked that we merge it into a separate series until at least all the dependent pieces are in place to not lose functionality. I set up the lp:utouch-grail/2.x series for this purpose. Once we have this and the next merge proposals approved we can look into merging it into trunk.

Thanks!