SlicerRt / SlicerRT

Open-source toolkit for radiation therapy research, an extension of 3D Slicer. Features include DICOM-RT import/export, dose volume histogram, dose accumulation, external beam planning (TPS), structure comparison and morphology, isodose line/surface generation, etc.
https://slicerrt.org
128 stars 60 forks source link

Isodose model generation fails due to non-default FeatureAngle value #227

Open cpinter opened 1 year ago

cpinter commented 1 year ago

There is a thermal dose volume (with value range 0-300) for which the module generation yields very incorrect isodose surface (for level value 240).

This is a correct-looking result image

And here is the result yielded by the Isodose module image

The only difference between the two results is that the top one was computed with the default FeatureAngle value (15), and the bottom with the value set by Isodose logic (60), see https://github.com/SlicerRt/SlicerRT/blob/4c0ceb8900f6c12ab9b66e2ae6e7d1077c99c409/Isodose/Logic/vtkSlicerIsodoseModuleLogic.cxx#L754

Does anyone (@lassoan @gregsharp) remember why we chose this non-default value? There are no comments in the code. I found the commit, however, where this value was set (by Kevin Wang): https://github.com/SlicerRt/SlicerRT/commit/dc07b613a1385dc8e5cdeca5d4fcbb0a5781f4fa The comment was: "Fixed the isodose color in 2d viewer. It probably has something to do with the scalar and normals computation. Once I modified the pipeline to generate the polydata, it works fine now", so it was not clear to him either but there were 2D display issues before this change. That said, it was more than ten years ago...

If there is no good reason I'd remove this non-default feature angle setting. Thank you.

lassoan commented 1 year ago

Feature angle of 60 deg should not cause much trouble.

SetMaximumError to 1 looks suspicious. Is it 1mm or 100% of the bounding box? Both can be quite bad.

Number of smoothing iterations set to 2 does not make much sense either. Smoothing is an interative method and we cannot expect much to happen in 2 iterations.

cpinter commented 1 year ago

Feature angle of 60 deg should not cause much trouble

In this case, however, changing this single property fixes the problem (I can confirm this). Do you suggest to experiment with other properties to try to fix this issue and leave the feature angle as is?

lassoan commented 1 year ago

It might be just a matter of luck that changing the feature angle fixes the issue in this case. The number of iterations and maximum error values look more concerning to me.

The safest would be to reduce decimation and smoothing. Maybe remove decimation completely or even better replace with uniform remeshing (unfortunately ACVD is not in VTK yet). Then test it on many cases to verify that the result is smooth and still accurate.

cpinter commented 1 year ago

The safest would be to reduce decimation and smoothong

Yes I agree with this.

We could just change TargetReduction to 0.4 or 0.3, that seems to be a safe option if you'd rather not touch FeatureAngle.

lassoan commented 1 year ago

My main concern that the processing is unstable if changing the feature angle cause such huge difference. Decreasing the feature angle does not really stabilize the processing. Decreasing the target reduction should stabilize the process, so if that fixes the issue then it is a better solution.

What is very strange is that for continuous images, marching cubes should provide a fairly smooth output. What is the scalar type of the dose volume and what are the isovalues? I'm also not sure about why the vtkImageReslice is needed and what resolution is used (fixed isotropic 1mm? what if the volume is small?).

cpinter commented 1 year ago

The dose volume is of float, scalar range is 0 to 300, and the level used here was 240.

cpinter commented 1 year ago

I think the image reslice is used to apply the volume node's geometry.

lassoan commented 1 year ago

OK, so it seems that image resampling is done to achieve uniform remeshing. But then we don't need to do decimation at all, but we should just use the output image spacing that matches the desired resolution. After this resampling we only need marching cubes and maybe a little smoothing (but if we used bicubic interpolation for the image resampling and the spacing was right then probably we don't even need smoothing). Normal computation should be enabled in marching cubes to have a nice, smooth-looking surface in 3D.

cpinter commented 1 year ago

Do you think if I just removed the decimation step it would be enough? Or if we want to keep it an option I can add a member to the parameter node.