Oh no, I can remember similar issues with German Ubuntu and "Arbeitsfläche" ... Basically, CI clients should probably always have a path compontent like /bäh / included to catch new issues like this one, and build system stuff, which is also a candidate.
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
All Stories
Feb 8 2021
Additional info: This is not a DICOM reader problem in the first place, as the filename string encoding seems to be unexpected (see screenshot above). Other readers work but the node still appears with a dummy symbol in the data node name.
Feb 7 2021
Deactivated for 4D images as a work arround fix for the release. Will be removed if this tool is properly refactored.
Feb 5 2021
In T28283#219736, @nolden wrote:I was hoping a bit to include it in the release, if the release would e.g. be used for kaapana containers for a longer time this could be helpful at some point. But of course it's not strictly necessary.
I can understand the reasoning. But adding this feature last minute would basically mean to move the release again and also to test again. And we should realy try to get into the time based pattern.
In T28283#219738, @nolden wrote:I don't know why this was not linked automatically: https://phabricator.mitk.org/D469
I don't know why this was not linked automatically: https://phabricator.mitk.org/D469
I was hoping a bit to include it in the release, if the release would e.g. be used for kaapana containers for a longer time this could be helpful at some point. But of course it's not strictly necessary.
Valgrind shows some reading beyond memory boundaries, around mitkCoreActivator.cpp:129 and 218. Since it is the last line of all the AddPropertyPersistence calls there and all the creating of temp objects in there, I was wondering if it could be some reference to deleted temp objects issues. Just an idea.
It's the CoreActivator if I read this correcty:
In T28282#219716, @nolden wrote:@floca the comment I mention was written by you, line 112, any spontaneous idea?
If this is caused in UseRegEx() then its preconditions are not meet anymore. Meaning that for whatever reasons the RegEx strings are not valid any more when using Qt 5.15. I would also guess that something in the intialization behavior changes and therfore some string constants are not initialized when used.
Which activator causes this?
And I just tested: as expected the unit tests do pass, I think it has to be a side effect of dlopen'ing the activator
@floca the comment I mention was written by you, line 112, any spontaneous idea?
In addition, suppressing deprecation warnings is in most cases no solution that lasts forever
Feb 4 2021
Deleted branch from rMITK MITK: bugfix/T28271-3DInterpolation.
The tool is a mess. Overhaul for this is not realistic. I deactivate the tool for dynamic images as a quick fix. I will reactivate it after rework.
Diving into it, it turned out to be not that hard to fix as it was just the combination of two "bugs/missing features" that appeared to be so chaotic without knowing the interference of both issues.
Oh fudge.... I totally have overlooked the tool. 😱
Pushed new branch to rMITK MITK: bugfix/T28271-3DInterpolation.
Probably it would just help to replace the MITK Error by a warning? This is also done when there is no point set at all.
I couldn't reproduce this bug in the latest Release installer on Windows. However, there is no volume visualization shown at all (no matter which timestep or which histogram I select).
If the crash does not occur anymore on Ubuntu etc. we can resolve the crash bug.
Most likely something for the next release IMO, but please decide @kislinsk :)
As it is no regression since the last release but seems to be a longer issue (or always was one), I would think about completly deactivating interpolation for dynamic images for now and mark it as known issue. I am hesitant to dive in the whole interpolation condundrum for the release. I would tend to discuss it in context our segmentation roadmap this year.
OK. also added all my fixes in the tasks changelog
Feb 3 2021
Cherrypicked the fix into the release branch.
- Rendering of the crosshair including its image planes in 3-d is done by the PlaneGeometryDataVtkMapper3D.
- Image rendering in 2-d is done by ImageVtkMapper2D.
- ImageVtkMapper2D shares its textures with PlaneGeometryDataVtkMapper3D, hence:
- The actual image rendering is done in the instances of this 2-d mapper
- The 3-d mapper just applies these textures to 3-d planes
- Result: everything that is rendered by the 2-d image mapper is automatically shown in 3-d as well, including binary images
We discussed this and decided to open a new task to cleanly define the problem. We thought that this task mixes many different things without stating the minimal steps to reproduce a single problem, so please continue here: T28270
Was recently removed in T28000. It just (tried to) enabled volumerendering on the segmentations. At least when the segmentation view was open to actually apply the preference. We can reintroduce the feature if we figured out to find a volume rendering setting that is actually able to show something meaningful (in v2018.04 is just got a black box). Not the best idea in the first place to be honest. If we want a good and accurate representation in 3-d we would need to create pixel-aligned surfaces out of the images.
I don't mind, I'm not planing to fix this before the release, so if you have time. I'm testing this with the develop branch and comparing it with the MITK 2018 installer.
Beware of a few things. T28211: [Image Rendering] Color change/ Opacity strange behaviour (only in Windows installers) also fixed rendering of image slices in 3-d, so I suggest to use the latest release branch to test anything related instead of the snapshot installers.
Oh, it's even simpler. I see that in DCMQI it is only float. max_digits10 is 9 for float and according to the documentation its purpose is exactly for serializing and deserializing floating point numbers without losing any precision, even though the intermediate text representation may not be exact.
@floca When DICOM says 16 characters/bytes, does it include a trailing \0 character or is it really a fixed array of size 16?
Here's the PR with CI testing results https://github.com/QIICR/dcmqi/pull/415. You can look at the CI results for earlier commits, if you would like to investigate.
Feb 2 2021
@kislinsk I think we have to look again in the patch. Andrey and I had a conversion because even if the standard says otherwise he had problems with resulting invalid dicom representtions (longer then 16 bytes). May be you have an idea. Have a look on the git hub thread. Andrey changed it now back to scientific with precision 8, but this would mean we are still not on the same side. On the other hand I do not want to risk producing invalid dicom (what our patch produces). So I think in the end the best way, might be to ensure it by hand. So check after stream conversion, if we have more then 16 characters. If this is the case give a warning and truncate so that it is a valid DICOM decimal string. What do you think?
Ok, I withdraw from this for now. It is useless to fix this since both views are basically the same but their implementation is different (some things are copied, others aren't). Before putting any effort into this I refer to T28142 and propose to refactor the whole thing after the release.
Fixed in release branch by patching DCMQI. I leave this task open as a reminder to switch back to an official DCMQI release as soon as they fixed it upstream.