Allow loading user defined treatment machines in Room's Eye View

[cpinter]

Many times researchers would like to calculate collisions for their machines but they cannot share the treatment machine models. At the same time they would be willing to define the parts if there was a relatively convenient way to do so.

This could be achieved by defining a JSON schema that describes the parts. The currently available treatment machines should be converted to this new method as well. A separate repository could be set up for the publicly available machine definitions.

A project with @ferdymercury

[MichaelColonel]

You can count on me, if you need help with models transform programming and other stuff!

[cpinter]

Thank you :)

[cpinter]

Working on a sample JSON file to describe the existing Varian machine. This is what I have now. Please comment on anything that could need to be added or changed:

{
  "TreatmentMachineName": "Varian TrueBeam STx",
  "$schema": "https://raw.githubusercontent.com/???/???-schema.json#",
  "Part": [
    {
      "Type": "Collimator",
      "File": "Collimator.stl",
      "FileToPartTransformMatrix": [ [1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1] ]
    },
    {
      "Type": "Gantry",
      "File": "Gantry.stl",
      "FileToPartTransformMatrix": [ [1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1] ]
    },
    {
      "Type": "ImagingPanelLeft",
      "File": "ImagingPanelLeft.stl",
      "FileToPartTransformMatrix": [ [1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1] ]
    },
    {
      "Type": "ImagingPanelRight",
      "File": "ImagingPanelRight.stl",
      "FileToPartTransformMatrix": [ [1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1] ]
    },
    {
      "Type": "LinacBody",
      "File": "LinacBody.stl",
      "FileToPartTransformMatrix": [ [1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1] ]
    },
    {
      "Type": "PatientSupport",
      "File": "PatientSupport.stl",
      "FileToPartTransformMatrix": [ [1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1] ]
    },
    {
      "Type": "TableTop",
      "File": "TableTop.stl",
      "FileToPartTransformMatrix": [ [1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1] ]
    },
  ]
}

One thing that made me thinking is that the movement of the imaging panels is more complex than a simple translation or rotation (first rotates in then starts to translate too). After checking, actually, it seems that the movement of the panels is broken. It seems out of scope for this project to fix that I guess (I may still do it), but certainly worth mentioning that there are some magic numbers in the code that moves the imaging panels, and also worth considering if we want to load that parameter from JSON as well:

https://github.com/SlicerRt/SlicerRT/blob/19572b5d6f598a0d94c75141afe0ea203d4be9e4/RoomsEyeView/Logic/vtkSlicerRoomsEyeViewModuleLogic.cxx#L1030-L1037

[ferdymercury]

Looks very good, thanks for your work!

Some comments below:

