Code review comment for lp:~unity-team/qtmir/touch_tracing

=== modified file 'debian/rules'
-DNO_TESTS=1 - please undo, we want tests!

=== added file 'demos/qml-demo-client/main.cpp'
I don't understand what this is useful for. Isn't the "qml" binary sufficient?

=== added file 'demos/qml-demo-shell/main.cpp'
same here.

=== added file 'src/common/utils.h'
file name "utils" as vague as possible:) Can you be more specific?

+++ src/common/utils.cpp
+qint64 resetStartTime(qint64 timestamp) {
+qint64 getStartTime_nsec(qint64 timestamp) {
please stick into an anonymous namespace

The indenting is inconsistent in the whole file.

+ QCoreApplication::instance()->setProperty("appStartTime", timestamp);
I suspect a static variable would do the job just as well.

+ qint64 startTime = getStartTime_nsec(timestamp.count())/1000000;
std::chrono has nanosecond to milisecond conversion abilities built in, I think you can just cast and it works. It would be safer than dividing by 1000000. I suspect you can avoid all usages of qint64 as a result.

====
This compression is a clever idea to work around the difference in timestamp units Qt & Mir uses. I'm still curious if instead of tying ourselves to the event APIs Qt has, e.g.

void QWindowSystemInterface::handleFrameStrutMouseEvent(QWindow *w,
    ulong timestamp, const QPointF & local, const QPointF & global,
    Qt::MouseButtons b, Qt::KeyboardModifiers mods,
    Qt::MouseEventSource source)

we could make our own handle*Event() methods. In the end, the above method simply does:

QWindowSystemInterfacePrivate::MouseEvent * e =
    new QWindowSystemInterfacePrivate::MouseEvent(w, timestamp,
        QWindowSystemInterfacePrivate::FrameStrutMouse,
        QHighDpi::fromNativeLocalPosition(local, w),
        QHighDpi::fromNativePixels(global, w), b, mods, source);

QWindowSystemInterfacePrivate::handleWindowSystemEvent(e);

If we subclassed QWindowSystemInterfacePrivate::MouseEvent, and added the timestamp type we want, that event might pass through Qt safely. This would be especially usesful for Mir events which have more detail that Qt events have.

To be investigated. For now, your approach is good.
====

std::chrono::milliseconds compressTimestamp(std::chrono::nanoseconds timestamp);
Do you need the milliseconds anywhere, as I only see you calling .count() on this everywhere it's used. Why not just return ulong?

Also, would be good to document *why* you've done the timestamp compression/decompression, for future souls.

=== modified file 'src/platforms/mirserver/tracepoints.tp'
Typos:
touchEventDisptach_start
touchEventDisptach_end
and these propagate throughout the whole of mirserver.