Merge lp:~nick-dedekind/ubuntu-ui-toolkit/round.floor.ceil-GridUnits into lp:ubuntu-ui-toolkit

Proposed by Nick Dedekind
Status: Rejected
Rejected by: Tim Peeters
Proposed branch: lp:~nick-dedekind/ubuntu-ui-toolkit/round.floor.ceil-GridUnits
Merge into: lp:ubuntu-ui-toolkit
Diff against target: 217 lines (+174/-0)
4 files modified
components.api (+21/-0)
modules/Ubuntu/Components/plugin/ucunits.cpp (+51/-0)
modules/Ubuntu/Components/plugin/ucunits.h (+5/-0)
tests/unit/tst_units/tst_units.cpp (+97/-0)
To merge this branch: bzr merge lp:~nick-dedekind/ubuntu-ui-toolkit/round.floor.ceil-GridUnits
Reviewer Review Type Date Requested Status
Florian Boucault (community) Needs Fixing
PS Jenkins bot continuous-integration Approve
Review via email: mp+195796@code.launchpad.net

Commit message

Added [floor/ceil/round]GridUnit function to get closest gu boundary position.

Description of the change

Added [floor/ceil/round]GridUnit function to get closest gu boundary position.

This is to enable us to easily align graphical items to the gu grid.
Use case:
Indicator panel items can be variable width (fixed height with aspect calculated width + variable length label) but should be aligned to the gu grid.

To post a comment you must log in.
Revision history for this message
Florian Boucault (fboucault) wrote :

Why do we need this? What is the use case? What is the problem that we are trying to solve?

review: Needs Information
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

> Why do we need this? What is the use case? What is the problem that we are
> trying to solve?

This is to enable us to easily align graphical items to the gu grid.
Use case:
Indicator panel items can be variable width (fixed height with aspect calculated width + variable length label) but should be aligned to the gu grid.

Currently there is a function in the indicators to calculate it, but should be in a common place so we can use it elsewhere. Units seemed the logical place to put it.

Revision history for this message
Florian Boucault (fboucault) wrote :

> > Why do we need this? What is the use case? What is the problem that we are
> > trying to solve?
>
> This is to enable us to easily align graphical items to the gu grid.
> Use case:
> Indicator panel items can be variable width (fixed height with aspect
> calculated width + variable length label) but should be aligned to the gu
> grid.
>
> Currently there is a function in the indicators to calculate it, but should be
> in a common place so we can use it elsewhere. Units seemed the logical place
> to put it.

Thanks for the explanation. Sorry for taking so long. Do you know where in the indicators code this is happening?

Revision history for this message
Florian Boucault (fboucault) wrote :

Found it! :)

Panel/Indicators/DefaultIndicatorWidget.qml

Revision history for this message
Florian Boucault (fboucault) wrote :

1) After reading function guRoundUp(width) in Panel/Indicators/DefaultIndicatorWidget.qml I did not expect to see a second 'multiple' argument in floorGridUnit, ceilGridUnit and roundGridUnit. I don't think they should have it.

2) I understand the use case for ceilGridUnit but not for floorGridUnit and roundGridUnit. Generally speaking, we avoid introducing APIs until we have a concrete use case for them. One of the main reason is that each API we have to support for a number of years.

review: Needs Fixing
Revision history for this message
Tim Peeters (tpeeters) wrote :

Was this MR abolished?

If not, please resubmit it to our staging and ask for another review.

Unmerged revisions

840. By Nick Dedekind

Added [floor/ceil/round]GridUnit for finding gu boundaries.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'components.api'
2--- components.api 2013-11-11 06:39:55 +0000
3+++ components.api 2013-11-19 14:56:38 +0000
4@@ -756,6 +756,27 @@
5 Method {
6 name: "gu"
7 Parameter { name: "value"; type: "float" }
8+ Method {
9+ name: "floorGridUnit"
10+ Parameter { name: "value"; type: "float" }
11+ Parameter { name: "precision"; type: "float" }
12+ Method {
13+ name: "floorGridUnit"
14+ Parameter { name: "value"; type: "float" }
15+ Method {
16+ name: "ceilGridUnit"
17+ Parameter { name: "value"; type: "float" }
18+ Parameter { name: "precision"; type: "float" }
19+ Method {
20+ name: "ceilGridUnit"
21+ Parameter { name: "value"; type: "float" }
22+ Method {
23+ name: "roundGridUnit"
24+ Parameter { name: "value"; type: "float" }
25+ Parameter { name: "precision"; type: "float" }
26+ Method {
27+ name: "roundGridUnit"
28+ Parameter { name: "value"; type: "float" }
29 name: "UCUriHandler"
30 prototype: "QObject"
31 exports: ["UriHandler 0.1"]
32
33=== modified file 'modules/Ubuntu/Components/plugin/ucunits.cpp'
34--- modules/Ubuntu/Components/plugin/ucunits.cpp 2013-10-18 08:56:39 +0000
35+++ modules/Ubuntu/Components/plugin/ucunits.cpp 2013-11-19 14:56:38 +0000
36@@ -110,6 +110,57 @@
37 return qRound(value * m_gridUnit);
38 }
39
40+/*!
41+ \qmlmethod real Units::floorGridUnit(real value, real multiple)
42+
43+ Returns the lower number of grid units (in pixels) that a pixel \a value maps to for a given \a multiple of gu
44+*/
45+float UCUnits::floorGridUnit(float value, float multiple) const
46+{
47+ if (value == 0) {
48+ return 0;
49+ }
50+ const float guMultiple = m_gridUnit * multiple;
51+ const float mod = fmod(value, guMultiple);
52+
53+ return value - mod;
54+}
55+
56+/*!
57+ \qmlmethod real Units::ceilGridUnit(real value, real multiple)
58+
59+ Returns the upper number of grid units (in pixels) that a pixel \a value maps to for a given \a multiple
60+*/
61+float UCUnits::ceilGridUnit(float value, float multiple) const
62+{
63+ if (value == 0) {
64+ return 0;
65+ }
66+ const float guMultiple = m_gridUnit * multiple;
67+ const float mod = fmod(value, guMultiple);
68+
69+ return mod == 0 ? value : value + (guMultiple - mod);
70+}
71+
72+/*!
73+ \qmlmethod real Units::closestGridUnit(real value, real multiple)
74+
75+ Returns the closest number of grid units (in pixels) that a pixel \a value maps to for a given \a multiple
76+*/
77+float UCUnits::roundGridUnit(float value, float multiple) const
78+{
79+ if (value == 0) {
80+ return 0;
81+ }
82+ const float guMultiple = m_gridUnit * multiple;
83+ const float mod = fmod(value, guMultiple);
84+
85+ if (mod * 2 >= guMultiple) {
86+ return value + (guMultiple - mod);
87+ }
88+ return value - mod;
89+}
90+
91 QString UCUnits::resolveResource(const QUrl& url)
92 {
93 if (url.isEmpty()) {
94
95=== modified file 'modules/Ubuntu/Components/plugin/ucunits.h'
96--- modules/Ubuntu/Components/plugin/ucunits.h 2013-10-18 08:56:39 +0000
97+++ modules/Ubuntu/Components/plugin/ucunits.h 2013-11-19 14:56:38 +0000
98@@ -37,6 +37,11 @@
99 explicit UCUnits(QObject *parent = 0);
100 Q_INVOKABLE float dp(float value);
101 Q_INVOKABLE float gu(float value);
102+
103+ Q_INVOKABLE float floorGridUnit(float value, float precision = 1.0) const;
104+ Q_INVOKABLE float ceilGridUnit(float value, float precision = 1.0) const;
105+ Q_INVOKABLE float roundGridUnit(float value, float precision = 1.0) const;
106+
107 QString resolveResource(const QUrl& url);
108
109 // getters
110
111=== modified file 'tests/unit/tst_units/tst_units.cpp'
112--- tests/unit/tst_units/tst_units.cpp 2013-02-05 17:10:20 +0000
113+++ tests/unit/tst_units/tst_units.cpp 2013-11-19 14:56:38 +0000
114@@ -153,6 +153,103 @@
115 QCOMPARE(units.dp(1000.01), 2500.0f);
116 }
117
118+ void floorGridUnitTen() {
119+ UCUnits units;
120+ units.setGridUnit(10);
121+
122+ QCOMPARE(units.floorGridUnit(0.0, 1.0), 0.0f);
123+ QCOMPARE(units.floorGridUnit(1.0, 1.0), 0.0f);
124+ QCOMPARE(units.floorGridUnit(5.0, 1.0), 0.0f);
125+ QCOMPARE(units.floorGridUnit(8.0, 1.0), 0.0f);
126+ QCOMPARE(units.floorGridUnit(130.0, 1.0), 130.0f);
127+ QCOMPARE(units.floorGridUnit(131.3, 1.0), 130.0f);
128+ QCOMPARE(units.floorGridUnit(138.8, 1.0), 130.0f);
129+
130+ QCOMPARE(units.floorGridUnit(0.0, 0.5), 0.0f);
131+ QCOMPARE(units.floorGridUnit(1.0, 0.5), 0.0f);
132+ QCOMPARE(units.floorGridUnit(5.0, 0.5), 5.0f);
133+ QCOMPARE(units.floorGridUnit(8.0, 0.5), 5.0f);
134+ QCOMPARE(units.floorGridUnit(130.0, 0.5), 130.0f);
135+ QCOMPARE(units.floorGridUnit(131.0, 0.5), 130.0f);
136+ QCOMPARE(units.floorGridUnit(138.8, 0.5), 135.0f);
137+
138+ QCOMPARE(units.floorGridUnit(0.0, 2.0), 0.0f);
139+ QCOMPARE(units.floorGridUnit(1.0, 2.0), 0.0f);
140+ QCOMPARE(units.floorGridUnit(5.0, 2.0), 0.0f);
141+ QCOMPARE(units.floorGridUnit(8.0, 2.0), 0.0f);
142+ QCOMPARE(units.floorGridUnit(130.0, 2.0), 120.0f);
143+ QCOMPARE(units.floorGridUnit(131.0, 2.0), 120.0f);
144+ QCOMPARE(units.floorGridUnit(138.8, 2.0), 120.0f);
145+ QCOMPARE(units.floorGridUnit(141.0, 2.0), 140.0f);
146+ }
147+
148+ void ceilGridUnitTen() {
149+ UCUnits units;
150+ units.setGridUnit(10);
151+
152+ QCOMPARE(units.ceilGridUnit(0.0, 1.0), 0.0f);
153+ QCOMPARE(units.ceilGridUnit(1.0, 1.0), 10.0f);
154+ QCOMPARE(units.ceilGridUnit(5.0, 1.0), 10.0f);
155+ QCOMPARE(units.ceilGridUnit(8.0, 1.0), 10.0f);
156+ QCOMPARE(units.ceilGridUnit(130.0, 1.0), 130.0f);
157+ QCOMPARE(units.ceilGridUnit(131.3, 1.0), 140.0f);
158+ QCOMPARE(units.ceilGridUnit(138.8, 1.0), 140.0f);
159+
160+ QCOMPARE(units.ceilGridUnit(0.0, 0.5), 0.0f);
161+ QCOMPARE(units.ceilGridUnit(1.0, 0.5), 5.0f);
162+ QCOMPARE(units.ceilGridUnit(5.0, 0.5), 5.0f);
163+ QCOMPARE(units.ceilGridUnit(8.0, 0.5), 10.0f);
164+ QCOMPARE(units.ceilGridUnit(130.0, 0.5), 130.0f);
165+ QCOMPARE(units.ceilGridUnit(131.0, 0.5), 135.0f);
166+ QCOMPARE(units.ceilGridUnit(138.8, 0.5), 140.0f);
167+
168+ QCOMPARE(units.ceilGridUnit(0.0, 2.0), 0.0f);
169+ QCOMPARE(units.ceilGridUnit(1.0, 2.0), 20.0f);
170+ QCOMPARE(units.ceilGridUnit(5.0, 2.0), 20.0f);
171+ QCOMPARE(units.ceilGridUnit(8.0, 2.0), 20.0f);
172+ QCOMPARE(units.ceilGridUnit(130.0, 2.0), 140.0f);
173+ QCOMPARE(units.ceilGridUnit(131.0, 2.0), 140.0f);
174+ QCOMPARE(units.ceilGridUnit(138.8, 2.0), 140.0f);
175+ QCOMPARE(units.ceilGridUnit(141.0, 2.0), 160.0f);
176+ }
177+
178+ void roundGridUnitTen() {
179+ UCUnits units;
180+ units.setGridUnit(10);
181+
182+ QCOMPARE(units.roundGridUnit(0.0, 1.0), 0.0f);
183+ QCOMPARE(units.roundGridUnit(1.0, 1.0), 00.0f);
184+ QCOMPARE(units.roundGridUnit(5.0, 1.0), 10.0f);
185+ QCOMPARE(units.roundGridUnit(8.0, 1.0), 10.0f);
186+ QCOMPARE(units.roundGridUnit(12.0, 1.0), 10.0f);
187+ QCOMPARE(units.roundGridUnit(130.0, 1.0), 130.0f);
188+ QCOMPARE(units.roundGridUnit(131.3, 1.0), 130.0f);
189+ QCOMPARE(units.roundGridUnit(136.0, 1.0), 140.0f);
190+ QCOMPARE(units.roundGridUnit(138.8, 1.0), 140.0f);
191+
192+ QCOMPARE(units.roundGridUnit(0.0, 0.5), 0.0f);
193+ QCOMPARE(units.roundGridUnit(1.0, 0.5), 0.0f);
194+ QCOMPARE(units.roundGridUnit(5.0, 0.5), 5.0f);
195+ QCOMPARE(units.roundGridUnit(8.0, 0.5), 10.0f);
196+ QCOMPARE(units.roundGridUnit(12.0, 0.5), 10.0f);
197+ QCOMPARE(units.roundGridUnit(130.0, 0.5), 130.0f);
198+ QCOMPARE(units.roundGridUnit(131.0, 0.5), 130.0f);
199+ QCOMPARE(units.roundGridUnit(136.0, 0.5), 135.0f);
200+ QCOMPARE(units.roundGridUnit(138.8, 0.5), 140.0f);
201+
202+ QCOMPARE(units.roundGridUnit(0.0, 2.0), 0.0f);
203+ QCOMPARE(units.roundGridUnit(1.0, 2.0), 0.0f);
204+ QCOMPARE(units.roundGridUnit(5.0, 2.0), 0.0f);
205+ QCOMPARE(units.roundGridUnit(8.0, 2.0), 0.0f);
206+ QCOMPARE(units.roundGridUnit(12.0, 2.0), 20.0f);
207+ QCOMPARE(units.roundGridUnit(129.0, 2.0), 120.0f);
208+ QCOMPARE(units.roundGridUnit(130.0, 2.0), 140.0f);
209+ QCOMPARE(units.roundGridUnit(131.0, 2.0), 140.0f);
210+ QCOMPARE(units.roundGridUnit(136.0, 2.0), 140.0f);
211+ QCOMPARE(units.roundGridUnit(138.8, 2.0), 140.0f);
212+ QCOMPARE(units.roundGridUnit(141.0, 2.0), 140.0f);
213+ }
214+
215 void resolveEmpty() {
216 UCUnits units;
217 QString resolved;

Subscribers

People subscribed via source and target branches

to status/vote changes: