Fixed: related x_resolution, y_resolution

[u338steven]

Fixed: test_density(Image_Attributes_UT) in Image_attributes.rb fails #76
Fixed: test_x_resolution(Image_Attributes_UT) in Image_attributes.rb fails #81
Fixed: test_y_resolution(Image_Attributes_UT) in Image_attributes.rb fails #82

From ImageMagick 6.7.9-0:

Initialize image->x_resolution and y_resolution to 0 in image.c (previously they were initialized to DefaultResolution, which is 72.0).

ImageMagick: Changelog

[u338steven]

I'm sorry, I made a mistake...
This request is bad..

[ioquatix]

Okay, I'll wait for it to be fixed.

[ioquatix]

We should also get a travis test for 6.8 running so we can verify these fixes.

[u338steven]

Fixed
Added conditional branch about ImageMagick version

[ioquatix]

I understand what you've done here, but is this really a good fix?

1/ Some versions might have 10 or some other revision. Recommend to use Gem::Version if possible, e.g. Gem::Version.new("6.7.9") < Gem::Version.new("6.7.10")

2/ Additionally, how are we to expect user code to deal with this change? Should we set a reasonable default value rather than depending on ImageMagick? What was the motivation of the change?

[u338steven]

1/ Yeah, I think that would be better. I'll fix it to use Gem::Version.

2/ In order to distinguish an image resolution is defined or not.
I think it is necessary because some user needed.
(However, I can understand your concerns. hmm...)

A resolution of 0 means the resolution is not defined. This is a result of a bug fix where a user needed to be able to distinguish between an image whose resolution is defined @ 72DPI and those whose resolution is not defined.

ImageMagick • View topic - identify returns no resolution for TIFF generated by apple g

[ioquatix]

2/ I think we can only do what the library does. If they decided to return 0 by default, we should do that. But, it is unexpected for users, perhaps the code was working previously with this default (e.g. generating an image), but now the resolution won't be set implicitly.

It concerns me, because I can think of cases where it isn't backwards compatible. But we don't really have much choice here. In that case, what do you think about removing the test for the default value and just testing if we can assign to it and get back the assigned value?

Another possibly useful test would be to load a set of test images and verify that the correct resolution is set. But, this really should be something that is tested as part of ImageMagick - keep in mind RMagick is just a bridge...

But ultimately, we should try not to break existing code..

[u338steven]

In that case, what do you think about removing the test for the default value and just testing if we can assign to it and get back the assigned value?

That sounds good.
I think the test for the default value is not so important.
But, we might have to improve RMagick to show some warning if an image resolution is not defined.

[ioquatix]

Showing a warning could be great, i.e. when you save the image (or convert to binary blob), perhaps.

[ioquatix]

Let's fix this test and add the warning to the to do list somewhere :)

[u338steven]

Okay, I'll create a new issue for that.

[ioquatix]

Awesome! Thanks for creating a new issue.