Merge lp:~zsombi/ubuntu-ui-toolkit/migrate_unity8_gestures into lp:ubuntu-ui-toolkit/staging
| Status: | Merged |
|---|---|
| Approved by: | Christian Dywan on 2015-11-17 |
| Approved revision: | 1736 |
| Merged at revision: | 1718 |
| Proposed branch: | lp:~zsombi/ubuntu-ui-toolkit/migrate_unity8_gestures |
| Merge into: | lp:ubuntu-ui-toolkit/staging |
| Diff against target: |
7195 lines (+6597/-24) 72 files modified
components.api (+23/-0) debian/control (+23/-0) debian/libubuntugestures-dev.install (+28/-0) debian/libubuntugestures.install (+1/-0) documentation/overview.qdoc (+7/-0) examples/ubuntu-ui-toolkit-gallery/SwipeAreaPage.qml (+144/-0) examples/ubuntu-ui-toolkit-gallery/Template.qml (+1/-0) examples/ubuntu-ui-toolkit-gallery/WidgetsModel.qml (+5/-0) examples/ubuntu-ui-toolkit-gallery/gallery (+1/-1) examples/ubuntu-ui-toolkit-gallery/gallery-logging.config (+4/-0) examples/ubuntu-ui-toolkit-gallery/ubuntu-ui-toolkit-gallery.pro (+2/-1) export_modules_dir.sh (+2/-0) features/ubuntu_qml_plugin.prf (+11/-0) features/ubuntu_qt_module.prf (+19/-0) src/Ubuntu/Components/plugin/gestures/CandidateInactivityTimer.cpp (+46/-0) src/Ubuntu/Components/plugin/gestures/CandidateInactivityTimer.h (+52/-0) src/Ubuntu/Components/plugin/gestures/damper.cpp (+24/-0) src/Ubuntu/Components/plugin/gestures/damper.h (+89/-0) src/Ubuntu/Components/plugin/gestures/ubuntugesturesqmlglobal.h (+24/-0) src/Ubuntu/Components/plugin/gestures/ucswipearea.cpp (+958/-0) src/Ubuntu/Components/plugin/gestures/ucswipearea.h (+98/-0) src/Ubuntu/Components/plugin/gestures/ucswipearea_p.h (+157/-0) src/Ubuntu/Components/plugin/plugin.cpp (+2/-0) src/Ubuntu/Components/plugin/plugin.pri (+9/-4) src/Ubuntu/Test/plugin/plugin.pri (+4/-2) src/Ubuntu/Test/plugin/testplugin.cpp (+10/-0) src/Ubuntu/Test/plugin/ucmousetouchadaptor.cpp (+198/-0) src/Ubuntu/Test/plugin/ucmousetouchadaptor.h (+60/-0) src/Ubuntu/Test/plugin/uctestcase.cpp (+2/-1) src/Ubuntu/Test/plugin/uctestcase.h (+1/-1) src/Ubuntu/Test/plugin/uctestextras.h (+2/-0) src/Ubuntu/UbuntuGestures/UbuntuGestures.pro (+31/-0) src/Ubuntu/UbuntuGestures/candidateinactivitytimer.cpp (+46/-0) src/Ubuntu/UbuntuGestures/candidateinactivitytimer.h (+51/-0) src/Ubuntu/UbuntuGestures/debughelpers.cpp (+95/-0) src/Ubuntu/UbuntuGestures/debughelpers.h (+31/-0) src/Ubuntu/UbuntuGestures/pool.h (+132/-0) src/Ubuntu/UbuntuGestures/timer.cpp (+152/-0) src/Ubuntu/UbuntuGestures/timer.h (+117/-0) src/Ubuntu/UbuntuGestures/timesource.cpp (+47/-0) src/Ubuntu/UbuntuGestures/timesource.h (+62/-0) src/Ubuntu/UbuntuGestures/touchownershipevent.cpp (+35/-0) src/Ubuntu/UbuntuGestures/touchownershipevent.h (+50/-0) src/Ubuntu/UbuntuGestures/touchregistry.cpp (+520/-0) src/Ubuntu/UbuntuGestures/touchregistry.h (+206/-0) src/Ubuntu/UbuntuGestures/ubuntugesturesglobal.h (+23/-0) src/Ubuntu/UbuntuGestures/unownedtouchevent.cpp (+39/-0) src/Ubuntu/UbuntuGestures/unownedtouchevent.h (+45/-0) src/src.pro (+18/-4) sync.profile (+29/-0) tests/license/checklicense.sh (+1/-1) tests/qmlapicheck.sh (+1/-1) tests/unit/add_makecheck.pri (+1/-1) tests/unit/add_qmlmakecheck.pri (+1/-1) tests/unit_x11/add_makecheck.pri (+1/-1) tests/unit_x11/add_qmlmakecheck.pri (+1/-1) tests/unit_x11/test-include.pri (+1/-1) tests/unit_x11/tst_deprecated_theme_engine/tst_deprecated_theme_engine.cpp (+1/-1) tests/unit_x11/tst_subtheming/tst_subtheming.cpp (+1/-1) tests/unit_x11/tst_swipearea/DownwardsLauncher.qml (+72/-0) tests/unit_x11/tst_swipearea/GestureTest.cpp (+140/-0) tests/unit_x11/tst_swipearea/GestureTest.h (+92/-0) tests/unit_x11/tst_swipearea/LeftwardsLauncher.qml (+76/-0) tests/unit_x11/tst_swipearea/RightwardsLauncher.qml (+76/-0) tests/unit_x11/tst_swipearea/UpwardsLauncher.qml (+76/-0) tests/unit_x11/tst_swipearea/tst_swipearea.cpp (+1305/-0) tests/unit_x11/tst_swipearea/tst_swipearea.pro (+13/-0) tests/unit_x11/tst_swipearea/tst_swipearea.qml (+77/-0) tests/unit_x11/tst_touchregistry/tst_TouchRegistry.cpp (+915/-0) tests/unit_x11/tst_touchregistry/tst_touchregistry.pro (+6/-0) tests/unit_x11/unit_x11.pro (+3/-1) ubuntu-sdk.pro (+1/-0) |
| To merge this branch: | bzr merge lp:~zsombi/ubuntu-ui-toolkit/migrate_unity8_gestures |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| PS Jenkins bot | continuous-integration | Approve on 2015-11-17 | |
| Christian Dywan | Approve on 2015-11-17 | ||
| Benjamin Zeller (community) | Approve on 2015-11-16 | ||
| Zsombor Egri (community) | Approve on 2015-11-11 | ||
| Daniel d'Andrada (community) | 2015-10-21 | Approve on 2015-11-10 | |
| Michał Sawicz | 2015-10-21 | Needs Information on 2015-10-23 | |
|
Review via email:
|
|||
Commit Message
Migrate DirectionalDragArea from Unity8, named as SwipeArea. Original code (from lp:unity8) by: Daniel d'Andrada <email address hidden>
Description of the Change
SwipeArea is the new name of the Unity8 DirectionalDrag
- 1696. By Zsombor Egri on 2015-10-21
-
add trace functions
- 1697. By Zsombor Egri on 2015-10-21
-
documentation added, point properties compacted
- 1698. By Zsombor Egri on 2015-10-21
-
missing file added
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1698
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 1699. By Zsombor Egri on 2015-10-21
-
gallery page added, doc sample as well
- 1700. By Zsombor Egri on 2015-10-21
-
commented code removed
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1700
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 1701. By Zsombor Egri on 2015-10-21
-
specifying the Flickable relationship in the documentation
| Daniel d'Andrada (dandrader) wrote : | # |
I think you need the "Area" suffix in the name, otherwise it will be misleading. Because it *really* is an area that detects gestures.
| Daniel d'Andrada (dandrader) wrote : | # |
And I had the "Directional" in the name because it's not for all kinds of drag gestures. It's specifically for axis-aligned drags. It does not support dragging freely, in both X and Y axes. But I understand it makes for a mouthful if you keep it. On the other and it might clash if you provide some generic drag component in the future.
| Daniel d'Andrada (dandrader) wrote : | # |
DragGesture.svg is outdated. You're better off without it.
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1701
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Michał Sawicz (saviq) wrote : | # |
Shouldn't it be DragGestureArea? It is an area after all...
The gallery page for this is quite tricky to use, planning to update it somehow?
We discussed the potential need for a "monitoring-only" version of DragGesture, that would never grab the touch points, but work exactly the same otherwise. Now might be a good time to add?
Inline comments didn't work, so...
+ * The drag recognision is performed in a distanceThreshold, which is the size
→ recognition
+ * DragGesture {
+ * anchors {
+ * left: parent.left
+ * top: parent.top
+ * bottom: parent.bottom
+ * }
+ * height: units.gu(5)
+ * Label {
+ * text: "Drag upwards"
+ * anchors {
+ * centerIn: parent
+ * verticalOffset: parent.dragging ? parent.distance : 0
+ * }
+ * }
+ * }
Something wrong here with anchors - it's anchored left/top/bottom but has height, should probably be anchored left/right/bottom.
+ * \qmlproperty bool DragGesture:
+ * \readonly
+ * Drives whether the gesture should be recognized as soon as the touch lands on
+ * the area. With this property set it will work the same way as a MultiPointTouch
+ *
+ * Defaults to false. In most cases this should not be set.
Not sure of the use case, Daniel?
+ * \qmlproperty real DragGesture:
+ * \readonly
+ * The distance travelled by the finger along the axis specified by \l direction
+ * in scene coordinates
Distance in coordinates? Shouldn't distance be oblivious of coordinates?
TouchRegistry documentation isn't built, not sure if we want to commit to it, though.
[TBC]
| Daniel d'Andrada (dandrader) wrote : | # |
On 21/10/2015 13:14, Michał Sawicz wrote:
> + * \qmlproperty bool DragGesture:
> + * \readonly
> + * Drives whether the gesture should be recognized as soon as the touch lands on
> + * the area. With this property set it will work the same way as a MultiPointTouch
> + *
> + * Defaults to false. In most cases this should not be set.
>
> Not sure of the use case, Daniel?
Indicator panel drag handles.
| Daniel d'Andrada (dandrader) wrote : | # |
On 21/10/2015 13:14, Michał Sawicz wrote:
> + * \qmlproperty real DragGesture:
> + * \readonly
> + * The distance travelled by the finger along the axis specified by \l direction
> + * in scene coordinates
>
> Distance in coordinates? Shouldn't distance be oblivious of coordinates?
In scene coordinates. No, "distance" is the length of a line segment in
some coordinate system.
This property is in scene coordinates. The "distance" one (without the
"scene" prefix) is in local item coordinates.
| Daniel d'Andrada (dandrader) wrote : | # |
It's missing tests/libs/
| Zsombor Egri (zsombi) wrote : | # |
> Shouldn't it be DragGestureArea? It is an area after all...
After Daniel's arguments I decided to move back to your naming. After all that is also a suitable name for it as well...
>
> The gallery page for this is quite tricky to use, planning to update it
> somehow?
Yes, we discussed it with Daniel that the DDA (DirectionalDra
>
> We discussed the potential need for a "monitoring-only" version of
> DragGesture, that would never grab the touch points, but work exactly the same
> otherwise. Now might be a good time to add?
Perhaps, but let's get that in a separate MR, this is already a monster one :)
>
> Inline comments didn't work, so...
That's weird, it used to work for us!
>
> + * The drag recognision is performed in a distanceThreshold, which is the
> size
>
> → recognition
Fixed.
>
>
> + * DragGesture {
> + * anchors {
> + * left: parent.left
> + * top: parent.top
> + * bottom: parent.bottom
> + * }
> + * height: units.gu(5)
> + * Label {
> + * text: "Drag upwards"
> + * anchors {
> + * centerIn: parent
> + * verticalOffset: parent.dragging ? parent.distance : 0
> + * }
> + * }
> + * }
>
> Something wrong here with anchors - it's anchored left/top/bottom but has
> height, should probably be anchored left/right/bottom.
Yes. Fixed. In addition the direction is also specified.
>
>
> + * \qmlproperty bool DragGesture:
> + * \readonly
> + * Drives whether the gesture should be recognized as soon as the touch lands
> on
> + * the area. With this property set it will work the same way as a
> MultiPointTouch
> + *
> + * Defaults to false. In most cases this should not be set.
>
> Not sure of the use case, Daniel?
>
>
> + * \qmlproperty real DragGesture:
> + * \readonly
> + * The distance travelled by the finger along the axis specified by \l
> direction
> + * in scene coordinates
>
> Distance in coordinates? Shouldn't distance be oblivious of coordinates?
>
> TouchRegistry documentation isn't built, not sure if we want to commit to it,
> though.
This is only the QML part. I am not exporting any libraries here. And seems it would make sense to move the whole UbuntuGestures library as well.
>
> [TBC]
- 1702. By Zsombor Egri on 2015-10-22
-
review comments applied; component renamed into DirectionalDragArea
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1702
http://
Executed test runs:
UNSTABLE: http://
SUCCESS: http://
deb: http://
UNSTABLE: http://
Click here to trigger a rebuild:
http://
| Michał Sawicz (saviq) wrote : | # |
> > + * \qmlproperty real DragGesture:
> > + * \readonly
> > + * The distance travelled by the finger along the axis specified by \l
> direction
> > + * in scene coordinates
> >
> > Distance in coordinates? Shouldn't distance be oblivious of coordinates?
>
> In scene coordinates. No, "distance" is the length of a line segment in
> some coordinate system.
>
> This property is in scene coordinates. The "distance" one (without the
> "scene" prefix) is in local item coordinates.
So it's not "distance travelled", because if the finger went back and forth, travel increases all the time. Not sure if there's a better word for "distance", but the doc needs improving IMO.
Also:
+ * The distance travelled by the finger along the axis specified by \l direction.
Not "distance travelled", definitely not "by the finger". Not even sure how to describe that, it's the distance between the current position and TouchBegin point, in local coordinates, or something.
+ // TODO: Remove this workaround once we start using Qt 5.4
Time to do so?
+ // TODO: Consider when more than one touch starts in the same event (although it's not possible
+ // with Mir's android-input). Have to track them all. Consider it a plus/bonus.
Now, I believe, we've moved away from android-input, can this become a problem?
At least some of the documentation in ucdirectionaldr
- 1703. By Zsombor Egri on 2015-11-05
-
addin Gestures library to the project
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1703
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 1704. By Zsombor Egri on 2015-11-05
-
fixups
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1704
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 1705. By Zsombor Egri on 2015-11-06
-
fix library build
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1705
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| Daniel d'Andrada (dandrader) wrote : | # |
Would be great if you could turn all the logging from the current have-to-rebuild model (#ifdef) to QLoggingCategory (I do hope QLoggingCategories have negligible impact when turned off). I didn't bother doing that in unity8 because since the component lived in our source tree, rebuilding was trivial and painless. But this won't be the case anymore once it moves to ubuntu-ui-toolkit.
NB: Please keep the message format, like printing object names, as this helps immensely to tell instances apart.
- 1706. By Zsombor Egri on 2015-11-06
-
staging sync
- 1707. By Zsombor Egri on 2015-11-06
-
fixup, DirectionalDragArea renamed into SwipeArea
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1707
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| Zsombor Egri (zsombi) wrote : | # |
> Would be great if you could turn all the logging from the current have-to-
> rebuild model (#ifdef) to QLoggingCategory (I do hope QLoggingCategories have
> negligible impact when turned off). I didn't bother doing that in unity8
> because since the component lived in our source tree, rebuilding was trivial
> and painless. But this won't be the case anymore once it moves to ubuntu-ui-
> toolkit.
>
> NB: Please keep the message format, like printing object names, as this helps
> immensely to tell instances apart.
Ok, I will try to move to that. It has some impact on the execution, but it suppose dot be minimal - otherwise none would use it.
But first we need to spend more time with the API, especially that the AxisVelocityCal
AxisVelocityCal
- 1708. By Zsombor Egri on 2015-11-09
-
fixing gallery
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1708
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 1709. By Zsombor Egri on 2015-11-09
-
API and test launcher fix
- 1710. By Zsombor Egri on 2015-11-09
-
licence changed; add LD_LIBRARY_PATH to unit and unit_x11 makecheck
- 1711. By Zsombor Egri on 2015-11-09
-
leftover :/
- 1712. By Zsombor Egri on 2015-11-09
-
proper proeprty names for touch positions
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1710
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1712
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 1713. By Zsombor Egri on 2015-11-09
-
fixing SwipeArea tests and test launcher
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1712
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1713
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 1714. By Zsombor Egri on 2015-11-09
-
fix documentation and remove orphan code
- 1715. By Zsombor Egri on 2015-11-09
-
remove workaround for touch events required prior Qt 5.4
- 1716. By Zsombor Egri on 2015-11-09
-
use UbuntuTestCase for the test file
- 1717. By Zsombor Egri on 2015-11-09
-
fix API; remove unneeded axisvelocitycal
culator - 1718. By Zsombor Egri on 2015-11-09
-
remove touchScenePosit
ion() - 1719. By Zsombor Egri on 2015-11-09
-
remove sceneDirection property
- 1720. By Zsombor Egri on 2015-11-09
-
simplify position updates
- 1721. By Zsombor Egri on 2015-11-09
-
reorder members to optimize allocation
- 1722. By Zsombor Egri on 2015-11-09
-
add include folder to the licence exceptions
- 1723. By Zsombor Egri on 2015-11-09
-
touch registry tests added
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1722
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1723
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 1724. By Zsombor Egri on 2015-11-09
-
staging sync
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1724
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 1725. By Zsombor Egri on 2015-11-10
-
packaging added
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1725
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 1726. By Zsombor Egri on 2015-11-10
-
add logging support for SwipeArea and ActiveTouchPoints; logging can be activated through ubuntu.
components. SwipeArea. log.debug= true and ubuntu. components. SwipeArea. ActiveTouchInfo .log.debug= true either set individually to QT_LOGGING_RULES or in a file set to QT_LOGGING_CONF env variable; gallery- logging. config added and gallery runs with this rule file when launched from terminal - 1727. By Zsombor Egri on 2015-11-10
-
turn logging default off
| Daniel d'Andrada (dandrader) wrote : | # |
Please remove that from ucswipearea.h: " See doc/Directional
| Daniel d'Andrada (dandrader) wrote : | # |
You have two copies of MouseTouchAdaptor:
ubuntu-
src/Ubuntu/
Would be great if you could have only one in your source tree.
| Daniel d'Andrada (dandrader) wrote : | # |
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1726
http://
Executed test runs:
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:1727
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Zsombor Egri (zsombi) wrote : | # |
> You have two copies of MouseTouchAdaptor:
> ubuntu-
> src/Ubuntu/
>
> Would be great if you could have only one in your source tree.
Ah, good catch!
| Zsombor Egri (zsombi) wrote : | # |
| Zsombor Egri (zsombi) wrote : | # |
> You have two copies of MouseTouchAdaptor:
> ubuntu-
> src/Ubuntu/
>
> Would be great if you could have only one in your source tree.
Ahm... actually those are two separate modules, the launcher has its own stuff, toolkit test code has its own as well. I also noticed that we have a collision between the MouseTouchAdaptror I took from unity8 and our touch functions from TestExtras. I have to fix that as well.
| Daniel d'Andrada (dandrader) wrote : | # |
On 10/11/2015 12:26, Zsombor Egri wrote:
>> You have two copies of MouseTouchAdaptor:
>> ubuntu-
>> src/Ubuntu/
>>
>> Would be great if you could have only one in your source tree.
> Ahm... actually those are two separate modules, the launcher has its own stuff, toolkit test code has its own as well. I also noticed that we have a collision between the MouseTouchAdaptror I took from unity8 and our touch functions from TestExtras. I have to fix that as well.
You could still compile it twice but have only one copy of it in your
source tree
- 1728. By Zsombor Egri on 2015-11-10
-
make sure we have only one touch device registered no matter if MouseTouchAdaptor or TestExtras is used; add QT_LOGGING_RULES to test runner to stop logging on tests
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1728
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Zsombor Egri (zsombi) wrote : | # |
qmlapicheck.sh prints loads of warnings. Must be fixed!
| Christian Dywan (kalikiana) wrote : | # |
+
+ // disable logging filters
+ QLoggingCategor
+ QLoggingCategor
This overrides the filter rule set by apicheck which is "*=false".
Also the second filter replaces the first one.
It's definitely wrong for a QML component to override the global filter. If any other component or library does that you'll get undefined behavior. And as seen here, if the app (here apicheck) sets it, you're also screwed, depending on who sets it first.
| Daniel d'Andrada (dandrader) wrote : | # |
On 10/11/2015 16:31, Christian Dywan wrote:
> Review: Needs Fixing
>
> +
> + // disable logging filters
> + QLoggingCategor
> + QLoggingCategor
>
> This overrides the filter rule set by apicheck which is "*=false".
>
> Also the second filter replaces the first one.
>
> It's definitely wrong for a QML component to override the global filter. If any other component or library does that you'll get undefined behavior. And as seen here, if the app (here apicheck) sets it, you're also screwed, depending on who sets it first.
By the way, when you define a logging category you can specify its
default logging level. This would be a more appropriate approach:
Q_LOGGING_
And, by the way, the ".log" suffix is redundant. We already know it's
about logging. It's a logging category after all! :)
| Zsombor Egri (zsombi) wrote : | # |
> +
> + // disable logging filters
> + QLoggingCategor
> rea.log.
> + QLoggingCategor
> .ActiveTouchInf
>
> This overrides the filter rule set by apicheck which is "*=false".
If you read the docs, you will see it won't.
>
> Also the second filter replaces the first one.
Nope, the second filter is a different filter. Please read the docs if you are in dpoubths. Also, you can test it by enabling the two same time or separately from gallery-logs.config
>
> It's definitely wrong for a QML component to override the global filter. If
> any other component or library does that you'll get undefined behavior. And as
> seen here, if the app (here apicheck) sets it, you're also screwed, depending
> on who sets it first.
Please, again, read the docs. The order is (quote from Qt Docs):
QtProject/
setFilterRules()
QT_LOGGING_CONF
QT_LOGGING_RULES
| Zsombor Egri (zsombi) wrote : | # |
> By the way, when you define a logging category you can specify its
> default logging level. This would be a more appropriate approach:
> Q_LOGGING_
Eventually QtDebugMsg, as we use qCDebug() here and not qCWarning(). Or we switch to use qCWarning() instead.
> And, by the way, the ".log" suffix is redundant. We already know it's
> about logging. It's a logging category after all! :)
Ok, makes sense.
| Daniel d'Andrada (dandrader) wrote : | # |
On 11/11/2015 06:31, Zsombor Egri wrote:
>> By the way, when you define a logging category you can specify its
>> >default logging level. This would be a more appropriate approach:
>> >Q_LOGGING_
> Eventually QtDebugMsg, as we use qCDebug() here and not qCWarning(). Or we switch to use qCWarning() instead.
>
Ok, maybe you misunderstood me. The construction above says that by
default only warnings and above (like criticals) are logged. debug
messages being ignored. Then if you want debug messages to get logged as
well you have to explicitly enable then in the logging configuration ini
file or env var.
This is what should be done for SwipeArea debug messages otherwise they
will flood logs with events that are not interesting at all unless
you're debugging some swipe related issue.
| Zsombor Egri (zsombi) wrote : | # |
OK, I think I misunderstood you partly. Let me try again :)
> +
> + // disable logging filters
> + QLoggingCategor
> rea.log.
> + QLoggingCategor
> .ActiveTouchInf
>
> This overrides the filter rule set by apicheck which is "*=false".
Nope, as the QT_LOGGING_RULES env var overrides the setFilterRules() settings.
>
> Also the second filter replaces the first one.
THIS is true.
>
> It's definitely wrong for a QML component to override the global filter. If
> any other component or library does that you'll get undefined behavior. And as
> seen here, if the app (here apicheck) sets it, you're also screwed, depending
> on who sets it first.
I do not see from where have you got this idea that we do override the global filter. We don't. We define two special categories here, one for SwipeArea and one for the internal ActiveTouchInfo. The default category is intact, so we don't override the global filter in any component. specifying *.debug=false means we turn of all the debug logs so the api check (and also the other loggers) are turned off. And again QT_LOGGING_CONF and/or QT_LOGGING_RULES overrides the setFilterRules(). We have to turn them off, otherwise the Q_LOGGING_
- 1729. By Zsombor Egri on 2015-11-11
-
remove .log from categories; rise the message type to warnings to rule out qCDebug() logs s default, therefore no more need to override the filter rules
| Christian Dywan (kalikiana) wrote : | # |
> > +
> > + // disable logging filters
> > +
> QLoggingCategor
> > rea.log.
> > +
> QLoggingCategor
> > .ActiveTouchInf
> >
> > This overrides the filter rule set by apicheck which is "*=false".
>
> Nope, as the QT_LOGGING_RULES env var overrides the setFilterRules() settings.
>
> >
> > Also the second filter replaces the first one.
>
> THIS is true.
>
> >
> > It's definitely wrong for a QML component to override the global filter. If
> > any other component or library does that you'll get undefined behavior. And
> as
> > seen here, if the app (here apicheck) sets it, you're also screwed,
> depending
> > on who sets it first.
>
> I do not see from where have you got this idea that we do override the global
> filter. We don't. We define two special categories here, one for SwipeArea and
> one for the internal ActiveTouchInfo. The default category is intact, so we
> don't override the global filter in any component. specifying *.debug=false
> means we turn of all the debug logs so the api check (and also the other
> loggers) are turned off. And again QT_LOGGING_CONF and/or QT_LOGGING_RULES
> overrides the setFilterRules(). We have to turn them off, otherwise the
> Q_LOGGING_
You agreed above that QLoggingCategor
For apicheck it's fine if QT_LOGGING_* is set by a developer or script btw, if intended, since that allows you to debug the component causing the spam (which some day should be done for Ubuntu.Components).
Daniel's suggestion seems sensible - if you set the default level for the category to warnings only you won't effectively see any debug logging.
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1729
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Christian Dywan (kalikiana) wrote : | # |
./examples/
import Ubuntu.Components 1.3
^
Usage: ./ubuntu-
Should I do something special to run the launcher now? Running it from the build folder like I used to doesn't work anymore with the branch.
| Zsombor Egri (zsombi) wrote : | # |
> ./examples/
> cannot be loaded for module "Ubuntu.
> ./qml/Ubuntu/
> shared object file: No such file or directory)
> import Ubuntu.Components 1.3
> ^
> Usage: ./ubuntu-
> filename
>
> Should I do something special to run the launcher now? Running it from the
> build folder like I used to doesn't work anymore with the branch.
If you launch gallery from the terminal, it should work, the scripts have the LD_LIBRARY_PATH set. If you run it from QtCreator, you must set LD_LIBRARY_PATH to
a) ./libn if in-source build is used
b) relative path to your out-of-source lib path if that one is chosen
if you launch launcher just like that you need to set the LD_LIBRARY_PATH
- 1730. By Zsombor Egri on 2015-11-13
-
fixing wasSingleShot setting, remove GCC pragmas
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1730
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Christian Dywan (kalikiana) wrote : | # |
+#define TOUCHREGISTRY_DEBUG 0
Why not use proper logging here as well?
+/* Defines an interface for a Timer. Useful for tests. */
2931 +class UBUNTUGESTURES_
I'm inclined to agree with the comment - why isn't this in UbuntuTest?
+Description: Ubuntu gestures library - DragArea
65 + Ubuntu gestures library with DragArea
Should be "SwipeArea".
There's also many uses of "dragArea" in the tests, which might be fixed with a search and replace, but I could live with those.
More importantly, what's the plan for DraggingArea? I see no FIXME and no bug number anywhere. Even if that's done in a follow-up we should have a plan so it's not forgotten.
| Zsombor Egri (zsombi) wrote : | # |
> +#define TOUCHREGISTRY_DEBUG 0
>
> Why not use proper logging here as well?
Good point. Will do that! in RevNo 1731.
>
> +/* Defines an interface for a Timer. Useful for tests. */
> 2931 +class UBUNTUGESTURES_
>
> I'm inclined to agree with the comment - why isn't this in UbuntuTest?
That is a leftover from the old times... I'll remove that line, as the class is the base class for the timers used in the gesture recognition. Revno 1732.
>
> +Description: Ubuntu gestures library - DragArea
> 65 + Ubuntu gestures library with DragArea
>
> Should be "SwipeArea".
You seemed to have an older merge. It has been fixed since that.
>
> There's also many uses of "dragArea" in the tests, which might be fixed with a
> search and replace, but I could live with those.
done, revno 1733.
>
> More importantly, what's the plan for DraggingArea? I see no FIXME and no bug
> number anywhere. Even if that's done in a follow-up we should have a plan so
> it's not forgotten.
I think we will ditch that, I checked it but I thought to have a separate MR for that. Here's a bug for it: https:/
- 1731. By Zsombor Egri on 2015-11-17
-
use logging for TouchRegistry; fix parameters for the SwipeArea signals; do not export debughelper functions
- 1732. By Zsombor Egri on 2015-11-17
-
remove misleading comment
- 1733. By Zsombor Egri on 2015-11-17
-
renaming dragArea into swipeArea
- 1734. By Zsombor Egri on 2015-11-17
-
fix API file
| Daniel d'Andrada (dandrader) wrote : | # |
On 17/11/2015 10:23, Zsombor Egri wrote:
>> +/* Defines an interface for a Timer. Useful for tests. */
>> >2931 +class UBUNTUGESTURES_
>> >
>> >I'm inclined to agree with the comment - why isn't this in UbuntuTest?
> That is a leftover from the old times... I'll remove that line, as the class is the base class for the timers used in the gesture recognition. Revno 1732.
>
No, the whole point of hiding the real timers behind this interface is
to have the ability to use fake/mock timers in tests. So it does exactly
what the commment says.
| Zsombor Egri (zsombi) wrote : | # |
> On 17/11/2015 10:23, Zsombor Egri wrote:
> >> +/* Defines an interface for a Timer. Useful for tests. */
> >> >2931 +class UBUNTUGESTURES_
> >> >
> >> >I'm inclined to agree with the comment - why isn't this in UbuntuTest?
> > That is a leftover from the old times... I'll remove that line, as the class
> is the base class for the timers used in the gesture recognition. Revno 1732.
> >
>
> No, the whole point of hiding the real timers behind this interface is
> to have the ability to use fake/mock timers in tests. So it does exactly
> what the commment says.
Christian was wondering whether the stuff should be moved in UbuntuTest module, as it wasn't clear. But as SwipeArea uses it as well, we must keep it here... not sure about the fake timers then... Or what exactly should we export at all from this lib?
| Christian Dywan (kalikiana) wrote : | # |
+# debughelpers.h \
2346 + timer.h \
Should this be removed?
- 1735. By Zsombor Egri on 2015-11-17
-
revert removing debughelper.h
- 1736. By Zsombor Egri on 2015-11-17
-
fix package description
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1734
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Zsombor Egri (zsombi) wrote : | # |
> +# debughelpers.h \
> 2346 + timer.h \
>
> Should this be removed?
Nope, should not, only the declspec was changed, so not being exported by the lib.
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1736
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://

FAILED: Continuous integration, rev:1695 jenkins. qa.ubuntu. com/job/ ubuntu- sdk-team- ubuntu- ui-toolkit- staging- ci/2401/ jenkins. qa.ubuntu. com/job/ ubuntu- sdk-team- ubuntu- ui-toolkit- staging- vivid-amd64- ci/1129 jenkins. qa.ubuntu. com/job/ ubuntu- sdk-team- ubuntu- ui-toolkit- staging- vivid-armhf- ci/1131 jenkins. qa.ubuntu. com/job/ ubuntu- sdk-team- ubuntu- ui-toolkit- staging- vivid-armhf- ci/1131/ artifact/ work/output/ *zip*/output. zip jenkins. qa.ubuntu. com/job/ ubuntu- sdk-team- ubuntu- ui-toolkit- staging- vivid-i386- ci/1128
http://
Executed test runs:
UNSTABLE: http://
SUCCESS: http://
deb: http://
UNSTABLE: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/ubuntu- sdk-team- ubuntu- ui-toolkit- staging- ci/2401/ rebuild
http://