ngageoint / tiff-java

Tagged Image File Format Java Library
https://ngageoint.github.io/tiff-java
MIT License
72 stars 34 forks source link

uses int instead of Number for image and tile sizes #11

Closed Jochen0x90h closed 7 years ago

Jochen0x90h commented 7 years ago

This makes getImageWidth() etc. more handy. but breaks compatibility. also Math.floor() and Math.ceil() was replaced by integer arithmetic.

bosborn commented 7 years ago

I'm not sure about some of these changes. The response types are objects to attempt handling unset fields (while writing, invalid tiffs, or not required). The methods returning the type Number (as opposed to Integer) are also SHORT or LONG in the TIFF 6.0 spec. A LONG is a 32 bit unsigned so the positive half of the Java long. They are set up to try to handle multiple tiff scenarios and int values are retrieved when assumptions are made for certain logic. Compatibility changes would also require some time to validate on applications using this library.

Jochen0x90h commented 7 years ago

I realized that Number can contain a 32 bit unsigned LONG. That's why the cast to int is done as late as possible the methods where a java int should be enough, namely getImageWith(), getImageHeight(), getTileWidth(), getTileHeight(), getRowsPerStrip(). imagewidth/height must be there and if FieldTagType.RowsPerStrip is absent to indicate a tiled image, then tilewidth/height must also be there. Maybe there should be a method to check if absolutely necessary fields are present. If you read a tiff without width/height then you can't do anything with it anyway, so why not add something like isValid() that can be checked before accessing it?

bosborn commented 7 years ago

You've actually moved the cast earlier rather than later. The field retrieval methods need to support the spec declared return types. Generic spec support to support all possible use cases, not just the most common ones for convenience sake. Validation methods providing information for making decisions would be fine. Appreciate the pull requests.

Jochen0x90h commented 7 years ago

Is there a use case where any of getImageWith(), getImageHeight(), getTileWidth(), getTileHeight(), getRowsPerStrip() can be greater than 2^31-1? I think the probability is extremely low and if there is one you could use getNumberEntryValue(FieldTagType.ImageWidth) because it's now public ;) Therefore getImageWidth() etc. are only convenience wrappers for 99.9% of all use cases. But it's your code, if you don't like this pull request we should drop it (maybe keep the int arithmetic instead of Math.ceil())