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

Hi Chase,

thanks for your comments!

* Sending 3x3 matrices

The elements store 2x3 for memory reasons. The recipe to create the full 3x3 matrix is simple, so I would prefer to leave it as is.

* Deep copy

All functions copy the pointers, there is no deep copy anywhere. Perhaps the memcpy() in set_slot_multi caused the confusion.

* Pumping

The utouch frames are constructed as a fixed ring, so all pointers are always valid, although they will pointer to the "wrong" data after a while when the data wraps around.

The semantics around prev points could be improved upon, as you suggest. There was originally an idea that a gesture frame could span several touch frames, hence the extra logic around prev pointers for those. The reason for the idea was to be able to skip frames with "unstable" touch configurations, using the glue time. However, since then, this stabilization logic has been moved to
the actual gesture fragment activation, and it seems to me one could again simplify the semantics and say that every touch frame corresponds to a gesture frame. Any thoughts on that?

Regardless of the above, the get_frame methods could be further specified to only be well defined from a callback, for instance. In all other instances, the pointers are known anyways, from the flow of the code.

* Touch gestures

It works in this end, when running grail-gesture

* Merging

The grail2 branch i feel is pretty well tested, so that could in principle go to trunk. It only contains additions, no replacements. No biggie if we do the series for this as well, that is fine. Perhaps the series should be called 1.1, though, since be are not breaking ABI?

The grail2.next branch is not fully complete, agreed. There are test cases missing to guard the replacement patches. Minor behavior changes might also need to be checked and defined. Also, I am working on yet another simplification of the code, that should get rid of two of the three event buffers in use.

So yes, merging everything into a new series sounds good.

Tbanks!