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@xxxxxxxx> > 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@xxxxxxxx> 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@xxxxxxxxx > > > >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@xxxxxxxxxxxxxxxxxxxx). > > > > > + * by Dov Grobgeld. (dov.grobgeld@xxxxxxxxx). > > > > > */ > > > > > > > > > > #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@xxxxxxxxx > > > > > https://mail.gnome.org/mailman/listinfo/gimp-developer-list > > > > > > > > > > > _______________________________________________ > > gimp-developer-list mailing list > > gimp-developer-list@xxxxxxxxx > > https://mail.gnome.org/mailman/listinfo/gimp-developer-list > > > > _______________________________________________ gimp-developer-list mailing list gimp-developer-list@xxxxxxxxx https://mail.gnome.org/mailman/listinfo/gimp-developer-list
