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:

SlicerRT/RoomsEyeView/Logic/vtkSlicerRoomsEyeViewModuleLogic.cxx

Lines 1030 to 1037 in 19572b5

	if (panelMovement > 0)
	{
	rasToRotatedRasTransform->RotateZ(68.5);
	}
	else
	{
	rasToRotatedRasTransform->RotateZ(panelMovement + 68.5);
	}

[ferdymercury]

Looks very good, thanks for your work!

Some comments below:

• 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.
• 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.
• 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.
• 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"}
• 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 :) .
• 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.
• 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:

• Disabled (never loaded)
• Passive (loaded but not checked for collision)
• 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 (#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

  SlicerRT/RoomsEyeView/Logic/vtkSlicerRoomsEyeViewModuleLogic.cxx

  Lines 1212 to 1221 in 4c0ceb8

  	//TODO: Support this from the descriptor JSON file
  	//char* treatmentMachineType = parameterNode->GetTreatmentMachineType();
  	//if (treatmentMachineType && !strcmp(treatmentMachineType, "VarianTrueBeamSTx"))
  	//{
  	tableTopDisplacementScaling = 0.525;
  	//}
  	//else if (treatmentMachineType && !strcmp(treatmentMachineType, "SiemensArtiste"))
  	//{
  	// tableTopDisplacementScaling = 0.095;
  	//}

  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?

[cpinter]

I added your grant acknowledgment to the Room's Eye View source files (see c7f6452) and to the module acknowledgment section (see 52a3900). 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
   

   SlicerRT/Beams/Logic/vtkSlicerIECTransformLogic.cxx

   Line 367 in 3ae787b

   fixedReferenceToRasTransform->Translate(isocenterPosition[0], isocenterPosition[1], isocenterPosition[2]);

2. Tabletop-driven FixedReferenceToRAS: It considers only the table top, but not the isocenter, or other parameters of the beam, see
   

   SlicerRT/RoomsEyeView/Logic/vtkSlicerRoomsEyeViewModuleLogic.cxx

   Lines 980 to 981 in 3ae787b

   	fixedReferenceToRASTransform->Concatenate(vtkTransform::SafeDownCast(tableTopToTableTopEccentricRotationTransformNode->GetTransformFromParent()));
   	fixedReferenceToRASTransform->Concatenate(vtkTransform::SafeDownCast(patientSupportRotationToFixedReferenceTransformNode->GetTransformFromParent()));

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: