add dcm:window option #484

jcupitt · 2017-05-10T10:13:22Z

This patch adds a new dcm:window option which lets you override the
window center and width stored in the DICOM file with a new centerXwidth
pair. You can also use centerX or Xwidth. The window settings are ony
applied if the dcm:rescale option is enabled.

This is useful for transforming images from scan units to display units for
visualisation.

Example:

convert -define dcm:rescale -define dcm:window=2500x3000 MRIm5.dcm x.tif

This patch also reverts an earlier change which moved the transform for
signed data into the main path. Window handling assumes a 0 - max_value
output range and takes no account of signedness of the data, so
the subtract for signed data is not appropriate. Perhaps it should be,
but we'd need to track min_value as well, and that's beyound the scope
of this PR.

Related PR:

#483

DICOM spec:

https://www.dabsoft.ch/dicom/3/C.11.2.1.2/

This patch adds a new `dcm:window` option which lets you override the window center and width stored in the DICOM file with a new centerXwidth pair. You can also use centerX or Xwidth. The window settings are ony applied if the `dcm:rescale` option is enabled. This is useful for transforming images from scan units to display units for visualisation. Example: convert -define dcm:rescale -define dcm:window=2500x3000 MRIm5.dcm x.tif This patch also reverts an earlier change which moved the transform for signed data into the main path. Window handling assumes a 0 - `max_value` output range and takes no account of signedness of the data, so the subtract for signed data is not appropriate. Perhaps it should be, but we'd need to track `min_value` as well, and that's beyound the scope of this PR. Related PR: ImageMagick#483 DICOM spec: https://www.dabsoft.ch/dicom/3/C.11.2.1.2/

dlemstra · 2017-05-10T10:33:45Z

coders/dcm.c

+	  {
+            GeometryInfo
+              geometry_info;
+            MagickStatusType


This needs a newline between the variables :)

OK, I'll add one.

dlemstra · 2017-05-10T10:34:23Z

coders/dcm.c

@@ -4097,7 +4095,24 @@ static Image *ReadDCMImage(const ImageInfo *image_info,ExceptionInfo *exception)
          }
        option=GetImageOption(image_info,"dcm:rescale");
        if (option != (char *) NULL)
-          info.rescale=IsStringTrue(option);


You should really use IsStringTrue here.

The problem I had with IsStringTrue() is you can't default true. So you must write -define dcm:rescale=true, plain -define dcm:rescale will actually turn it off.

I would prefer it as a key value pair and force the user to set it to true.

OK, if you're sure.

I'm sure 👍

dlemstra · 2017-05-10T10:38:32Z

coders/dcm.c

+          info.rescale=(int) ParseCommandOption(MagickBooleanOptions,
+	    MagickFalse,option);
+        option=GetImageOption(image_info,"dcm:window");
+        if (option != (char *) NULL)


I wonder if we should make it so that if this option is not null we set info.rescale to MagickTrue. This option will only do something when info.rescale is true. And the you would only need to do this: convert -define dcm:window=2500x3000 MRIm5.dcm x.tif

jcupitt · 2017-05-10T11:36:38Z

I had another thought about signedness. The .signed_data field of course refers to the input data (the data in the file), not the output. So the subtract should always happen, and it should happen before rescale processing. I'll make this change too.

code review changes, plus move signed data handling before rescale and window processing

dlemstra · 2017-05-11T05:22:19Z

Your changes have been merged (and I added the IsStringTrue check). Could you also send the pull request for IM7?

jcupitt · 2017-05-11T05:59:55Z

Ooop, sorry, I did make that change, I think I forgot to push it.

I'll try an IM7 patch that does the same thing.

jcupitt · 2017-05-11T06:06:02Z

I meant to say, I didn't write a note in the ChangeLog, nor any docs or tests. Do they need doing too?

dlemstra · 2017-05-11T06:36:06Z

Forgot to mention that you should add something to the change log. And our website is in another repo. We have a page where we explain all the defines. Would love it if you could add something there for the two new options that you created.

docs for new options added in ImageMagick/ImageMagick#484

jcupitt · 2017-05-11T07:42:28Z

I added a note to the ChangeLog in this branch, hopefully you can just merge that too.

I added some docs to the website.

IM7 next!

jcupitt · 2017-05-11T08:18:42Z

IM7 PR: #485

dlemstra · 2017-05-11T08:29:18Z

I don't see the push for that ChangeLog change?

jcupitt · 2017-05-11T08:31:08Z

I pushed it, but it looks like I have to open a new PR for github to see it. Hang on.

docs for new options added in ImageMagick/ImageMagick#484