• [x] An optional "Color" tag (in HEX or RGB) for each subpart could be helpful, in case there is the option to override the one from the STL upon loading.
• [x] A "visible" or "active" tag could be also helpful, in the sense that maybe when you commission the JSON file, you have all the parts included, but then for speedup reasons you might want always to hide the patient support that never leads to collisions, so instead of removing that part from the INI file, you just point to deactivate it without having to delete potentially future useful info.
• [x] A human-readable "Title" tag could be added, which would be useful to later say, there was a collision with the "Gantry head" and the "Patient support", rather than between "PatientSupport" and "Gantry" as specified in the Type tag, that I guess will map to an enum rather than a string.
• [x] Maybe it is useful to add a tag "Instructions" onto this file, that details (among other things) that files should be given in the IEC? frame of reference, or if not, specified otherwise in the FileToPartTransformMatrix. This way, people could just download an "example" json file and modify to their needs in a text editor, following those in-place instructions. Or would it be better to separate the instructions to a different place (a README.md in the repo or sth like that?).
• [ ] Optional "Author", "Date" and "Source" tags could be useful to locate the source of this machine or who modeled it. For example "Author": "Eduardo Meilán Bermejo", "Date":"2017" , "Source" : "github.com/SlicerRt/3Drepo"}
• [x] An additional tag "Folder" could be added at the top, that can be either empty (or "./"), or instead pointing to the full path where the files are stored. So that, if one later moves the folder somewhere else, one can just change that line, and not all the other ones of each part, that are relative to that parent folder.
• [ ] Optionally, the Folder could be also a URL if the data are stored somewhere online rather than locally ? If this makes things too complicated, let's leave it.
• [ ] As far as I understand, the "Type" tag is associated with a given internal logic in slicer that controls the degrees of freedom. For example, if it's a couch, then we are allowing XYZ and azimuthal freedom, if it's a gantry, we only allow polar rotation but no translation. I am wondering if it would be to internally doable to represent this information in terms of this list of degrees of freedom rather than / or in addition to / Types. This is how I had implemented it for RayStation, see https://github.com/mghro/rad-collision/blob/08a29e0250e2243989d4eda2699b7c1c6aabc76e/RayStation/collision_detection.py#L148 The rationale for this is in the next point:
• [ ] Some gantries only allow gantry angles from 0 to 190 or so. It would be nice if the JSON file had a tag to describe the maximum motion range, and this info is forwarded later to the GUI sliders. The same happens with the couch, you cannot infinitely move in Z. Thus, I was thinking it would be helpful to have tags like: "DegreesOfFreedom": { "X": [-100,100], "Y":[-100,100], "Phi":[0,190deg]} for each the parts defined. I guess the "Type" tag should be used to cross-check for consistency, and warn the user if he is using a DOF that does not exist for that Type-Part in the internal SlicerRt logic. Or maybe we could even have a "Generic" Type that does not preassume any constraint, and is only defined by the "DOF" in the file.
• [ ] Yes, the panel rotation, although it's not something I'm directly interested in, could be also parameterized via the JSON file to remove any hard-coded magic numbers :) .
• [x] Do we want to optionally allow defining multiple machines inside the same JSON file? The different treatment machine names would then be loaded to the dropdown dialog and one can later switch easily among them.
• [x] Would it be helpful to add a security feature? Saying at the top "MaximumSizePerPart": 100Mb, "TotalMaximumSize": 1Gb, so that SlicerRt does not load those STL files if these constraints are not constrained in the currently free memory? But maybe it's a bit of overkill.

[cpinter]

Thanks for the comments! Good that you have more ideas now seeing this file - I only added those that we defined at our meeting. A few things:

• There is no good way to have enum in JSON (at least I did an extensive search yesterday and this was the conclusion), so it needs to be something we check in C++ whether the string used for Type is among the accepted ones or not.
• I'd probably add the instructions in the README.md of the repository where we have the publically available machines
• Indicating author is a good idea, although I wouldn't add URLs pointing to its own repository in the JSON
• Folder: I'd rather just have it in the File. Maybe rename it to FilePath?
• Degrees of freedom: Yes this could be very useful. I'd suggest we add this after the first simple approach works
• Multiple machines in one JSON: I don't see the rationale. It puts a development burden on us, when for the user it would be a minor technicality and maybe 10s of extra work to save it in two files instead of one. Also cleaner to have one JSON for each machine in every sense I can think of
• The file size limit would make more sense in the C++ logic I think, because I don't see a reason for having it different for each machine. But this also seems like a feature we can add later

Here's the updated sample JSON file. I changed some element names to be more unambiguous (title vs name, active vs visible vs enabled):

{
  "TreatmentMachineName": "Varian TrueBeam STx",
  "$schema": "https://raw.githubusercontent.com/???/???-schema.json#",
  "Part": [
    {
      "Type": "Collimator",
      "Name": "Collimator",
      "FilePath": "Collimator.stl",
      "FileToPartTransformMatrix": [ [1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1] ],
      "Color": [255, 255, 255],
      "Enabled": true
    },
    {
      "Type": "Gantry",
      "Name": "Gantry",
      "FilePath": "Gantry.stl",
      "FileToPartTransformMatrix": [ [1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1] ],
      "Color": [255, 255, 255],
      "Enabled": true
    },
    {
      "Type": "ImagingPanelLeft",
      "Name": "Left imaging panel",
      "FilePath": "ImagingPanelLeft.stl",
      "FileToPartTransformMatrix": [ [1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1] ],
      "Color": [255, 255, 255],
      "Enabled": true
    },
    {
      "Type": "ImagingPanelRight",
      "Name": "Right imaging panel",
      "FilePath": "ImagingPanelRight.stl",
      "FileToPartTransformMatrix": [ [1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1] ],
      "Color": [255, 255, 255],
      "Enabled": true
    },
    {
      "Type": "LinacBody",
      "Name": "Linac body",
      "FilePath": "LinacBody.stl",
      "FileToPartTransformMatrix": [ [1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1] ],
      "Color": [255, 255, 255],
      "Enabled": true
    },
    {
      "Type": "PatientSupport",
      "Name": "Patient support",
      "FilePath": "PatientSupport.stl",
      "FileToPartTransformMatrix": [ [1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1] ],
      "Color": [255, 255, 255],
      "Enabled": true
    },
    {
      "Type": "TableTop",
      "Name": "Table top",
      "FilePath": "TableTop.stl",
      "FileToPartTransformMatrix": [ [1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1] ],
      "Color": [255, 255, 255],
      "Enabled": true
    }
  ]
}

