> On Feb. 18, 2016, 1:05 vorm., David Edmundson wrote: > > applets/digital-clock/package/contents/ui/DigitalClock.qml, line 555 > > <https://git.reviewboard.kde.org/r/127102/diff/1/?file=444552#file444552line555> > > > > rather than looping, can we use FontMetric's maximumCharacterWidth > > > > * numChars. > > > > Then we could kill sizeHelper competely (FontMetric's didn't exist when > > this was written) > > Marco Martin wrote: > hoping maximumCharacterWidth is reliable for all fonts, this loop really > needs to go > > Daniel Faust wrote: > As far as I understand maximumCharacterWidth returns the width of the > widest character of the font - which can be ridiculously wide given that the > font supports some wild unicode characters. > I did a quick test and maximumCharacterWidth returned about twice the > width actually needed. > > Martin Klapetek wrote: > You still don't need this whole loop though, just find the biggest in 0-9 > and use that for all the numbers (except year). > > Daniel Faust wrote: > Since I don't know the time format in advance, I have to construct > different times and process them with Qt.formatTime. > That means, I have to test 0-9 for hours/minutes/seconds, 0-2 for tens of > hours and 0-5 for tens of minutes/seconds. Otherwise the constructed times > will be invalid. > This can be done with three loops and it would bring down the amount of > advanceWidth and formatTime calls from 24+60=84 to 3+6+10=19. > > Martin Klapetek wrote: > You don't actually need them as a time, you just need them as a > placeholder. At which point it doesn't matter if it's a valid time or not. > You can even format 12:12 with Qt.formatTime and then replace the numbers > with your placeholder number. > > In other words, the time format /is/ known in advance. > > Daniel Faust wrote: > I'm not sure if I understand you, I just tried it with a single loop from > 0-9 to construct the time strings: > > for (var i=0; i<=9; i++) { > var str = Qt.formatTime(new Date(2000, 0, 1, i*10 + i, i*10 + i, i*10 + > i), main.timeFormat); > console.log("hour: " + (i*10 + i) + ", minute: " + (i*10 + i) + " -> " > + str); > } > > qml: hour: 0, minute: 0 -> 00:00 > qml: hour: 11, minute: 11 -> 11:11 > qml: hour: 22, minute: 22 -> 22:22 > qml: hour: 33, minute: 33 -> 09:33 > qml: hour: 44, minute: 44 -> 20:44 > qml: hour: 55, minute: 55 -> 07:55 > qml: hour: 66, minute: 66 -> 19:07 > qml: hour: 77, minute: 77 -> 06:18 > qml: hour: 88, minute: 88 -> 17:29 > qml: hour: 99, minute: 99 -> 04:40 > > Thomas Lübking wrote: > Forget about the timeFormat - you iterate from 0-9 and figure the widest > glyphs in [0,1], [0,5] and [0,9] - let's reasonably say "0,3 and 7". > Then you check the width for 00:37 > > (If you also need the date, you also need to check for the widest glyph > in [0,3]) > > Daniel Faust wrote: > That would work if it wasn't for 12 hour time formats. > For 24 hour formats you actually need [0,2], [0,3] and [0,9] for the hour > and [0,5] and [0,9] for the minutes/seconds. > For 12 hour formats AM you need [0,1], [0,2] and [0,9] for the hour and > [0,5] and [0,9] for the minutes/seconds. > For 12 hour formats PM you need [1,2], [3,9] and [0,3] for the hour and > [0,5] and [0,9] for the minutes/seconds. > On top of that you need to account for format changes like hour=0, > minute=0 -> "12:00 AM". > > The code for this logic would be hard to maintain. > > Daniel Faust wrote: > Ok, what about this? > Find the widest glyph between 0 and 9. > Use a regex to replace [hmsz] from the format string with the widest > number. (This will produce strings like "44:44") > Use Qt.formatTime once with an AM time and once with a PM time and choose > the widest of the produced strings. > This code is fairly short and overestimates the total width only slightly. > > Thomas Lübking wrote: > As long as the formatter accepts invalid times, that's certainly the most > straight forward solution, yes.
So what do you think about the changes I made (revision 2)? - Daniel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127102/#review92514 ----------------------------------------------------------- On Feb. 19, 2016, 6:11 nachm., Daniel Faust wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127102/ > ----------------------------------------------------------- > > (Updated Feb. 19, 2016, 6:11 nachm.) > > > Review request for kde-workspace and Plasma. > > > Bugs: 347724 > https://bugs.kde.org/show_bug.cgi?id=347724 > > > Repository: plasma-workspace > > > Description > ------- > > Currently the width of the date label is not fixed but changes depending on > the text. This causes the entire applet to change its width (if the time is > the widest displayed item). This in turn can cause all other applets in the > same panel to move whenever the displayed time changes. > > This patch uses FontMetrics to iterate over all possible time strings (with > different width) and chooses the widest of them as reference for the fixed > width of the time label. > > This way the width of the applet stays the same (unless the date is displayed > and changes). The text remains centered though, which means that it can still > move within the applet when the time changes. > > > Diffs > ----- > > applets/digital-clock/package/contents/ui/DigitalClock.qml 95bb071 > > Diff: https://git.reviewboard.kde.org/r/127102/diff/ > > > Testing > ------- > > Works with horizontal and vertical panel. > Also displaying different combinations of "seconds", "date" and "timezone" > works. > > > Thanks, > > Daniel Faust > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