Upstream changes: 2017-05-12 7.0.5-6 Cristy <quetzlzacatenango@image...> * Release ImageMagick version 7.0.5-6, GIT revision 20039:9371904:20170512. 2017-05-10 7.0.5-6 John Cupitt <jcupitt@gmail.com> * Revise DICOM window and rescale handling (reference ImageMagick/ImageMagick#484) 2017-05-06 7.0.5-6 Cristy <quetzlzacatenango@image...> * Restore the -alpha Shape option (reference https://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=31879). * Fix transient PDF bug (reference ImageMagick/ImageMagick#463). * The +opaque option now works on all channels (reference https://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=31862). * Ensure backwards compatibility for the -combine option (reference https://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=31855). * Check for EOF conditions for RLE image format. * Reset histogram page geometry (reference https://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=31920).

Pkgsrc changes: Adapt to new version, PLIST (2 new files). Upstream changes: 2017-05-12 6.9.8-5 Cristy <quetzlzacatenango@image...> * Release ImageMagick version 6.9.8-5, GIT revision 11575:186b15d:20170512. 2017-05-10 6.9.8-5 John Cupitt <jcupitt@gmail.com> * Revise DICOM window and rescale handling (reference ImageMagick/ImageMagick#484) 2017-05-08 6.9.8-5 Cristy <quetzlzacatenango@image...> * Fix transient PDF bug (reference ImageMagick/ImageMagick#463). * Check for EOF conditions for RLE image format. * Reset histogram page geometry (reference https://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=31920). 2017-04-24 6.9.8-4 Cristy <quetzlzacatenango@image...> * Release ImageMagick version 6.9.8-4, GIT revision 11521:d7433aa:20170424. 2017-03-26 6.9.8-4 Cristy <quetzlzacatenango@image...> * Minimize buffer copies to improve OpenCL performance. * Patch a PCD writer problem, dark pixels (reference https://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=3164). * Support ICC based PDF's (reference ImageMagick/ImageMagick#417). 2017-03-24 6.9.8-3 Cristy <quetzlzacatenango@image...> * Release ImageMagick version 6.9.8-3, GIT revision 11444:3f523e5:20170324. 2017-03-20 6.9.8-3 Cristy <quetzlzacatenango@image...> * MagickWand-config, use --cflags, not --clags (reference https://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=31602). * Respect -loop option for animate -window (reference https://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=31619). 2017-03-18 6.9.8-2 Cristy <quetzlzacatenango@image...> * Release ImageMagick version 6.9.8-2, GIT revision 11436:a912791:20170318. 2017-03-14 6.9.8-2 Cristy <quetzlzacatenango@image...> * Support namespaces for the security policy. 2017-03-11 6.9.8-1 Cristy <quetzlzacatenango@image...> * Release ImageMagick version 6.9.8-1. 2017-03-03 6.9.8-0 Cristy <quetzlzacatenango@image...> * Release ImageMagick version 6.9.8-0, GIT revision 11408:da91a7c:20170311. 2017-03-06 6.9.8-0 Cristy <quetzlzacatenango@image...> * Respect throttle policy (reference ImageMagick/ImageMagick#393). * Support the -authenticate option for PDF (reference https://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=31530). 2017-03-03 6.9.7-10 Cristy <quetzlzacatenango@image...> * Release ImageMagick version 6.9.7-10, GIT revision 11396:44b1bc6:20170303. 2017-02-21 6.9.7-10 Cristy <quetzlzacatenango@image...> * Fix Spurious memory allocation message (reference https://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=31438). * Identical images should return inf for PSNR (reference https://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=31487).

docs for new options added in ImageMagick/ImageMagick#484

jcupitt mentioned this pull request May 10, 2017

make DICOM read work for fractional rescale #483

Merged

dlemstra requested changes May 10, 2017

View reviewed changes

incorportate code review

f340b3b

code review changes, plus move signed data handling before rescale and window processing

dlemstra merged commit 79984bc into ImageMagick:ImageMagick-6 May 10, 2017

dlemstra added a commit that referenced this pull request May 10, 2017

IsStringTrue should be used instead in #484.

0992c5f

jcupitt added a commit to jcupitt/Website that referenced this pull request May 11, 2017

document new dcm options

0f0dae9

docs for new options added in ImageMagick/ImageMagick#484

jcupitt mentioned this pull request May 11, 2017

document new dcm options ImageMagick/Website#1

Merged

dlemstra pushed a commit to ImageMagick/Website that referenced this pull request May 11, 2017

document new dcm options

2b8be94

docs for new options added in ImageMagick/ImageMagick#484

dlemstra pushed a commit to ImageMagick/Website that referenced this pull request Aug 28, 2017

document new dcm options

70228c0

docs for new options added in ImageMagick/ImageMagick#484

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add dcm:window option #484

add dcm:window option #484

jcupitt commented May 10, 2017

dlemstra May 10, 2017

jcupitt May 10, 2017

dlemstra May 10, 2017

jcupitt May 10, 2017

dlemstra May 10, 2017

jcupitt May 10, 2017

dlemstra May 10, 2017

dlemstra May 10, 2017

jcupitt May 10, 2017

jcupitt commented May 10, 2017

dlemstra commented May 11, 2017

jcupitt commented May 11, 2017

jcupitt commented May 11, 2017

dlemstra commented May 11, 2017

jcupitt commented May 11, 2017

jcupitt commented May 11, 2017

dlemstra commented May 11, 2017

jcupitt commented May 11, 2017

add dcm:window option #484

add dcm:window option #484

Conversation

jcupitt commented May 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcupitt commented May 10, 2017

dlemstra commented May 11, 2017

jcupitt commented May 11, 2017

jcupitt commented May 11, 2017

dlemstra commented May 11, 2017

jcupitt commented May 11, 2017

jcupitt commented May 11, 2017

dlemstra commented May 11, 2017

jcupitt commented May 11, 2017