[ferdymercury]

• There is no good way to have enum in JSON (at least I did an extensive search yesterday and this was the conclusion), so it needs to be something we check in C++ whether the string used for Type is among the accepted ones or not.

Sounds good.

• I'd probably add the instructions in the README.md of the repository where we have the publically available machines

Perfect.

• Indicating author is a good idea, although I wouldn't add URLs pointing to its own repository in the JSON

Agreed.

• Folder: I'd rather just have it in the File. Maybe rename it to FilePath?

Good idea, too.

• Degrees of freedom: Yes this could be very useful. I'd suggest we add this after the first simple approach works

Great.

• Multiple machines in one JSON: I don't see the rationale. It puts a development burden on us, when for the user it would be a minor technicality and maybe 10s of extra work to save it in two files instead of one. Also cleaner to have one JSON for each machine in every sense I can think of

You're right.

• The file size limit would make more sense in the C++ logic I think, because I don't see a reason for having it different for each machine. But this also seems like a feature we can add later

Perfect.

Thanks so much :)

[cpinter]

The basic loading of treatment machine from JSON descriptor file works now for the default Varian TrueBeam machine. Next steps are implementing color, enabled state, and the transform.

One question I have is what to do with the user interface. So far there has been a combobox with the two default machines. For now, as the easiest solution, I added a third option "From file...", clicking on which a file selector pops up and we can load the machine using that:

@ferdymercury @gregsharp what do you think the ideal GUI would look like for machine selection?

[cpinter]

@ferdymercury Also an idea as I was thinking about the Enabled state. I can imagine you want to show the model but not include it in the collision detection (also for speedup reasons). Should we include this somehow?

[ferdymercury]

@ferdymercury Also an idea as I was thinking about the Enabled state. I can imagine you want to show the model but not include it in the collision detection (also for speedup reasons).

Sounds good. Maybe it would be worth defining State as a string with three possible options:

• [x] Disabled (never loaded)
• [x] Passive (loaded but not checked for collision)
• [x] Active (loaded and checked for collision)

[ferdymercury]

@ferdymercury @gregsharp what do you think the ideal GUI would look like for machine selection?

Thanks, it's looking great. Maybe it would be interesting to let the user define a MachineDatabasePath in the UserPreferences of Slicer, or via an env variable, that would prepopulate this combobox with all machines /json files found in this folder.

[cpinter]

Thanks a lot for the answers, I agree with both.

[cpinter]

I implemented all the features discussed related to loading a treatment machine model already included in the JSON files (part name, file path, file to RAS matrix, color, state), and tested with the two default machines. I integrated (https://github.com/SlicerRt/SlicerRT/pull/226) the commits into the upstream to make it easier to try what is ready so far.

This means that tomorrow afternoon (when the build machines in the US are done building and testing all extensions too) you can download the latest Slicer preview, install SlicerRT, and try the latest changes.

A few outstanding things:

• There was a custom scaling applied to the table top displacement for the treatment macines, see https://github.com/SlicerRt/SlicerRT/blob/4c0ceb8900f6c12ab9b66e2ae6e7d1077c99c409/RoomsEyeView/Logic/vtkSlicerRoomsEyeViewModuleLogic.cxx#L1212-L1221 What should we do with these? Add a special scaling member to the table top part? Or is this displacement scaling factor something that could apply to any part?
• The collision detection already did not work for the additional machine parts (e.g. ApplicatorHolder, ElectronApplicator), but now I disabled even loading them, because those should work via their own JSON file as well. So I hid the entire section to avoid confusion. Do we need these custom parts fully working again?
• Just as a note, I realized that the reason the imaging panel movement for the Varian machine didn't work due to the change in Slicer to by default load STLs in the LPS coordinate system. Because we can do it now, I tried simply applying the LPS->RAS transform for each part, and the panel movement is fixed now. The machine is by default shown "from the back", but it's a minor inconvenience, and I suppose it was like this before the STL loading defaults changed.

I think what we already discussed and is missing are:

• Degree of freedom specification for parts in the JSON
• Instructions about JSON definition (although I think the two examples in the SlicerRT repo should serve well for this purpose, and I added quite talkative error messages if something is not right)
• Uploading/trying your own custom machines

Anything else?

[ferdymercury]

I implemented all the features discussed related to loading a treatment machine model already included in the JSON files (part name, file path, file to RAS matrix, color, state), and tested with the two default machines. I integrated (#226) the commits into the upstream to make it easier to try what is ready so far.

This means that tomorrow afternoon (when the build machines in the US are done building and testing all extensions too) you can download the latest Slicer preview, install SlicerRT, and try the latest changes.

Awesome, that was quick. I'll try in a few days then.

A few outstanding things:

* There was a custom scaling applied to the table top displacement for the treatment machines, see https://github.com/SlicerRt/SlicerRT/blob/4c0ceb8900f6c12ab9b66e2ae6e7d1077c99c409/RoomsEyeView/Logic/vtkSlicerRoomsEyeViewModuleLogic.cxx#L1212-L1221
   What should we do with these? Add a special scaling member to the table top part? Or is this displacement scaling factor something that could apply to any part?

Oh, I see, it's for flexible parts that change size. I guess it should be used only for the part of the couch trunk that extends, not for the whole table top. That being said, maybe it's easier from the code-side to implement it for every part in the JSON and not making distinctions in the code depending on type. EDIT: Or maybe better, automatically calculate how much it has to extend. This should be doable by getting it's height. It should be glued from the bottom and only extend or compress vertically the upper part by a dynamic factor, such that the height changes by the same number of centimeters than the table-top is moved vertically.

* The collision detection already did not work for the additional machine parts (e.g. ApplicatorHolder, ElectronApplicator), but now I disabled even loading them, because those should work via their own JSON file as well. So I hid the entire section to avoid confusion. Do we need these custom parts fully working again?

I'd say the applicators should be part of the main JSON file of the machine, not a separate one (unless there is a universal applicator that works on many machines). The "Type" tag can indicate that is an Accessory or sth like that.

Anything else?

• Posibility of indicating a Machine DatabasePath containing several JSON files, which prepopulates the combo box
• We could maybe add an additional tag in the JSON defining whether the FilePath is absolute, or relative to the location of the JSON file. PathBehaviorAbsolute = true/false Or leave it always Relative, but specify this info clearly in the instructions.
• It could make sense to move https://github.com/SlicerRt/SlicerRT/tree/master/RoomsEyeView/TreatmentMachineModels to a separate git repository so that SlicerRT does not become too big because of adding STL files (I'd like to add some more later on). The downloaded STL files would be located in $USER/.config/... (or via cmake-fetch?) rather than in Slicer's main program directory and then in Slicer preferences you just put the path to this database. Or maybe something in the same way that you can download "CT/MRI Sample Data" from Slicer's main screen? If you are in the collision detection module, you can click to download the public Slicer STL database samples, and/or specify a path to your custom folder with JSON files. This point is useful so that the STL and JSON files are exactly the same ones / can be re-used also by other rad-collision flavors such as the custom scripts on RayStation https://github.com/mghro/rad-collision/issues/21.
• In this separate repo, there should be some instructions, recommending a coordinate system when creating the STL files. Also recommending (important!) to create files with reasonable size, otherwise the collision detection takes forever. For example, it could be useful to deploy "stepreduce" before converting STEP to STL, see https://gitlab.com/kicad/code/kicad/-/issues/2479#note_1252745439.
• About/Credits
• [ ] Special parts if some free hours left (robotic arm? / couch control / ...) https://github.com/SlicerRt/TreatmentMachines/pull/4
• [ ] snout extraction option https://github.com/SlicerRt/TreatmentMachines/pull/2
• [ ] coaxial detector

[ferdymercury]

This means that tomorrow afternoon (when the build machines in the US are done building and testing all extensions too) you can download the latest Slicer preview, install SlicerRT, and try the latest changes.

Works great with default machines! thanks.

One idea: Maybe (if not too complicated) a button "Remove treatment machine" could be helpful? Or if one loads a second treatment machine, that a prompt appears asking if the previous one should be deleted.

I'll try later with external STL files.

[cpinter]

Thanks for the feedback! I usually just clear the scene. Also it can be deleted from the Data module with two clicks (it is organized into a folder). In normal Slicer modules we usually don't complicate the already complex user interfaces with convenience functions that can be reached so easily. But it you think it's important we can add a button...

[ferdymercury]

Sounds reasonable, no need for an extra button, I agree. But maybe the previous machine can be automatically hidden from the scene if a new one is loaded?

[cpinter]

But maybe the previous machine can be automatically hidden from the scene if a new one is loaded?

Yes this makes complete sense.

[ferdymercury]

Some feedback from my side:

• [ ] Are the default machines in the right coordinate system? Shouldn't the couch height be AP instead of IS ?

• I managed to load a custom treatment machine, it works great. I get these warnings in the console log, but probably they are not important:

  Warning: In /work/Preview/S-0-E-b/SlicerRT/RoomsEyeView/Logic/vtkSlicerRoomsEyeViewModuleLogic.cxx, line 268
  vtkSlicerRoomsEyeViewModuleLogic (0x241e1e0): LoadTreatmentMachine: State for part VarianTrueBeam_Collimator is set to Disabled but the part is mandatory. Loading anyway.
  LoadTreatmentMachine: Failed to load VarianTrueBeam_Collimator model from file /home/username/Desktop/stl/Collimator.stl
  ReadDataInternal ((unknown)): File /home/username/Desktop/stl/Varian TrueBeam/RotatingHeads.stl does not contain coordinate system information. Assuming LPS.
  ReadDataInternal ((unknown)): File /home/username/Desktop/stl/Varian couch/CouchSupport.stl does not contain coordinate system information. Assuming LPS.
  ReadDataInternal ((unknown)): File /home/username/Desktop/stl/Varian couch/TableTop.stl does not contain coordinate system information. Assuming LPS.
  SetupTreatmentMachineModels: Unable to access Collimator model

• [ ] The UI feedback could be improved a bit. In one case, I moved the slider very quickly, and Slicer probably understood to calculate all the rotations and at the same time check for collisions for many intermediate angles while swiping, I guess. I waited about 3 minutes for Slicer to unfreeze). I guess that Slicer should 'abort' redrawing a geometry and checking for a collision if it takes more time to show the rotated figure than sliding to the next intermediate step in the slider while the mouse is still pressed.

• [x] Another thing that could be helpful is to tell the user what slicer is doing. For example, if am typing numbers in the gantry angle, and I go from 0 to 10, 10 to 20, all goes well. Now the collision will take place at 60. If I type the 6 before the preexisting 0, then Slicer freezes, spends 1 minute calculating the collision, and then the 6 appears in the text field of the GUI :) It would be better if first it gives a message "Calculating collisions...", and the 6 is there.

