@schererj Thank you after tomorrows retreat, I will work on these fixes (tfda)

I build the Kaapana develop branch from scratch and got the following issues:
(The last part of the log is sufficient -> issues get collected there.)

@s669m is there still something to discuss?
Otherwise please remove the request_for_discussion tag :)

Go go go!

I tested it and now this explains T27498#217602.

In T19522#68902, @goch wrote:

Specifically it is not visible on the 2D planes in the 3D render window, this error is unrelated to volume rendering.

Since the default colormaps are overwritten by the levelwindow I added a Boolean property to give the user a possibility to prevent the custom colormap from being overwritten

In T28230#219380, @nolden wrote:

In T28230#219321, @neher wrote:

Who should I assign as reviewer for this?

For arc you don't have to assign one explicitly, if you want to I would assume from the bug history @floca could be a candidate ;)

If you want speed up the review process. We currently have the policy that you add a reviewer, after you have clarified that she/he can do the review.

In T28230#219321, @neher wrote:

Who should I assign as reviewer for this?

In T28262#219362, @kalali wrote:

Also: so it is a pretty solid and natural assumption to define timesteps to have a duration until the next time step of the same image: For me this is not a natural assumption. My natural assumption would be to align the images - as you mentioned - time step after time step so 0, 2, 10, 12, 20, 22 ms.

Here I second Stefan and think it is OK to see it as a valid practical assumption. I have perfusion and other dynamic series use cases in mind. I think it would just break user expectations if you say, "sorry I cannot show you the last frame in between time points, because officially it is associated with the very time point".
And we do the same in the spatial domaine all the time. We have no black gaps in our CT volumes with 10 mm z-spacing, but the slice is only defined at one position in world space coordinate. If the thickness (==duration) is not defined, we fill the gap with known content.

In T28258#219286, @a178n wrote:

Hi Ralf, Stefan & others involved,
I am sorry for attracting unnecessary concerns on a bug which never really existed.

In T28262#219288, @kislinsk wrote:

In the task description you say that all time geometries handle time bounds as semi-closed interval [...). Except for collapsed time steps and this is the only exception, right? On the other side, our minimum time resolution seems to be 1ms? That means, that if we want to get rid of the edge case while keeping semi-closed intervals, setting the last time step duration to the minimum duration would be a very valid solution without introducing new meta-data as you said durations are purely artificial/internal to MITK - since DICOM does not seem to save durations anyway, we do not magically introduce new artificial data to derived DICOM files. The only thing left here is the confusing naming of "MaximumTimePoint" which feels more like a back() instead of and end() when using a standard library analogy.

Second option is to redefine our time bounds to be a closed interval in all cases, right? That would also makes more sense with the term "MaximumTimePoint". Then we have to identify many code locations were we have to potentially change < to <= and make sure that we use max() instead of +inf() for indefinitely long timesteps (same for lowest() vs. -inf()).

In T28262#219358, @kislinsk wrote:

Since we are supporting multiple images in a single scene. These images can have different time bounds and their time steps can be discretized differently. So even if you have kind of matching dynamic images but one image specifies its acquisition times at 2, 12, 22ms seconds, and the other one at 0, 10, and 20ms, you would have a very sparse discrete time line and only see either no image or at maximum a single image, but never both at the same time. So it is a pretty solid and natural assumption to define timesteps to have a duration until the next time step of the same image, if not explicitly specified otherwise. However, we also have many images that do not have time bounds stored in their files at all (in particular static images) and we need to come up with something completely artificial. It is a pretty severe issue but luckily is not very present in 99% of workflows as users rarely need to have scenes with different dynamic images aligned perfectly or at least not matching 1:1 on a time step basis instead of true time for their purposes.

It would be overkill but to get things sorted out for everyone MITK is missing a complete sophisticated time-line component were you can arrange images and change their time properties just like every video editing software allows you to do for clips.

In T28262#219352, @kalali wrote:

I'd like to know why we need a duration in the first place?

In T28262#219352, @kalali wrote:

In T28262#219287, @floca wrote:

In T28262#219283, @kislinsk wrote:

It is philosophical indeed, but how is any of these strategies worse than creating a time step with a duration of 0?

We should keep in mind that the whole concept of duration is purely artefical. There is no duration defined. The only things we know are the very time points where they exist.

I'd like to know why we need a duration in the first place?

So far we do not need it. And it is not stored. There are use cases in DICOM where the duration of a frame could be less or more then the distance in time between two frames (simelar to fact that slices may have a thickness different to the z spacing), but we currently do not care.
So duration comes implicetly in our workbench as "the time till the next frame is valid".

Ok, then I would propose to reuse this task just for the problems that are documented by Stefan above (https://phabricator.mitk.org/T27883#219162 ) and who we deal with them for the release (because T28262 might have a more longterm perspective).

I tested again with UltrasoundImages\4D_TEE_Data_MV.dcm and everything seems to work as expected. So this task is only valid for strange arbitrary time geometry, as the title describes it.
So I guess the refactoring done in the segmentation tools changed most of the bugs and we got now down to the problem mentioned in T28262.

In T28262#219287, @floca wrote:

In T28262#219283, @kislinsk wrote:

It is philosophical indeed, but how is any of these strategies worse than creating a time step with a duration of 0?

We should keep in mind that the whole concept of duration is purely artefical. There is no duration defined. The only things we know are the very time points where they exist.

In T28258#219285, @a178n wrote:

I have finally found the fix! This bug is due to my GPU driver.
My laptop has an AMD Radeon dedicated GPU and MITK uses it by default for rendering its "Standard Display".
I checked the crash log using Event Viewer and found that atio6axx.dll was the culprit here. This is an AMD driver related DLL file.
I switched the default GPU for Mitk workbench to the onboard integrated graphics and that's it!
Problem solved. I am attaching the working screen recording herewith :-)

If you have suggestions for a better structure, please feel free to add it to T28130. The checklists were not even existing before and my goal was to overhaul all the (multi-)label segmentation checklists to include 4D-data tests. I also strove for consistency throughout all checklists.
I see that the bold written statement reads as a new test - that could be changed. However, the test right before already dealt with 4D image data (4D Bild als Testdatensatz) so I assumed it was clear that the 3D-data tests were already finished.
I will also think about some rephrasing / different text format.

Who should I assign as reviewer for this?

I don't know what the real world scenario is to change the Image Rendering Mode, but basically you just have to perform step 8 and 9 to see the bug (step 10).
I could imagine the Image Rendering Mode being changed to either LookupTable_Color or ColorTransferFunction_Color by a line of code inside a module or plugin, which would then make this bug appear, if the auto topmost mode was disabled.

Pushed new branch to rMITK MITK: feature/T28230-ResampleImageToReference.

To be honest, I just did not understand the checklist correctly. If you know the outcome and read the checklists, they are fine. But for me, it was like a new test starting with loading and testing 3D Data again. So I misunderstood the line Bei den folgenden Aktionen stets mehrere Label, auch auf unterschiedlichen Layern testen. as beeing a new test, because it had of kind of title character. And that is why I thought, I would start with a 3d Image again...

Okay, back to the beginning. I still see two options that do not feel like a workaround.

In T28262#219283, @kislinsk wrote:

It is philosophical indeed, but how is any of these strategies worse than creating a time step with a duration of 0?

We should keep in mind that the whole concept of duration is purely artefical. There is no duration defined. The only things we know are the very time points where they exist.
And we should differentiate between things we might do at the data level and at the UI/Interaction level. We do the same in the spatial domain. Here we just do not allow images that do not have a equidistant spacing, so we avoid that mess. There it was an option, because those cases are so rare. For time they are not rare, so we have to deal with it.
And I realy do not want to produce data with a magically added duration, if we store it. Because we do not know and in the end only assume it for nicer UI experience and because we are afraid of potential breaking.

Hi Ralf, Stefan & others involved,
I am sorry for attracting unnecessary concerns on a bug which never really existed.

I have finally found the fix! This bug is due to my GPU driver.
My laptop has an AMD Radeon dedicated GPU and MITK uses it by default for rendering its "Standard Display".
I checked the crash log using Event Viewer and found that atio6axx.dll was the culprit here. This is an AMD driver related DLL file.
I switched the default GPU for Mitk workbench to the onboard integrated graphics and that's it!
Problem solved. I am attaching the working screen recording herewith :-)

In T28258#219277, @kislinsk wrote:

I even extracted the same scene to reproduce your issues as close as possible but it just works for me. Can you please close the workbench, delete the DKFZ folder in %APPDATA%/../Local, and then try again? What locale do you use on your system? Maybe that's the difference.

In T28262#219282, @floca wrote:

In T28262#219280, @kislinsk wrote:

If we do not have any durations and just came up with the durations based on the deltas between time steps, we may want to also consider the following strategies:

• If all time steps have the same distance from each other, set the duration of the last time step accordingly
• If time steps have arbitrary durations, follow the MITK default for other cases and make it at least 1ms.

I think it is a question of philosophy. The reason why I always was and am hesitant to make up some duration, is that it is made up. We don't know it and we changing the information provided to us. We had this discussion several times in the past. And as for the dynamic modeling (e.g. perfusion) we only need to know when the frames start, it seem not right to make up things.
If the majority want's to go that way, OK.
But if I have to make something up, I would definetly picking a value that is also in hindside easier to identify as artfically added. E.g. std::numeric_traits::max or +inf.

In T28262#219279, @kislinsk wrote:

Why is the last timestep collapsed in the first place?

Because we have no information about the extend of the last time point... because, well, it is the last.

In T28258#219276, @a178n wrote:

Still, none of you could reproduce this issue? I am surprised. Maybe it's an isolated problem with my local machine?

No. Still cannot reprocue it. Very strange.

If we do not have any durations and just came up with the durations based on the deltas between time steps, we may want to also consider the following strategies:

Why is the last timestep collapsed in the first place? Without having any context knowledge it seems to be the actual issue for me and makes not much sense as from looking at the values, it simply has a duration of 0. Is it really a flaw of DICOM or do we just come up with these values based on some other DICOM tags?

If nothing else helps, I guess the only way we can actually get closer to what happens is that you build the WorkbenchRelease configuration of the latest snapshot to be able to start the workbench in debug mode. Do you need assistance for the build?

I even extracted the same scene to reproduce your issues as close as possible but it just works for me. Can you please close the workbench, delete the DKFZ folder in %APPDATA%/../Local, and then try again? What locale do you use on your system? Maybe that's the difference.

Hi Ralf,
Sure. I can reproduce the crash with both conditions viz. data from MITK-Data folder and with segmentation view closed. Here is the screen recording.

@a178n 2 more questions:

1. Can you also reproduce this problem with using using the data directly from the MITK-Data folder and not out of your extracted scene file?
2. Can you reproduce the problem with the segmentation view closed. (please close the view, close the workbench and then try it with the seg view never activated) If it is not reproducable, can you reproduce it with an other view opened that has auto selection (e.g. the measurement view)?

Nothing special that I can see. The moment it crashes, the terminal window also closes quickly. I am attaching a screen recording with the terminal screen. From this run (as in the attached video), I feel, maybe the issue/bug has nothing to do with Segmentation. I only noticed it in the context of testing 'Segmentation'.

In T24766#131422, @floca wrote:

In T24766#131411, @floca wrote:

So basically I think currently we should check that all busiiness code checks time bounds of geometry time steps like a closed open set: [min bound, max bound). Meaning that min bound is within the coverage of a time step and max bound itself is not within the range of the time step

This is at least how it is internally implemented in the ArbitraryTimeGeometry if you request the time step for a certain time point.

Is there any output in the terminal window right before the crashes?

FYI- Since I am in windows now, I can't really see if "Core dumped" is the real error message. I merely use it synonymously with the word "Crashed".

In T28258#219234, @floca wrote:

Thanks for reporting. Several questions:

1. did you stumble upon it by accident/random testing or was it while you went through the check lists (so is this covered by the checklist, so that we can find such errors also in other releases?)
2. Were all crashes with the Win installer?
3. Which segmentation view did you use? classic or multi label?
4. Wich views are open in the workbench when you experienced the crashes?

I was not able to reproduce the problems on my machine. 🤔 Can any one else verify that problem on their system?

BTW, when using "Create subtask", please make sure to also check the other form fields during task creation, as they are by default set to the same values as the parent task. In particular, remove the assignee and probably some of the subscribers. :)

Also, what images did you use?

Thanks for reporting. Several questions:

1. did you stumble upon it by accident/random testing or was it while you went through the check lists (so is this covered by the checklist, so that we can find such errors also in other releases?)
2. Were all crashes with the Win installer?
3. Which segmentation view did you use? classic or multi label?
4. Wich views are open in the workbench when you experienced the crashes?

In T28000#219224, @floca wrote:

Just to be sure. How is the process of updating this task description. The person who closes a task/fixes a bug also updates the list in the Task description. Correct?

Just to be sure. How is the process of updating this task description. The person who closes a task/fixes a bug also updates the list in the Task description. Correct?