Merge lp:~unity-team/qtmir/touch_tracing into lp:qtmir
| Status: | Merged |
|---|---|
| Approved by: | Gerry Boland on 2015-10-01 |
| Approved revision: | 378 |
| Merged at revision: | 395 |
| Proposed branch: | lp:~unity-team/qtmir/touch_tracing |
| Merge into: | lp:qtmir |
| Prerequisite: | lp:~nick-dedekind/qtmir/remove-dpkg-CMAK_INSTALL_PREFIX |
| Diff against target: |
1257 lines (+941/-18) 28 files modified
CMakeLists.txt (+3/-1) benchmarks/CMakeLists.txt (+8/-0) benchmarks/README (+12/-0) benchmarks/common.py (+33/-0) benchmarks/report_types.py (+121/-0) benchmarks/touch_event_latency.R (+6/-0) benchmarks/touch_event_latency.py (+271/-0) debian/control (+20/-0) debian/qtmir-tests.install (+6/-0) debian/rules (+1/-1) demos/CMakeLists.txt (+4/-0) demos/paths.h.in (+40/-0) demos/qml-demo-client/CMakeLists.txt (+41/-0) demos/qml-demo-client/main.cpp (+73/-0) demos/qml-demo-client/qtmir-demo-client.desktop.in (+9/-0) demos/qml-demo-shell/CMakeLists.txt (+35/-0) demos/qml-demo-shell/main.cpp (+57/-0) src/common/timestamp.cpp (+18/-0) src/common/timestamp.h (+39/-0) src/common/timestamp_impl.h (+34/-0) src/modules/Unity/Application/mirsurface.cpp (+5/-4) src/modules/Unity/Application/mirsurfaceitem.cpp (+6/-0) src/modules/Unity/Application/tracepoints.tp (+5/-0) src/platforms/mirserver/CMakeLists.txt (+1/-0) src/platforms/mirserver/qteventfeeder.cpp (+14/-4) src/platforms/mirserver/tracepoints.tp (+5/-0) tests/modules/General/CMakeLists.txt (+2/-8) tests/modules/General/timestamp_test.cpp (+72/-0) |
| To merge this branch: | bzr merge lp:~unity-team/qtmir/touch_tracing |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Gerry Boland | 2015-09-18 | Approve on 2015-10-01 | |
| PS Jenkins bot | continuous-integration | 2015-09-18 | Needs Fixing on 2015-09-30 |
|
Review via email:
|
|||
This proposal supersedes a proposal from 2015-08-05.
Commit Message
Added touch performance tracing and test.
Description of the Change
Added touch performance tracing and test.
* Are there any related MPs required for this MP to build/function as expected? Please list.
Yes
* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes
* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:351
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| Gerry Boland (gerboland) wrote : | # |
=== modified file 'debian/rules'
-DNO_TESTS=1 - please undo, we want tests!
=== added file 'demos/
I don't understand what this is useful for. Isn't the "qml" binary sufficient?
=== added file 'demos/
same here.
=== added file 'src/common/
file name "utils" as vague as possible:) Can you be more specific?
+++ src/common/
+qint64 resetStartTime(
+qint64 getStartTime_
please stick into an anonymous namespace
The indenting is inconsistent in the whole file.
+ QCoreApplicatio
I suspect a static variable would do the job just as well.
+ qint64 startTime = getStartTime_
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 QWindowSystemIn
ulong timestamp, const QPointF & local, const QPointF & global,
Qt:
Qt:
we could make our own handle*Event() methods. In the end, the above method simply does:
QWindowSystemIn
new QWindowSystemIn
QWindowSystemIn
If we subclassed QWindowSystemIn
To be investigated. For now, your approach is good.
====
std::chrono:
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/
=== modified file 'src/platforms/
Typos:
touchEventDispt
touchEventDispt
and these propagate throughout the whole of mirserver.
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:352
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:370
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:372
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:373
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| Nick Dedekind (nick-dedekind) wrote : | # |
> === modified file 'debian/rules'
> -DNO_TESTS=1 - please undo, we want tests!
>
> === added file 'demos/
> I don't understand what this is useful for. Isn't the "qml" binary sufficient?
>
> === added file 'demos/
> same here.
>
It's so we can get them to work with the benchmark util. The client needs to parse the command line parameters for the socket.
>
> === added file 'src/common/
> file name "utils" as vague as possible:) Can you be more specific?
timestamp.h
>
> +++ src/common/
> +qint64 resetStartTime(
> +qint64 getStartTime_
> please stick into an anonymous namespace
>
> The indenting is inconsistent in the whole file.
>
> + QCoreApplicatio
> I suspect a static variable would do the job just as well.
Used extern functions with a variable. Needs to be shared by the different modules.
>
> + qint64 startTime = getStartTime_
> 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 QWindowSystemIn
> ulong timestamp, const QPointF & local, const QPointF & global,
> Qt::MouseButtons b, Qt::KeyboardMod
> Qt::MouseEventS
>
> we could make our own handle*Event() methods. In the end, the above method
> simply does:
>
> QWindowSystemIn
> new QWindowSystemIn
> QWindowSystemIn
> QHighDpi:
> QHighDpi:
>
> QWindowSystemIn
>
> If we subclassed QWindowSystemIn
> 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.
I'll take a look into it when i get a minute.
> ====
>
>
> std::chrono:
> 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/
Done. in the header.
>
>
> === modified file 'src/platforms/
> Typos:
> touchEventDispt
> touchEventDispt
> and these propagate throughout the whole of mirserver.
Fixed.
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:374
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:375
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:376
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| Gerry Boland (gerboland) wrote : | # |
+$ cd benchmarks
+$ sudo python3 touch_event_
I roughly followed this, but since I saw this script was installed by qtmir-tests in /usr/share/
sudo python3 /usr/share/
but it doesn't fully work:
file://
- 377. By Nick Dedekind on 2015-09-18
-
newline
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:376
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:377
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 378. By Nick Dedekind on 2015-09-30
-
merged with trunk
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:378
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Gerry Boland (gerboland) wrote : | # |
We're having mild CI issues. Testing locally, all is good. We've a bunch of landings coming up, so I expect conflicts before this appears at the top of the queue unfortunately.
- 379. By Nick Dedekind on 2015-10-08
-
fixed debian/control for qtmir-tests
- 380. By Nick Dedekind on 2015-10-13
-
merged with lp:~dandrader/qtmir/multimonitorNext
- 381. By Nick Dedekind on 2015-10-13
-
merged with cmake branch
- 382. By Nick Dedekind on 2015-10-14
-
merged parent

PASSED: Continuous integration, rev:350 jenkins. qa.ubuntu. com/job/ qtmir-ci/ 343/ jenkins. qa.ubuntu. com/job/ qtmir-wily- amd64-ci/ 76 jenkins. qa.ubuntu. com/job/ qtmir-wily- armhf-ci/ 76 jenkins. qa.ubuntu. com/job/ qtmir-wily- armhf-ci/ 76/artifact/ work/output/ *zip*/output. zip
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/qtmir- ci/343/ rebuild
http://