• [ ] If for example, there is no imaging panels or collimator loaded, the respective sliders should be disabled in the UI.

• [ ] For loading a part of a custom couch that moves L-R, S-A but not I-S, we have to wait until the degrees of freedom functionality is implemented, right? (It's the frame of the couch, which allows longitudinal shift of the table top by sliding with respect to it).

• [ ] Concerning scaling factor, as specified in the EDIT comment above, a dynamic scaling of the couch support to match the vertical displacement would be ideal, and remove the need to specify and parse anything a factor value in the JSON. Maybe just a tag to enable VerticalAutoScaling.

[cpinter]

Are the default machines in the right coordinate system? Shouldn't the couch height be AP instead of IS ?

I believe you're right. I just checked, and the coordinate systems were like this from the start. If we change the coordinate systems to match the patient, then I think we should do it in a way, so that we don't do a half job, that the patient is fixed within the coordinate system, i.e. when moving/rotating the table top, the table top is kept fixed, and the rest of the linac moves instead.

I get these warnings in the console log, but probably they are not important

This is a basic Slicer warning saying that the STL did not specify coordinate system and alerts the user that LPS is assumed. I think this warning was useful because there was a switch from an RAS default to an LPS default not so long ago. You can safely ignore it. If you think this is distracting we can propose removing this warning.

The UI feedback could be improved a bit

Yes, totally agreed. I have seen this too, especially with the linac that had more triangles (the Varian one is a low-detail one but the Siemens linac has many times more triangles and collision detection takes much longer for that). Dynamically assessing whether collision calculation is needed or not may not be so simple. Let me do some investigation.

tell the user what slicer is doing

OK this could be done fairly easily I think.

If for example, there is no imaging panels or collimator loaded, the respective sliders should be disabled in the UI

Good point. Would you prefer hiding or disabling? I think it makes sense also to hide them if not available for the currently loaded linac.

For loading a part of a custom couch that moves L-R, S-A but not I-S, we have to wait until the degrees of freedom functionality is implemented, right?

Yes I think this is what would be needed if we don't want to handle special cases and do some quick&dirty before that.

Concerning scaling factor...

Let's discuss.

As now the basics are done, and what we have at the moment works exactly how it worked before, I propose that we integrate the topic branch into master, and create tickets for the individual issues. That way nothing will fall through the cracks. We could create a special milestone to be able to handle the related issues more easily. If you agree I do the integration and can create the tickets.

[ferdymercury]

that the patient is fixed within the coordinate system, i.e. when moving/rotating the table top, the table top is kept fixed, and the rest of the linac moves instead.

Yep. How i did this in Raystation was to keep indeed the patient fixed. The room was placed around the isocenter defined for the current beam. When changing beam isocenter or angle, the room was moved accordingly. I still gave freedom to move table top a bit as you can put the patient more towards an edge of the table...

If you think this is distracting we can propose removing this warning.

No need. Is fine like it is now.

Good point. Would you prefer hiding or disabling? I think it makes sense also to hide them if not available for the currently loaded linac.

No preference from my side.

As now the basics are done, and what we have at the moment works exactly how it worked before, I propose that we integrate the topic branch into master, and create tickets for the individual issues. That way nothing will fall through the cracks. We could create a special milestone to be able to handle the related issues more easily. If you agree I do the integration and can create the tickets.

Great, agreed. Thanks for your work.

[cpinter]

I have had some time to resume working on this today, sorry for the delay.

Here's the list of tasks based on the conversation so that we have them in one spot (feel free to edit):

• [x] Change transform chain so that the table top coincides with RAS (and the whole machine moves around the patient)
  • [x] Place room around the isocenter defined for the current beam. Move room when isocenter changes
  • [ ] Give freedom to move table top a bit as you can put the patient more towards an edge of the table (TBD)
• [x] Scale flexible parts properly (https://github.com/SlicerRt/SlicerRT/issues/218#issuecomment-1428213359)
• [ ] JSON key indicating absolute/relative path
• [ ] Move treatment machine models from SlicerRT to the external repo
• [ ] Add instructions to treatment machine parts repo (coordinate systems, polydata polygon count)
• [x] Add about/credits
  • [x] SlicerRT source code
  • [x] Treatment machine parts repository
• [x] Hide current machine (or remove, maybe ask user) if one loads a second machine
• [ ] Handle lengthy collision detection (https://github.com/SlicerRt/SlicerRT/issues/218#issuecomment-1436850101)
  • [x] Display "Calculating collisions..." message on the GUI
  • [ ] Detect lengthy detection and abort / Do not allow loading models with too many polygons
  • [ ] Update GUI before starting collision detection (so that typed numbers show up)
• [x] Hide sliders that are for parts that are not loaded (e.g. imaging panels)
• [ ] Correct patient orientation: Use the ImageOrientationPatient tag from the patient's DICOM CT to set up the treatment machine orientation accordingly (once beam is selected we can get the plan, and from that the CT)

I started working on changing the transform logic so that the table top is fixed whichever transform the user is changing (except that the lateral movement of the table top will be allowed as discussed above). It is not so easy, but luckily there is a new implementation in a project I'm involved in, so trying to get inspiration from there to make the transition as lightweight and elegant as possible.

[ferdymercury]

Looks great, thanks so much.

Maybe add (cannot edit your post):

• [ ] Posibility of indicating a Machine DatabasePath containing several JSON files, which prepopulates the combo box

[cpinter]

I made the change that applies a transform to the whole machine so that the table top is kept fixed.

https://github.com/SlicerRt/SlicerRT/assets/1325980/04260deb-c216-4987-99df-76990d35bf29

In this implementation I use the so far unused FixedReferenceToRAS transform that is at the highest level of the transform tree to achieve this.

We still have the problem that the directions are wrong (vertical direction in treatment machine is not PA but IS). I was thinking maybe we could use the same FixedReferenceToRAS transform defined in the JSON files to do this rotation. Then this new code should take that transform and concatenate it with the one that keeps the table top fixed. I think this would fit into the IEC standard, as there is no RAS defined there, and the FixedReference has the Z as we have it currently (see here), so both transforms should go into FixedReferenceToRAS. What do you think?

[ferdymercury]

Hi Csaba. The video looks wonderful.

Concerning the positions PA IS:

What I was doing in RayStation was to define a first rotation that was patient-dependent. If patient CT was FFS or HFS or FFP or HFP i had a separate initial rotation matrix applied there. I guess the same info can be better taken from ImageOrientation DICOM tag matrix.

Wouldn't this be enough, so that we do not need to rely on the information of the JSON file? Or am I misunderstanding sth?

There is a corner case when image was done with HF but treatment delivered with FF, but we cannot cover everything heheh.

[cpinter]

I think you're right, the main thing is to have the orientations right when the beam is specified (and the beam belongs to a plan that belongs to a patient CT), so we can do it like that. Thanks!

[cpinter]

I added your grant acknowledgment to the Room's Eye View source files (see https://github.com/SlicerRt/SlicerRT/commit/c7f64521895a077430f54a853410950aec15db30) and to the module acknowledgment section (see https://github.com/SlicerRt/SlicerRT/commit/52a39001e4439dc841ea34e6f5bdfa7da794625a). Let me know if you'd like to add it anywhere else. We'll add it to the treatment machines repository too, but we don't have that yet.

[ferdymercury]

Perfect! thanks.

[cpinter]

A little progress:

• I checked above the about/credits task to done for the treatment machine parts repo beacuse you have done it. Let me know if there is something else to do in this topic
• JSON key indicating absolute/relative path: I was thinking about this and to me it seems that making the JSON more complex for something that we can detect automatically is not necessary. It is easy to determine if a path is relative or absolute, see https://doc.qt.io/qt-6/qdir.html#isAbsolute . Let me know if this looks sufficient to check. If so, I'll just make sure that the parts are loaded correctly in both cases.

I'll continue with the tasks one by one in the meantime. Thanks!

[ferdymercury]

Sounds good, agreed, thanks!

[cpinter]

Now if a treatment machine is already loaded when the user wants to load a new one, this dialog pops up:

Or if the user wants to load a machine that is already loaded, this is shown and loading is cancelled:

[ferdymercury]

Lovely :)

[cpinter]

I started working on this task "Place room around the isocenter defined for the current beam. Move room when isocenter changes" because it is related to an ugly bug related to the FixedReference system jumping from patient-aligned and RAS-aligned depending on what the user changes. Since we made the changes that allow the table top be the fixed part, we have two parallel worlds in terms of the FixedReference coordinate system:

1. Beam-driven FixedReferenceToRAS: It considers the isocenter and does the transforms to have the A and S directions correct, see https://github.com/SlicerRt/SlicerRT/blob/3ae787b5702caf764336f2d4c52b160a046cf5ff/Beams/Logic/vtkSlicerIECTransformLogic.cxx#L367
2. Tabletop-driven FixedReferenceToRAS: It considers only the table top, but not the isocenter, or other parameters of the beam, see https://github.com/SlicerRt/SlicerRT/blob/3ae787b5702caf764336f2d4c52b160a046cf5ff/RoomsEyeView/Logic/vtkSlicerRoomsEyeViewModuleLogic.cxx#L980-L981

This means that if the user changes the gantry angle for example, we jump to "world 1", in which the patient is seen correctly oriented with respect to the table and the gantry, but if the user changes a patient support parameter, we jump to "world 2".

I propose that we try to fix this simply by considering the isocenter in the tabletop-driven scenario, and see how the rest of the application behaves (changing beam parameters, changing gantry angle in REV module, etc.). One thing that I think we'll have to do is that if the user changes table top parameters we need to deselect the beam, but not sure about that.

@ferdymercury please let me know if this is OK with you. Thanks!

[ferdymercury]

Oh, I see, thanks for the analysis.

Yep, sounds good. Everything tabletop-driven. Which if I understand correctly, should be similar to patient-beam-isocenter-driven, because the tabletop is glued to the posterior of the patient.

In principle, if a beam isocenter is selected, we should allow to change the couch rotation and gantry angle without having to unselect that beam. Tabletop and patient remain glued at the same position, only walls with gantry rotate around.

Or am I misunderstanding something?

[cpinter]

Thanks for the quick response!

Yes, the solution is basically to merge the two and be both beam and tabletop driven.

In principle, if a beam isocenter is selected, we should allow to change the couch rotation and gantry angle without having to unselect that beam. Tabletop and patient remain glued at the same position, only walls with gantry rotate around. Or am I misunderstanding something?

You're right, I will keep thinking about this part. It seemed that things fall apart a bit when starting to change those, hence I suggested that I do the first fix and see how the rest behaves.

[cpinter]

I implemented a very simple way to prevent lengthy collision detections: if the product of the number of triangles for the two models taking part in collision detection is higher than a threshold, that collision detection is disabled, and the user is warned. This threshold is now 1e10, so for example two models of 100,000 triangles.

This is how the warning looks:

@ferdymercury Please let me know if this works for now.

[ferdymercury]

Hi Csaba, thanks. I am not sure if this addresses the issue I was seeing. The problem was (I believe) not the algorithm taking too long, but rather that while 'sliding', it was queuing many times the collision detetion for many intermediate angles, is that possible? I think it should 'abort' / throw out of the queue previous calculations if the slider moves faster than the solution of the previous angle.

Concerning the threshold, it sounds good, but maybe there should be an additional button saying 'calculate anyway even if it takes a long time' ?

[cpinter]

I spent the whole day today trying to see the reason for this phenomenon and how to fix it

• "The problem was (I believe) not the algorithm taking too long, but rather that while 'sliding'"
  • The issue is actually that the algorithm takes too long. In some cases it takes a whole minute (see Siemens linac Gantry - PatientSupport, angle at 179 deg).
  • I checked all the parameters of the collision detection algorithm, and although I changed the mode to "FirstContact", because it matches our use case better than "AllContact", I was not able to achieve notable improvement in speed (again, tried to tweak all the parameters: CellTolerance, BoxTolerance, NumberOfCellsPerNode, etc.)
  • The algorithm cannot be cancelled once started, so we cannot "abort / throw out" ongoing calculations. We either start the calculation and wait for it, or we don't.
  • And the problem with "while sliding", is that we are not in the position to decide which "in-between values" to ignore. So I think we do need to calculate all of the values "while sliding", unless we specifically want to change the functioning of the module to only do the collision check when releasing the slider.
• "Concerning the threshold, it sounds good, but maybe there should be an additional button saying 'calculate anyway even if it takes a long time' ?"
  • Yes I can do this

[ferdymercury]

Ohh, ok, I see. In my Python implementation, the collision check was running on a separate thread that could be aborted, see https://github.com/mghro/rad-collision/blob/main/RayStation/collision_detection.py#L1013 Something along this line would be maybe doable via std::thread ?

Also, we might explore faster (less accurate) collision detection algorithms such as Dice Similarity Coefficient with a threshold.

[cpinter]

This should be a discussion towards a future project. Threading is quite difficult in Slicer, but doable. Calculating DSC is an option, but for that we'd need to ensure that the loaded models are fully closed, otherwise voxelization will fail and collision detection will not work, and I think this is not a viable requirement.

This is the new warning popup I made for disabling collision detection for the too-high-resolution parts: Would this work for now?

In the future we could also offer automatically creating lower resolution models using Uniform remesh for the purpose of collision detection.

[ferdymercury]

Sounds good, let's keep it like now and write down the improvement ideas for the future :)

[cpinter]

One more thing I did was to indicate in the message if the user chose to disable: