Merge lp:~seb128/ubuntu-system-settings/battery-draw-x-text-1289460 into lp:ubuntu-system-settings

Proposed by Sebastien Bacher
Status: Merged
Approved by: Iain Lane
Approved revision: 780
Merged at revision: 787
Proposed branch: lp:~seb128/ubuntu-system-settings/battery-draw-x-text-1289460
Merge into: lp:ubuntu-system-settings
Diff against target: 130 lines (+59/-15)
1 file modified
plugins/battery/PageComponent.qml (+59/-15)
To merge this branch: bzr merge lp:~seb128/ubuntu-system-settings/battery-draw-x-text-1289460
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing
Iain Lane Approve
Review via email: mp+225856@code.launchpad.net

Commit message

battery, draw the x axis labels

Description of the change

battery, draw the x axis labels

To post a comment you must log in.
Revision history for this message
Sebastien Bacher (seb128) wrote :

The code might look a bit complex, but turns out it's not trivial to write on a canvas. The results to be decent on desktop and phone configs, from my testing

Revision history for this message
Iain Lane (laney) wrote :

Thanks.

I think you could have avoided doing so many manipulations involving the margins if you had used a combination of translating and scaling to avoid them. But it's done now so don't bother changing.

 - On N4, I made the "Today" label go off the end by setting the time to 01:53 (ish).
 - Please wrap long lines.

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:775
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/936/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/1640
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/1388
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-amd64-ci/128
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/128
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/128/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/128
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/1913
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2662
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2662/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9403
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1151
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1555
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1555/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/936/rebuild

review: Approve (continuous-integration)
776. By Sebastien Bacher

correctly calculate the space for the text

777. By Sebastien Bacher

Wrap some long lines

778. By Sebastien Bacher

don't check for 0 value, we start at 1

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:777
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/944/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/1709
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/1447
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-amd64-ci/136
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/136
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/136/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/136
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/1974
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2752
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2752/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9482
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1205
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1616
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1616/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/944/rebuild

review: Approve (continuous-integration)
779. By Sebastien Bacher

draw the first mark as well

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:779
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/945/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/1716
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/1452
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-amd64-ci/137
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/137
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/137/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/137
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/1979
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2759
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2759/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9488
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1210
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1621
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1621/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/945/rebuild

review: Approve (continuous-integration)
Revision history for this message
Iain Lane (laney) wrote :

I ran it (possibly with more data this time than last). There's a problem with the x coordinate for the previous day's data.

  http://people.canonical.com/~laney/weird-things/battery.png

And do you know if it's possible to smooth the text? If you size that image to 100% you can see that the text is jagged and not very nice looking.

review: Needs Fixing
780. By Sebastien Bacher

revert change that seemed logical but is creating issues

Revision history for this message
Sebastien Bacher (seb128) wrote :

> I ran it (possibly with more data this time than last). There's a problem with the x coordinate for the previous day's data.
> http://people.canonical.com/~laney/weird-things/battery.png

Ok, I reverted the change that created that. The formular there looks weird but it works, we can fix in another change

> And do you know if it's possible to smooth the text? If you size that image to 100% you can see that the text is jagged and not very nice looking.

I didn't find any while looking in the canvas api, not sure what to do there ...

Revision history for this message
Iain Lane (laney) wrote :

Alright, looks fine for now.

I think the line drawing code is a bit suspicious but that's pre-existing.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:780
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/953/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/1760
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/1480
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-amd64-ci/145
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/145
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-armhf-ci/145/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/145
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2021
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2820
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2820/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9552
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1236
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1662
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1662/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/953/rebuild

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'plugins/battery/PageComponent.qml'
--- plugins/battery/PageComponent.qml 2014-01-27 18:12:16 +0000
+++ plugins/battery/PageComponent.qml 2014-07-09 15:03:35 +0000
@@ -127,43 +127,84 @@
127 id: canvas127 id: canvas
128 width:parent.width - units.gu(4)128 width:parent.width - units.gu(4)
129 anchors.horizontalCenter: parent.horizontalCenter129 anchors.horizontalCenter: parent.horizontalCenter
130 height: units.gu(15)130 height: units.gu(23)
131131
132 antialiasing: true132 antialiasing: true
133133
134 function drawAxes(ctx, axisWidth, axisHeight) {134 function drawAxes(ctx, axisWidth, axisHeight, bottomMargin, rightMargin) {
135
136 var currentHour = Qt.formatDateTime(new Date(), "h")
137 var currentMinutes = Qt.formatDateTime(new Date(), "m")
138 var displayHour
139 var labelWidth
140 var zeroMark
141
135 ctx.save()142 ctx.save()
136 ctx.beginPath()143 ctx.beginPath()
137 ctx.strokeStyle = UbuntuColors.lightAubergine144 ctx.strokeStyle = UbuntuColors.lightAubergine
138145
139 ctx.lineWidth = units.dp(2)146 ctx.lineWidth = units.dp(2)
140147
148 var fontHeight = FontUtils.sizeToPixels("small")
149 ctx.font="%1px Ubuntu".arg(fontHeight)
150
141 ctx.translate(0, 1)151 ctx.translate(0, 1)
142152
143 // 11 ticks with 0, 5, 10 being big153 // 11 ticks with 0, 5, 10 being big
144 for (var i = 0; i <= 10; i++) {154 for (var i = 0; i <= 10; i++) {
145 var x = (i % 5 == 0) ? 0 : Math.floor(axisWidth / 2)155 var x = (i % 5 == 0) ? 0 : Math.floor(axisWidth / 2)
146 var y = (i / 10) * (height - axisHeight - ctx.lineWidth)156 var y = (i / 10) * (height - axisHeight - bottomMargin - ctx.lineWidth)
147 ctx.moveTo(x, y)157 ctx.moveTo(x, y)
148 ctx.lineTo(axisWidth, y)158 ctx.lineTo(axisWidth, y)
149 }159 }
150160
151 ctx.translate(axisWidth + ctx.lineWidth / 2,161 ctx.translate(axisWidth + ctx.lineWidth / 2,
152 height - axisHeight - ctx.lineWidth / 2)162 height - axisHeight - bottomMargin - ctx.lineWidth / 2)
153163
154 ctx.moveTo(0, 0)164 ctx.moveTo(0, 0)
155 ctx.lineTo(0, -ctx.lineWidth)165 ctx.lineTo(0, -ctx.lineWidth)
156166
157 // 25 ticks with 0, 6, 12, 18, 24 being big167 // 24 ticks with 6, 12, 18, 24 being big
158 for (i = 0; i <= 24; i++) {168 for (i = 0; i <= 24; i++) {
159 x = (i / 24) * (width - axisWidth - ctx.lineWidth)169 /* the marks need to be shifted on the hours */
170 x = ((i - currentMinutes / 60) / 24) * (width - axisWidth - ctx.lineWidth - rightMargin)
171 if (x < 0)
172 continue
160 y = (i % 6 == 0) ? axisHeight : axisHeight -173 y = (i % 6 == 0) ? axisHeight : axisHeight -
161 Math.floor(axisHeight / 2)174 Math.floor(axisHeight / 2)
162 ctx.moveTo(x, 0)175 ctx.moveTo(x, 0)
163 ctx.lineTo(x, y)176 ctx.lineTo(x, y)
177
178 /* Determine the hour to display */
179 displayHour = (currentHour - (24-i))
180 if (displayHour < 0)
181 displayHour = displayHour + 24
182 /* Store the x for the day change line */
183 if (displayHour === 0)
184 zeroMark = x
185
186 /* Write the x-axis legend */
187 if (i % 6 == 0) {
188 labelWidth = context.measureText("%1".arg(displayHour)).width;
189 ctx.fillText("%1".arg(displayHour),
190 x - labelWidth/2,
191 axisHeight + units.dp(1) + fontHeight)
192 }
164 }193 }
165194
166 ctx.translate(0, 0)195 labelWidth = context.measureText(i18n.tr("Yesterday")).width;
196 if(labelWidth < zeroMark)
197 ctx.fillText(i18n.tr("Yesterday"),
198 (zeroMark - labelWidth)/2,
199 axisHeight + units.dp(6) + 2*fontHeight)
200
201 ctx.fillText("|", zeroMark, axisHeight + units.dp(6) + 2*fontHeight)
202
203 labelWidth = context.measureText(i18n.tr("Today")).width;
204 if(labelWidth < (width - zeroMark - rightMargin - axisWidth - ctx.lineWidth))
205 ctx.fillText(i18n.tr("Today"),
206 zeroMark + (width - zeroMark - labelWidth)/2,
207 axisHeight + units.dp(6) + 2*fontHeight)
167208
168 ctx.stroke()209 ctx.stroke()
169 ctx.restore()210 ctx.restore()
@@ -174,12 +215,14 @@
174 ctx.save();215 ctx.save();
175 ctx.clearRect(0, 0, canvas.width, canvas.height)216 ctx.clearRect(0, 0, canvas.width, canvas.height)
176217
177 var axisWidth = 10218 var axisWidth = units.gu(1)
178 var axisHeight = 10219 var axisHeight = units.gu(1)
179220
180 var graphHeight = height - axisHeight221 /* Space to write the legend */
181222 var bottomMargin = units.gu(6)
182 drawAxes(ctx, axisWidth, axisHeight)223 var rightMargin = units.gu(1)
224
225 drawAxes(ctx, axisWidth, axisHeight, bottomMargin, rightMargin)
183226
184 /* Display the charge history */227 /* Display the charge history */
185 ctx.beginPath();228 ctx.beginPath();
@@ -191,10 +234,11 @@
191 // Invert the y axis so we draw from the bottom left234 // Invert the y axis so we draw from the bottom left
192 ctx.scale(1, -1)235 ctx.scale(1, -1)
193 // Move the origin to just above the axes236 // Move the origin to just above the axes
194 ctx.translate(axisWidth, axisHeight)237 ctx.translate(axisWidth, axisHeight + bottomMargin)
195 // Scale to avoid the axes so we can draw as if they aren't238 // Scale to avoid the axes so we can draw as if they aren't
196 // there239 // there
197 ctx.scale(1 - (axisWidth / width), 1 - axisHeight / height)240 ctx.scale(1 - ((axisWidth + rightMargin) / width),
241 1 - (axisHeight + bottomMargin) / height)
198242
199 var gradient = ctx.createLinearGradient(0, 0, 0, height);243 var gradient = ctx.createLinearGradient(0, 0, 0, height);
200 gradient.addColorStop(1, "green");244 gradient.addColorStop(1, "green");

Subscribers

People subscribed via source and target branches