Ticket UUID: | c9887a1fc9bbf2712f1f8eed6bf57178521722b8 | |||
Title: | Trailing zeros on scale widget ticks | |||
Type: | Bug | Version: | 8.6.6 | |
Submitter: | jal_frezie | Created on: | 2018-12-18 10:30:45 | |
Subsystem: | 15. [scale] | Assigned To: | fvogel | |
Priority: | 5 Medium | Severity: | Cosmetic | |
Status: | Closed | Last Modified: | 2019-03-31 13:58:54 | |
Resolution: | Fixed | Closed By: | fvogel | |
Closed on: | 2019-03-31 13:58:54 | |||
Description: |
Make a scale like this: % pack [scale .s -from 0.03 -to 0.1 -tickinterval 0.01 -resolution 0.0001 -length 360 -orient h] ...and the ticks are numbered 0.0300, 0.0400 etc which is confusing because the numbers run into each other. If you set tickinterval to 0.01 they look right but you cannot select a value between the ticks. | |||
User Comments: |
fvogel added on 2019-03-31 13:58:54:
Merged to core-8-6-branch and trunk. Many thanks for your report and implementation, Jasper! fvogel added on 2019-03-24 16:24:11: TIP #535 is now accepted through TCT vote. jal_frezie added on 2019-03-11 15:21:45: That looks fine to me, let's go ahead and vote on it. fvogel added on 2019-03-10 13:30:14: I see you have committed explanations in the man page, thanks! On my side I have added TIP #535. Let me know if you'd like to change this text, or if you think it's ready for a vote. fvogel added on 2019-03-04 20:55:29: Right, sorry. I didn't see it yesterday, but the branch does not build for me on Windows with MSVC. Therefore I must have tested with an old binary, leading to odd results. I have fixed the build in [69514b20], please have a look. And now all examples I have provided look right, and the scale.test file passes 100% OK on Windows. I think the last thing we need is to say a word about this new feature in the man page, and change the documentation for -resolution since it's no longer correct. Also we should try to test the new feature if you think this is possible. I'm afraid the change requires a TIP. Indeed we have a strong tradition of trying very hard to not break any user application in the wild. While change is very likely to be viewed only as a fix (useless trailing zeroes are removed), one cannot exclude someone wanted these zeroes to be displayed for some odd reason. I can help with the TIP easily, no worries. jal_frezie added on 2019-03-04 09:18:40: That's odd, both those examples work perfectly for me using current scale-tick-format branch. If you have the right code, your generic/tkScale.c should contain a procedure called MaxTickRoundingError. fvogel added on 2019-03-03 20:29:07: Trying the latest branch tip, I'm not seeing improvement with: package require Tk pack [scale .s -from 0.034 -to 0.1 -tickinterval 0.01 -resolution 0.0001 -length 360 -orient h] The left tick is still displayed as 0.03 whereas I would have expected 0.034 since the leftmost position of the slider is 0.034. Also trying this: package require Tk pack [scale .s -to 0.03 -from 0.1 -tickinterval 0.01 -resolution 0.0001 -length 360 -orient h] produces wrong results (all tick marks are 0.1 (6 times) or 0.0 (twice). In core-8-6-branch there are no such wrong behaviors, these two cases behave as on would expect. jal_frezie added on 2019-02-28 16:30:32: OK, I have added a routine to add extra digits to the tick values as required to get the maximum rounding error below any given value and updated my branch to include this, with max error currently 0.2x the tick interval. This fixes the scale example below, without adding extra digits when they are not needed. Also sorted out problems in the last version with scales from higher to lower values. For some reason my commits now appear to be from jaspert rather than jal_frezie. Will have a look at man page next. (I'm helping Marc resolve the 8.6.9 problems) fvogel added on 2019-02-26 11:29:38: Depending on the nature of the issues you're experiencing, either the tcl-core list (development discussions) or the comp.lang.tcl newsgroup (usage discussions, more or less) are appropriate. jal_frezie added on 2019-02-26 11:11:40: I have all sorts of problems with core-8-6-branch and the 8.6.9 releases but here is not the place to discuss them. Can I message you privately or go somewhere on the forum? fvogel added on 2019-02-25 20:35:44: Right, the display procedure for scale widgets is TkpDisplayScale in unix/tkUnixScale.c for all three pletforms. This explains why it works on all three platforms. About macOS 10.14 Mojave, the black windows have been fixed with 8.6.9 (and better in 8.6.9.1). Alternatively, just use core-8-6-branch to build a much improved macOS version (thanks to Marc Culler for his huge work). See details about this black windows issue in [09e18e42d7] and [821dbe47e1]. jal_frezie added on 2019-02-25 18:51:48: I have an idea for an algorithm that would work out how many digits were needed to display tick values to an accuracy of a given fraction of a tick interval, which would fix this example and other odd cases like a tick interval of 0.015. Some minor changes to the man page as you describe could well be needed. I'll be happy to make suggestions. Tests could use the fact that the reqwidth of a vertical scale will now be affected by the number of digits in the tick values. I developed the code on MacOS; I assumed that since OSX is built on Darwin, which is a kind of Unix, it would also use tkUnixScale.c. However I'm having a lot of problems actually building a good version of Tk; our product currently bundles Tk 8.6.8 which I built on OSX 10.13. If I try to build that release on 10.14 I just get black windows; do I need to install an earlier version of Xcode tools? fvogel added on 2019-02-23 14:59:28: OK, I see now what you're after. The code looks nice and it passes all tests, that's a very good start. An I see the improvement in readability of the scale widget. Actually your implementation is not trimming trailing zeroes (of the ticks values - nothing else is touched), it displays the ticks values such that they have just enough digits to display two adjacent ticks as strings differently (at least one digit must change between adjacent ticks). This is a good idea, however it's not the same as trimming trailing zeros. For instance try: package require Tk pack [scale .s -from 0.034 -to 0.1 -tickinterval 0.01 -resolution 0.0001 -length 360 -orient h]--> the left tick is displayed as 0.03, however the leftmost position of the slider is 0.034. This is a bit inconsistent. The unchanged scale widget does not exhibit this incorrect effect. How can we avoid this? Should we live with this perhaps, given the gain that the fix provides? Regarding the man page, do you think any change would be needed with your solution? At first sight, the man for -digits does not need a change however description of -resolution needs an update I think (rounding is no longer done for tick marks "to an even multiple of this value"). Also could we craft some new tests exercising this? I don't think so since the change only applies to the _display_ of tick marks, which cannot be retrieved from the widget, but perhaps you see some way of testing this? Also, did you test with other bounds (large -from/to) than the originally given test script? And/or with wegative bounds? Larger -from than -to? With -digits? With -tickinterval being -1? And so on. I did (a bit), and couldn't find any other oddity than the one above. Finally, regarding the implementation: it works on Windows because Windows uses the Linux code (from tkUnixScale.c) for displaying the scale widget. That's OK. But how come that the fix is OK on macOS too (I checked: it works on macOS)? scalePtr->tickFormat is not passed at all to the macOS specific code I think. jal_frezie added on 2019-02-20 12:02:08: OK, I've sorted it now. Branch checked in is scale-tick-format. jal_frezie added on 2019-02-20 11:27:02: Sorry, how do I identify myself to the server? fossil push -B|--httpauth jal_frezie:PASS doesn't seem to work (Error: not authorised to write) fvogel added on 2019-02-20 10:57:18: I have now given you commit access to the Tk repository. Please commit in a branch and let's review. jal_frezie added on 2019-02-20 10:08:44: OK, I have updated the code to do what I want, as follows: Eliminate trailing zeros from scale tick values. This is done by calculating a separate format string for these values, rather than using the same format string as for the current value. The basis is the same, i.e., enough digits are displayed to distinguish any pair of adjacent tick values, and all tick values have the same number of decimals so some may still have trailing zeros. Code for laying out vertical scales has been adjusted to take account of the fact that the tick column may now be narrower than the value column. The new version passes all tests that the core-8-6-branch passes, including all scale widget tests. I tried to commit it as a branch in the repository, but I don't have permission. Sure I can go ahead and use it in my own projects, but I'd quite like to see it adopted generally -- so what do I do next? fvogel added on 2019-02-13 13:26:54: I'm not sure what your proposal is and whether it needs a TIP or not. However I have proposed three solutions in my post below dated Jan. 04, 2019, all of them repairing a TIP. Is your proposal different? Anyway I suggest you try the implementation you have in mind and we'll see if a TIP is needed or not (if so I can help with the TIP). jal_frezie added on 2019-02-13 12:13:19: I think I am gradually catching on to the implications of this. All my scales are vertical, but if you have a horizontal scale then the tick values line up in a neat column next to the current value. I can see why they should all have the same number of digits, as should all the possible current values. However I don't see that the tick values should have the same number of digits as the current value if that means they all have trailing zeros, so I would suggest calculating separate format strings for the ticks vs the current value. I would want this to be the default behaviour. Would that require a TIP? If not, I shall have a look at implementing it myself. jal_frezie added on 2019-01-22 17:45:58: I was thinking of using the standard Tcl mechanism Tcl_PrintDouble in generic/tclUtil.c to generate the tick strings, which trims trailing zeros, but that uses a global variable to specify the precision so could be awkward. Also it looks like the rest of the scale widget code computes the required number of decimal places rather than significant figures. But there is an explicit mechanism for removing trailing zeros in that procedure -- perhaps we could just duplicate that for the tick values? fvogel added on 2019-01-06 09:24:44: > formatted in the way that any other number is when it is converted to a string without the format command Sorry, this does not make sense to me. How do you do that in C (give a printf example please)? There is no format command in play here (I mean: the format command of Tcl). There is a format specification for the printf C instruction. jal_frezie added on 2019-01-05 20:47:32: I'm quite happy for the current value to be displayed as it is now, it is only the tick values that seem wrong. Would it be so hard to have them formatted in the way that any other number is when it is converted to a string without the format command? That seems to get it right nearly all the time. fvogel added on 2019-01-04 16:30:01: The scale widget tries to be clever when displaying numbers in its ticks and scale value. From the -digits, -from, -to, -resolution and -length options values, it computes a format parameter that respects the -digits and/or -resolution specification, or if -digits is not given will be such that different positions of the scale slider will result in different string values of the scale. It then chooses the %f or %e type specification based on whichever results in a shorter string. The resultinf format specification always is "%.Nf" or "%.Ne" where N is a number denoting a precision. See ComputeFormat() in tkScale.c for more details. This format parameter is then used to snprintf both the ticks numbers and the value on top (for horizontal scales) or left (for vertical scales) of the scale. For the given example the format parameter computed by the scale widget is "%.4f". This is why you're seeing ticks as 0.0300 0.0400 0.0500 ... 0.1000, and a scale value using 4 decimal digits as well (e.g. 0.4158). Your point, as I understand it, is that non significant zeroes at the right of those values should not be displayed. Regarding the scale value, I just disagree. We have specified a -resolution of 0.0001, and we really shall display four digits past the decimal point since that is what was requested. Regarding the ticks, I can perhaps understand the need. The question is, how can we do that in a backwards compatibility way? I can see the following poposals: 1. The first thing that comes to mind is to always use %g for the type parameter (instead of %f or %e). Use this for both the ticks and the value. Consequence: the number of digits displaying the value will change depending on the slider position. This would not be backwards compatible at all (-digits contract is changed, in particular, and 48 tests fail among the 136 from scale.test). Obviously it would require a TIP. 2. The format parameter computed by the scale widget is unchanged but is only used for the scale value. For the ticks, that same parameter is used with an additional processing removing the trailing zeroes (I don't think this trimming can be done directly by a better format parameter). A new scale widget option, say -trimzeroes, defaulting to false, will enable the new feature or not. Note that the trimming will not be trivial due to the two possible type specification (%f and especially %e). This would be backwards compatible. The new option requires a TIP. 3. Forget about cleverness in the code and provide a way to let the user decide. That is, add a new parameter -ticksformat (and perhaps -valueformat) that directly specify the printf format for the ticks (and perhaps the value). When set to "" there would be no change in the display of those numbers. Those new options would default to "". This would be backwards compatible. Again the new options require a TIP. fvogel added on 2018-12-30 22:52:49: OK, clear enough now. The source code says it all (and this is also documented for the -digits option): when -digits is not specified, the format used to display the numbers when drawing the scale is computed so that at least one digit will be different between any two adjacent positions of the scale. This format is used for printing the value above the scale (and this is needed and correct, otherwise the user would wonder what the scale value really is since it wouldn't necessarily change for adjacent positions of the slider), and also for printing the ticks values. I understand you're questioning the latter only. This is more Cosmetic than Severe (I have just decreased the severity field accordingly). jal_frezie added on 2018-12-30 22:06:25: You're right, I meant to say specifying -resolution affected the trailing zeros. The bug is that trailing zeros appear on decimals at all; they do not appear in Tcl's default rendering of numbers, and are bad style. Numbers running into each other unnecessaarily is just a nasty consequence of this bug. fvogel added on 2018-12-30 16:52:38: This ticket is confusing to me. First of all, running the provided script (on Windows) does not show any problem for me. In particular the ticks numbering do not run into each other. The default font size used is small enough to avoid that effect given the -length you specified. However I can obviously achieve the undesired effect if I specify for instance "-length 100", or "-font Arial". Shouldn't it be the programmer's responsibility to specify a consistent {-tickinterval ; -length ; -font} triplet? Second, you say "If you set tickinterval to 0.01 they look right [...]": but this is already set. I think you rather meant "If you set resolution to 0.01 they look right but you cannot select a value between the ticks" - and this is correct and expected. So what is the problem, exactly? That the ticks numbering uses 4 digits after the "0." whereas -tickinterval specifies 0.01 (i.e. 2 digits only)? In other words, is the problem the fact that the scale widget renders ticks numbering with the -resolution accuracy instead of the -tickinterval accuracy? If so, that was intentional in the code. bll added on 2018-12-18 21:43:31: Right. scale and ttk::scale both need a -format option. jal_frezie added on 2018-12-18 16:24:34: Setting -digits 2 has same effect as setting -tickinterval 0.01, i.e., it fixes the zeros but you cannot select a value between ticks. bll added on 2018-12-18 15:22:37: Try the -digits option. |