Re: [Gimp-developer] Updated dicom writing plugin
- From: Michael Natterer <mitch gimp org>
- To: Dov Grobgeld <dov grobgeld gmail com>
- Cc: gimp-developer-list gnome org
- Subject: Re: [Gimp-developer] Updated dicom writing plugin
- Date: Sun, 26 May 2013 15:54:38 +0200
On Sun, 2013-05-26 at 08:47 +0300, Dov Grobgeld wrote:
After having read up on gegl I realize that that the "proper" thing to
do with the Dicom plugin, is to port it to gegl. Is that correct?
Yes, absolutely, but it can mean:
- port the GIMP plug-in to libgimp's new GEGL API
(gimp_drawable_get_buffer() and friends)
- port it to be a GEGL operation
Since the loading/saving in GEGL doesn't have a GIMP plug-in's
flexibility yet, and there is no decision about an API for having
GUI and whatever needed, the preliminary path is to GEGLize the
GIMP plug-in first, they will end up reading/writing to GeglBuffer,
so moving huge chunks of the code to GEGL is easy later, while they
keep working for GIMP as if nothing had happened.
Also, we want to get to GIMP 2.10 in whatever timeframe, and this is
the safe way to get there :)
Additionally, GIMP file plug-ins are supposed to move to GIO instead
of read/write fread/fwrite and work on the basis of URIs instead of
filenames, I have ported e.g. file-pat.c and file-gbr.c as examples.
GEGL ops don't have any such plans for network transparency yet.
It involves calling gimp_register_file_handler_uri() so you get URIs
passed from the GIMP core, and using GInputStream/GOutputStream,
but this part is a nice-to-have, the GEGL porting is more important.
I further wonder if it is ok to add additional readers and writers for
any format to gegl? Here is a list of file formats that I have been
using for matrix (image) data over the years:
* npy (numerical python)
* FITS
* HDF5 (http://www.hdfgroup.org/HDF5/)
* Dicom
* Bruker files
(http://imaging.mrc-cbu.cam.ac.uk/imaging/FormatBruker)
Should I just add them, or should more rare formats be kept out of
gegl tree?
Since you are planning to do it, it's your choice, but for a smooth
transition (and ease of hacking), I suggest simply porting the GIMP
plug-in first, and leave the move to GEGL ops for later.
Have a look at the GIMP tree, there was quite some work already
done in some of these plug-ins.
Note that I'm fine with having whatever additional file formats
supported by GIMP, but of course new plug-ins must use GEGL from
the beginning :)
Another question, which I encountered with Dicom (but is relevant to
the current gimp FITS plugin as well). I chose to support only a
subset of the standard, in order to reduce external dependencies. A
much more full coverage would e.g. rely on the external library dcmtk
(http://dicom.offis.de/dcmtk.php.en). So what is the current policy?
Minimal dependencies, or rely on external libraries.
Each line of code we don't have to maintain ourselves is good, so
you are free to use external file libraries, they are most likely
more complete than what we could come up ourselves.
Regards,
--Mitch
On Wed, May 22, 2013 at 11:13 PM, Michael Natterer <mitch gimp org>
wrote:
On Tue, 2013-05-21 at 14:58 +0300, Dov Grobgeld wrote:
> Thanks Mitch,
>
> I'll do my best to honor that role. Perhaps it will even
trigger me of
> getting into some gimp development. The prospect of a
FLOAT,16- and
> 32-bit,etc version of gimp is indeed intriguing, and I
realize looking at
> the wiki that there is still a lot of work to get there.
You are most welcome to do whatever improvements on master :)
We
seriously lack manpower.
> Besides running autogen.sh and see what fails, is there a
list of
> prerequisites needed to compile and run HEAD from the git
repo?
Just have git master of babl, gegl, gimp.
> A
> nother question about gegl-gimp is about the display of a
float images. Is
> the max intensity value always mapped to display=255
(assuming a display of
> 8bit?).
Yes, in the end everything is converted to the cairo-ARGB32
babl format
and goes thorugh an 8 bit cairo surface.
> Or is the mapping a configurable property? Let's say a user
has a pair of
> float images showing an energy field where the max energy in
one image is
> 0.5 and in the second image 1.0
> .
> The user would like to look and compare the images one
next. How can she
> set the display transfer function to get a fixed
image-intensity to display
> gray level for the two images?
There is no nondestructive display transfer function unless
you count
the "Color" tools in preview mode, you could certainly do the
task like
that. The display filters are unfortunately still working on
the cairo
surface used for rendering, so in 8 bit.
Regards,
--Mitch
> On Tue, May 21, 2013 at 1:06 AM, Michael Natterer
<mitch gimp org> wrote:
>
> > On Mon, 2013-05-20 at 19:56 +0300, Dov Grobgeld wrote:
> > > Since nobody objected, I interpreted that as a silent
ok, and I commited
> > > both to master and to the gimp-2-8 branch. Please let me
know if you
> > think
> > > I was out of line and feel free to revert the commits,
and let me know
> > the
> > > proper procedure.
> >
> > The proper procedure is that since you now touched the
plugin,
> > thou shalt henceforth maintain it for all eternity.
> >
> > Thanks for the patches,
> > --mitch
> >
> > > On Mon, May 20, 2013 at 12:00 AM, Dov Grobgeld
<dov grobgeld gmail com
> > >wrote:
> > >
> > > > Based on a user bug report, I updated the dicom file
plugin. The patch
> > is
> > > > included below.
> > > >
> > > > I haven't done any contribution to gimp for ages, so I
wonder if I can
> > > > just commit it on my own? If not, should I just attach
the patch file
> > to a
> > > > bug report?
> > > >
> > > > Is it ok if I create a version to the gimp-2-8 branch
as well, so that
> > it
> > > > will reach the user quicker?
> > > >
> > > > Regards,
> > > > Dov
> > > >
> > > > diff --git a/plug-ins/common/file-dicom.c
> > b/plug-ins/common/file-dicom.c
> > > > index de11b9c..d30cd34 100644
> > > > --- a/plug-ins/common/file-dicom.c
> > > > +++ b/plug-ins/common/file-dicom.c
> > > > @@ -18,7 +18,7 @@
> > > >
> > > > /*
> > > > * The dicom reading and writing code was written
from scratch
> > > > - * by Dov Grobgeld. (dov imagic weizman ac il).
> > > > + * by Dov Grobgeld. (dov grobgeld gmail com).
> > > > */
> > > >
> > > > #include "config.h"
> > > > @@ -1218,12 +1218,17 @@
dicom_ensure_required_elements_present (GSList
> > > > *elements,
> > > > /* 0002, 0001 - File Meta Information Version */
> > > > { 0x0002, 0x0001, "OB", 2, (guint8 *) "\0\1" },
> > > > /* 0002, 0010 - Transfer syntax uid */
> > > > - { 0x0002, 0x0001, "UI",
> > > > + { 0x0002, 0x0010, "UI",
> > > > strlen ("1.2.840.10008.1.2.1"), (guint8 *)
> > "1.2.840.10008.1.2.1"},
> > > > /* 0002, 0013 - Implementation version name */
> > > > { 0x0002, 0x0013, "SH",
> > > > strlen ("GIMP Dicom Plugin 1.0"), (guint8 *)
"GIMP Dicom Plugin
> > > > 1.0" },
> > > > /* Identifying group */
> > > > + /* ImageType */
> > > > + { 0x0008, 0x0008, "CS",
> > > > + strlen ("ORIGINAL\\PRIMARY"), (guint8 *)
"ORIGINAL\\PRIMARY" },
> > > > + { 0x0008, 0x0016, "UI",
> > > > + strlen ("1.2.840.10008.5.1.4.1.1.7"), (guint8
*)
> > > > "1.2.840.10008.5.1.4.1.1.7" },
> > > > /* Study date */
> > > > { 0x0008, 0x0020, "DA",
> > > > strlen (today_string), (guint8 *)
today_string },
> > > > @@ -1236,14 +1241,23 @@
dicom_ensure_required_elements_present (GSList
> > > > *elements,
> > > > /* Content Date */
> > > > { 0x0008, 0x0023, "DA",
> > > > strlen (today_string), (guint8 *)
today_string},
> > > > - /* Modality - I have to add something.. */
> > > > + /* Content Time */
> > > > + { 0x0008, 0x0030, "TM",
> > > > + strlen ("000000.000000"), (guint8 *)
"000000.000000"},
> > > > + /* AccessionNumber */
> > > > + { 0x0008, 0x0050, "SH", strlen (""), (guint8 *)
"" },
> > > > + /* Modality */
> > > > { 0x0008, 0x0060, "CS", strlen ("MR"), (guint8 *)
"MR" },
> > > > + /* ConversionType */
> > > > + { 0x0008, 0x0064, "CS", strlen ("WSD"), (guint8
*) "WSD" },
> > > > + /* ReferringPhysiciansName */
> > > > + { 0x0008, 0x0090, "PN", strlen (""), (guint8 *)
"" },
> > > > /* Patient group */
> > > > /* Patient name */
> > > > { 0x0010, 0x0010, "PN",
> > > > strlen ("DOE^WILBER"), (guint8 *)
"DOE^WILBER" },
> > > > /* Patient ID */
> > > > - { 0x0010, 0x0020, "CS",
> > > > + { 0x0010, 0x0020, "LO",
> > > > strlen ("314159265"), (guint8 *) "314159265" },
> > > > /* Patient Birth date */
> > > > { 0x0010, 0x0030, "DA",
> > > > @@ -1251,6 +1265,12 @@
dicom_ensure_required_elements_present (GSList
> > > > *elements,
> > > > /* Patient sex */
> > > > { 0x0010, 0x0040, "CS", strlen (""), (guint8 *)
"" /* unknown */
> > },
> > > > /* Relationship group */
> > > > + /* StudyId */
> > > > + { 0x0020, 0x0010, "IS", strlen ("1"), (guint8 *)
"1" },
> > > > + /* SeriesNumber */
> > > > + { 0x0020, 0x0011, "IS", strlen ("1"), (guint8 *)
"1" },
> > > > + /* AcquisitionNumber */
> > > > + { 0x0020, 0x0012, "IS", strlen ("1"), (guint8 *)
"1" },
> > > > /* Instance number */
> > > > { 0x0020, 0x0013, "IS", strlen ("1"), (guint8 *)
"1" },
> > > >
> > > > @@ -1576,7 +1596,7 @@ add_tag_pointer (GByteArray
*group_stream,
> > > > */
> > > > if (strstr ("UI|OB", value_rep) != NULL)
> > > > {
> > > > - g_byte_array_append (group_stream, (guint8
*) 0x0000, 1);
> > > > + g_byte_array_append (group_stream, (guint8
*) "\0", 1);
> > > > }
> > > > else
> > > > {
> > > >
> > > >
> > > _______________________________________________
> > > gimp-developer-list mailing list
> > > gimp-developer-list gnome org
> > >
https://mail.gnome.org/mailman/listinfo/gimp-developer-list
> >
> >
> >
> _______________________________________________
> gimp-developer-list mailing list
> gimp-developer-list gnome org
> https://mail.gnome.org/mailman/listinfo/gimp-developer-list
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
