GNOME Bugzilla – Bug 131016
tiff plugin doesn't support layer positions
Last modified: 2004-01-09 21:18:11 UTC
Gimp 2pre1 ignores the layer offsets in tiff files, the attached patch adds layer offset support.
Created attachment 23164 [details] [review] honor layer offset when loading tiff files
Should be applied before 2.0. However the patch has a number of issues and will need fixing: (1) There must not be // comments in the GIMP code. (2) A gchar* variable should not be initialized to 0 but to NULL rather. (3) There are a number of unneeded casts introduced by the patch. These should be avoided in order to keep the code legible. Also I would suggest to not use gfloat for layer_offset_[xy] but to use the type that is used in the libtiff API.
Created attachment 23171 [details] [review] cleanup - next try ;)
Hmm, issues (2) and (3) are still apparent but you removed some rounding that seemed correct. Also, are you sure that it is correct to resize the image to the size of the largest layer? Shouldn't the image size be contained in the TIFF header?
issue 2: ups. fixed in next patch issue 3: what is the correct way in gimp to round a float to the nearest integer and assign it to an int? X_POSITION and Y_POSITION are not nessecarily integers. The multi-image tiff file consists of several IFD's. One for each subimage. (tiffspec p. 16). this IFD holds the attributes of each subimage (layer). There is not global, dummy directory, which contains attributes common to all layers. at least its not mentioned or required by the spec. in order to show the whole image, I set the image size to show all images in complete, from in the region of (0,0) to (max_x_used, max_y_used). I could also implement (min_x_used, min_y_used) to (max_x_used, max_y_used).
Excuse my ignorance, I haven't read the spec, it only looked suspicious to me. So resizing seems to be a good idea then. I would prefer to see the image resized to the bounding box of all layers though. The preferred way to round a float to the nearest integer is to use the ROUND() macro. To assign it to an integer, you simply assign it w/o an explicit cast.
Created attachment 23175 [details] [review] 3rd rev. uses ROUND() + resizes image to bounding box of all layers
ok, implemented the bounding box. I hope using INT_MAX is ok.
Looks just like the previous version; I think you attached the wrong patch. Please use G_MAXINT for portability.
Created attachment 23178 [details] [review] rev 4. (sorry for attaching the old version last time)
This looks good and seems to work fine. There's one issue though. Your patch adds code that attempts to use the layer name from the TIFF file. This is nice but what is the encoding of this string? For GIMP2 this must be UTF-8 encoding or we need to add a conversion routine. Also we should set the layer name on save if that's possible. I have now applied your patch without this change. 2004-01-09 Sven Neumann <sven@gimp.org> * plug-ins/common/tiff.c (load_image): applied a patch from Pablo d'Angelo that fixes layer offsets for multipage TIFF files (bug #131016).
The tiff spec requires 7bit clean strings, so it shouldn't be a problem. on the other hand, programs might not comply to the spec. Support for saving is still missing.. the original multi layer patch didn't support save multiple layers either.
If the spec is clear about 7bit ascii then we can use the name as specified in the file after checking that it is indeed 7bit only. I'd appreciate to see this handled in a new report. Saving the name will be more complex however since GIMP layer names are not restricted to 7bit.