Added GD tests

[radarhere]

This code is duplicated at https://github.com/python-pillow/Pillow/blob/master/PIL/ImageFile.py#L87. The only difference made by removing it here is that filename may be None instead of an empty string.

[coveralls]

Changes Unknown when pulling 060bfaf on radarhere:gd into * on python-pillow:master*.

[wiredfool]

First of all, we have no tests at all on the GdImageFile.py. Nothing references it. Anywhere.

Also, when looking at the comments in the file, there's this:

# NOTE: This format cannot be automatically recognized, so the
# class is not registered for use with Image.open().  To open a
# gd file, use the GdImageFile.open() function instead.

# THE GD FORMAT IS NOT DESIGNED FOR DATA INTERCHANGE.  This
# implementation is provided for convenience and demonstrational
# purposes only.

So, it looks like it's not designed to be used from within the ImageFile process. So, it doesn't look like duplicated code.

[radarhere]

The GdimageFile class inherits from ImageFile. So the parameters passed in are handled by ImageFile's init method. Yes?

[radarhere]

Okay, I have now added basic tests.

[hugovk]

Is there some documentation about the GD uncompressed format somewhere?

Where's the test GD file from, or how was it generated? Do we know it's a valid GD image?

How do we know the changes are correct?

Without the changes, the test file gives:

>>> from PIL import GdImageFile
>>> im = GdImageFile.open("/tmp/Downloads/gd")
>>> im.size
(65535, 10)
im.show()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "PIL/Image.py", line 1783, in show
    _show(self, title=title, command=command)
  File "PIL/Image.py", line 2537, in _show
    _showxv(image, **options)
  File "PIL/Image.py", line 2542, in _showxv
    ImageShow.show(image, title, **options)
  File "PIL/ImageShow.py", line 51, in show
    if viewer.show(image, title=title, **options):
  File "PIL/ImageShow.py", line 75, in show
    return self.show_image(image, **options)
  File "PIL/ImageShow.py", line 94, in show_image
    return self.show_file(self.save_image(image), **options)
  File "PIL/ImageShow.py", line 90, in save_image
    return image._dump(format=self.get_format(image))
  File "PIL/Image.py", line 576, in _dump
    self.load()
  File "PIL/ImageFile.py", line 171, in load
    self.map, self.size, decoder_name, extents, offset, args
ValueError: buffer is not large enough

With the changes, we get:

>>> from PIL import GdImageFile
>>> im = GdImageFile.open("/tmp/Downloads/gd")
>>> im.size
(10, 20)
>>> im.show()
>>> im
<PIL.GdImageFile.GdImageFile image mode=L size=10x20 at 0x108B3AD50>

The image shows as a black rectangle in Mac Preview, but how can we verify this is correct? (e.g. ImageMagick cannot identify it.)

The transparent/transparency key isn't tested, so docs would also be useful for confirming this change.

[radarhere]

I generated the image using PHP's GD library.

<?php
$im = imagecreatetruecolor(10, 20);
imagegd($im, 'gd');

I changed the key for consistency with other formats.

[hugovk]

Would it be possible to save a Hopper image in GD format with PHP?

Or make an image with something a bit more than plain black, maybe just a line or two? Even better if it can include transparency.

Then we can compare it to a corresponding image in another format, and at least assert some properties like size.

[radarhere]

Okay. I used the following code to create a hopper image -

<?php
$im = imagecreatefromjpeg('Tests/images/hopper.jpg');
imagegd($im, 'hoppergd');

The resulting image loaded by Pillow is clearly not correct.

[hugovk]

The GD image opened and saved as JPG by PHP:

The GD image opened and saved as JPG by Pillow:

Let's add this to #2373.

[hugovk]

Flipping that -1 to 1 flips the picture:

self.tile = [("raw", (0, 0)+self.size, 775, ("L", 0, -1))]

self.tile = [("raw", (0, 0)+self.size, 775, ("L", 0, 1))]

Changing those "P"s to "L" as suggested by the FIXME gives:

More work is needed! Note this test image is a GD v2 file because i16(s[0:2]) is 65535. And let's call the file hopper.gd.

[radarhere]

Thanks for that. While I'm happy to go with whatever the consensus is, my concern with naming the test file hopper.gd is giving the false impression that .gd is a usable extension.

[hugovk]

.gd is mentioned several times in the PHP source above.

[radarhere]

Fair enough! I mistakenly inferred that it wasn't valid from the comment in GdImageFile that 'This format cannot be automatically recognized'.

[wiredfool]

@radarhere I think that comment means that we can't figure out the file type from the content, as the file extension really doesn't factor into the open loop. (It's only taken into account when you save, and then only if a format isn't specified)

And. Another format that doesn't work.

[radarhere]

Okay, I've added the extension now.

[wiredfool]

Here's what I can find for a spec: libgd/libgd@5ee392d#diff-909ccc1a14087aa650c846156ae32572

GD and GD2 are listed as both proprietary and obsolete. On the bright side, GD2 can be recognized from the first 4 bytes.

I suspect that the problem is in the interpretation of the palette, as there's the basis of the image there. OTOH, the palette is supposed to be 256*32bit, and we're starting at offset 775, so there's something odd there.

[homm]

This line is not covered by tests

[hugovk]

Whilst GD files are still not working properly, this is at least a step in the right direction.

With a better PR title, I'm okay for this to be merged, and further work can be done if anyone is so inclined (for example, by using libgd directly).

Alternatively, I'm also good for this GD code to be removed completely (perhaps in the next major release). We've never had any issues opened about it. The GD and GD2 image formats were invented by libgd and they say:

The GD image format is a proprietary image format of libgd. * The GD image format is a proprietary image format of libgd. It has to be regarded as being obsolete, and should only be used for development and testing purposes.

libgd/libgd#288
libgd/libgd@5ee392d

[codecov]

Codecov Report

Merging #1817 into master will increase coverage by 0.13%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##           master    #1817      +/-   ##
==========================================
+ Coverage   83.28%   83.42%   +0.13%     
==========================================
  Files         168      168              
  Lines       22621    22615       -6     
  Branches     2794     2795       +1     
==========================================
+ Hits        18841    18866      +25     
+ Misses       3780     3749      -31

Impacted Files	Coverage Δ
src/PIL/GdImageFile.py	96.15% <91.66%> (+96.15%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 894ff64...6b224be. Read the comment docs.

[radarhere]

I've renamed the PR.

Note that the current code is what I believe to be most theoretically correct, while also running without errors - XBGR is clearly not correct, but that was done in order to keep the rest of the code that resulted from my current understanding. While I may not at all be on the right path, if I am, this will provide a starting point for the next person.

[aclark4life]

Thanks