I tried running the Workbench on Linux both with valgrind as well as AdressSanitizer. Unfortunately there is a lot of noise from memory leaks and stuff, but I did not spot anything obvious which could be the cause.
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
All Stories
Feb 10 2021
Deleted branch from rMITK MITK: bugfix/T28284-FixRenderWindowMenuVisibility.
It was an alternative code path for handling render window menu visibility on macOS that is no longer needed, resp. now even broke the visibility handling. Now all supported platforms use the same code path and to make it work in the first place without any workarounds, a few adjustmens/simplifications were necessary. The visibility status is no longer coupled to the mouse move event, which was ill-formed from the beginning and resulted in trouble on different platforms like flickering on Linux. The visibility state is now way more effectively coupled to the enter and leave events of the render windows. There's also an immediate reaction now on resize events.
Feb 9 2021
Pushed new branch to rMITK MITK: bugfix/T28284-FixRenderWindowMenuVisibility.
Closing an empty workbench or loaded workbench using X button on the title bar causes this.
What views were opened or data loaded? Or was it an empty workbench?
Attaching full stacktrace herewith:
The fix was merged upstream and this task will be resolved with the merge of the current release branch this week.
Deleted branch from rMITK MITK: bugfix/T28285-Local8BitToUtf8.
Dear future assignee of this task,
In T28285#219856, @nolden wrote:I did not dive into Windows APIs, so I just hope you know what you are doing ;)
The test looks reasonable and passes also on Linux.
One improvement could be to output the string (and maybe the hex version) in the WARNING output if the conversion fails.
And did you look into boost? There is also some conversion stuff
I did not dive into Windows APIs, so I just hope you know what you are doing ;)
In T28285#219852, @nolden wrote:I did some Googling, it seems that starting with 2019 release, Windows 10 has a UTF8 mode. It's a bit difficult to enable: "old" control panel -> region -> Administrative settings -> Change System locale -> Enable UTF8
After checking this box, MITK loads the data without code changes, but DataNode name is just a single slash "/" ... ;)
Pushed new branch to rMITK MITK: bugfix/T28285-Local8BitToUtf8.
I did some Googling, it seems that starting with 2019 release, Windows 10 has a UTF8 mode. It's a bit difficult to enable: "old" control panel -> region -> Administrative settings -> Change System locale -> Enable UTF8
Hi you both, thanks for the digging.
I also tend too follow @nolden propsal 1) declaring it a kown issue and 2) returning a meaningfull error message, of invalid characters are used.
That's a thorough dive in the topic ... Yes, toUtf8(), and nicely prints the correct Umlaut in the message box, telling you the file doesn't exist ;) I only had a quick glance around, and since a lot of stuff goes though ITK, and ITK decided to stay with local 8bit as far as I understand, I also think this would be the way to go. But I could really live with declaring this a "Known issue". Maybe something on the application user level would be nice, like
if toUtf8 != toLocal8Bit then show error
Uh, maybe I made it more dramatic than it is. We can also do the complete opposite and stay with toLocal8Bit(). That means, that we only need conversion when writing the path and name into the properties. I wrote a Local8BitToUtf8() function and it works in a first test. We can also use it to pass an UTF-8 string to the GDCM reader. Didn't check for all code locations where to do it but its late already and tomorrow is another day. :-)
Feb 8 2021
I also looked into this and one of the problems/differences to "back in the days" is that Qt nowadays assumes by default that C-strings in the constructor of QString are UTF-8 encoded. So, for example, Marco, if you change toLocal8Bit() in QmitkIOUtil to toUtf8(), you get working strings in the GUI (can be easily tested with a MessageBox::information()), but invalid strings for most of the readers, as they assume the local codepage. So we can have one or the other, but not both at the moment. The thing is that QmitkIOUtil works with QString, but mitk::IOUtil uses std::string in the structure that holds the file path (LoadInfo). This is then later used to be stuffed into StringProperty for the name property or the path property. Qt of course assumes these strings to be UTF-8 and hence we see the ?'s in the GUI.
Ok, the GDCM unicode windows fix was introduced in the 3.0 series: https://github.com/malaterre/GDCM/commit/d4dfa144b941c6cc4da87f9795f7f5aba2bb7481
Ok, I did look around a bit more. To summarize, I guess going for the "local 8bit representation" seems to be the most feasible way, since ITK also assumes this and I think we have a lot of ITK file handling code.
Ok, I debugged to SetFileName @floca mentioned, it gets called from GDCMImageIO::InternalReadImageInformation in ITK
This is a problem that becomes visible in gdcm and it is not the first time that we encouter it.
Currently the gdcm reader fails to open the file. I assume it is correlated with gdcm changes of how to handle filenames under windows in gdcm (gdcmReader.cxx 836ff)
void Reader::SetFileName(const char *uft8path)
Here some conversion between utf8 and utf16 was introduced and this string:
"C:/Users/floca/Downloads/sämple.IMA"
is changed to:
L"C:/Users/floca/Downloads/s�mple.IMA"
and for the later string the loading fails.
Mostly as a reminder I tried on top of the branch above:
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.
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.