View Issue Details

IDProjectCategoryView StatusLast Update
0003266Slicer4Core: Base Codepublic2014-03-06 05:23
Reporterjcfr Assigned Tofinetjul  
PrioritynormalSeverityminorReproducibilityhave not tried
Status closedResolutionfixed 
Product Version 
Target VersionSlicer 4.3.0Fixed in VersionSlicer 4.3.0 
Summary0003266: Check that CTK topic "ctkvalueproxy" do not cause issue
Description

From Julien:

The first part of the "dimensionality" problem (0001694, 0002973, 0002480...) is already part of Slicer 4.3:
As of now possible to control (via settings) the default number of decimals for "length" aware widgets. Moreover
the number of decimals is dynamic in a sense that it adapts to best fit the input value. The user is also able to
edit that precision on each widget individually.

The second part is to be able to change the order of magnitude of the values displayed to the user. This second part
is soon to be ready to be merged into Slicer. It is however still a bit experimental.

According to the release schedule (see [1]) , to the value of the feature and to the potential impact on the code
base, is it something that can still go in the release. Or should we wait the next release ?

Note that if the user does not play with the units, it should not bring too much regression (part 0000002 comes primarily
from CTK and is not really MRML related).

// --------------------------------------------
From Jc:

I think we should remain conservative and focus on the issue currently targeted for 4.3.

To help us take a final decision, few questions / remarks:

  • Better management of units and dimensionality for 4.3 would be great

  • Could you share topic(s) illustrating the extent of the changes ?

    CTK: https://github.com/finetjul/CTK/commits/ctkvalueproxy
    Slicer: https://github.com/finetjul/Slicer/commits/ProxyValueForUnits

  • Can you confirm that this new set of commit will behave as expected after the work of Nicole related
    to "Markups" module will be integrated ?
    It will still works fine with the Annotations, however, it might require some minor work to work with
    the markups module

  • Can you confirm that it build on MacOSX/ Linux / Windows ... doing Experimental builds on the factory
    could be used to confirm this.
    Have tried Linux and Windows, not macosx yet

  • Any impact on existing extensions ?
    It should not impact extensions.

TagsNo tags attached.

Relationships

related to 0003261 closedfinetjul qMRMLVolumeInfoWidget unexpectedly changes the volume image spacing and image origin to lower precision 

Activities

yuzheng

yuzheng

2013-07-31 11:04

developer   ~0009309

I found two issues related to the CTK topic ctkvalueproxy.
CTK: https://github.com/finetjul/CTK/commits/ctkvalueproxy [^]
Slicer: https://github.com/finetjul/Slicer/commits/ProxyValueForUnits [^]

1) It introduces lots of "Single Step ... is out of bounds" warning messages in Slicer's output when Slicer starts, a volume is loaded, etc.

2) It fixed the original issue 0002973, but when user types a small number with higher precision than currently displayed number, the displayed value in other fields are still not correct.
E.g.

  • download MRHead. Its space origin is (86.644897460937486,-133.92860412597656,116.78569793701172) [open the file and check the meta data in its header].
  • go to Volumes module, Information fields. (-86.645, 133.929, 116.786) is displayed in Image Origin field (so far, so good)
  • type 133.9301 in second Image Origin field, the first Image Origin field is then updated to -86.6450. In other words, the first Image Spacing field only appends 0 at its end. Should it display -86.6449 which is the actual round-off from 86.644897460937486?
jcfr

jcfr

2013-07-31 18:40

administrator   ~0009313

Reminder sent to: finetjul

Hi Julien,

Could you have a look at note 9309 ?

Thanks

finetjul

finetjul

2013-08-01 07:00

administrator   ~0009328

1) I'll fix it soon
2) It is a limitation of QDoubleSpinBox (and for that matter ctkDoubleSpinBox and ctkCoordinatesWidget): input values are internally rounded to the number of decimals. A solution would be to save the input value (and probably the input minimum and maximum) and trigger signals only when needed. This technique has already been done in ctkDoubleSlider (to convert from double to int). Note that now value proxy are in the game, the stored value should be the input value (before the proxy is applied).

jcfr

jcfr

2013-08-01 07:36

administrator   ~0009329

Last edited: 2013-08-01 07:40

What about setting the number of decimals doing something like:

this->setDecimals(16);

finetjul

finetjul

2013-08-01 07:55

administrator   ~0009331

Because it will enforce 16 decimals no matter what.
So if you have 1,1,1, then it will render "1.0000000000000000, 1.0000000000000000, 1.0000000000000000" which takes a lot of space and clutter the view

jcfr

jcfr

2013-08-02 08:06

administrator   ~0009355

But it would at least be accurate and allow us to move forward with the other issues. When done, we could then address the issue of decoupling displayed value from internal value.

An other option would be to display the truncated value in a label, and when the user click on the "link" we would popup a value editor with full precision.

finetjul

finetjul

2013-08-02 08:21

administrator   ~0009358

I would prefer subsolution 2) (edit in a popup) rather than subsolution1)
But the time spent to implement it might be better spent in fixing the root cause.

finetjul

finetjul

2013-08-07 11:43

administrator   ~0009424

Please try https://github.com/finetjul/Slicer/commit/14a1b7743244e3d9d665ae1497327e41f2fe7a85

yuzheng

yuzheng

2013-08-08 09:50

developer   ~0009430

Last edited: 2013-08-08 09:55

Nice work! The issues in note 0009309 have been fixed.

finetjul

finetjul

2013-08-09 11:52

administrator   ~0009447

Fixed in r22281

jcfr

jcfr

2014-03-06 05:21

administrator   ~0011162

Closing resolved issues that have not been updated in more than 3 months

Issue History

Date Modified Username Field Change
2013-07-31 08:37 jcfr New Issue
2013-07-31 08:37 jcfr Status new => assigned
2013-07-31 08:37 jcfr Assigned To => jcfr
2013-07-31 08:38 jcfr Target Version => Slicer 4.3.0
2013-07-31 08:38 jcfr Relationship added related to 0003261
2013-07-31 11:04 yuzheng Note Added: 0009309
2013-07-31 18:39 jcfr Assigned To jcfr => yuzheng
2013-07-31 18:40 jcfr Note Added: 0009313
2013-08-01 07:00 finetjul Note Added: 0009328
2013-08-01 07:36 jcfr Note Added: 0009329
2013-08-01 07:40 jcfr Note Edited: 0009329
2013-08-01 07:55 finetjul Note Added: 0009331
2013-08-02 08:06 jcfr Note Added: 0009355
2013-08-02 08:21 finetjul Note Added: 0009358
2013-08-07 07:14 yuzheng Assigned To yuzheng => finetjul
2013-08-07 11:43 finetjul Note Added: 0009424
2013-08-08 09:50 yuzheng Note Added: 0009430
2013-08-08 09:55 yuzheng Note Edited: 0009430
2013-08-09 11:52 finetjul Note Added: 0009447
2013-08-09 11:52 finetjul Status assigned => resolved
2013-08-09 11:52 finetjul Fixed in Version => Slicer 4.3.0
2013-08-09 11:52 finetjul Resolution open => fixed
2014-03-06 05:21 jcfr Note Added: 0011162
2014-03-06 05:23 jcfr Status resolved => closed