[OpenJDK 2D-Dev] [9] 8152293: Incomplete fraction reduction in getValueAsString() for TIFFTag.TIFF_RATIONAL, TIFFTag.TIFF_SRATIONAL

[Alexander Stepanov]

Hello Brian,

Thank you for the comments (hopefully I wasn't too confident picking the 
issue).

Yes, from the "do not harm" position it seems better to remove all the 
reduction-related fragments (leaving these worries to the user), but 
keep the zero denominator checks + the sign checks for the unsigned 
fractions.

One more silly question - if some upper bound (2^32 - 1) check should be 
added for TIFF_RATIONAL's numerator/denominator? At a 1st glance, it 
seems to be needless; on the other hand, it seems that the longs should 
be used to store 32-bit unsigned integers(?). Not sure if it could cause 
any side effects.

Thanks,
Alexander


On 11/15/2016 12:13 AM, Brian Burkhalter wrote:
> Hello Alexander,
>
> Thanks for picking up this issue. I actually worked on it a little last week as well but did not yet finish a patch.
>
> The TIFF 6.0 specification does not provide any guidance on the handling of rational values in terms of the three questions at hand:
>
> 1 - Should the rational values be stored as given or with common factors divided out?
> 2 - Should the displayed form of rational values be fractions as given (if these are not reduced before being stored), fractions with common factors divided out, or real values obtained by floating point division?
> 3 - Should providing a rational value with a zero denominator be an error?
>
> Also, the libtiff tools ‘tiffdump’ and ‘tiffinfo’ seem to handle rational values inconsistently. For example, for the images in the libtiffpic set of sample images, XResolution and YResolution are reported to have a value such as “72” instead of “72/1”, the values of ReferenceBlackWhite are displayed as-is, but those of YCbCrCoefficients are divided out to real values.
>
> My preference right now leans towards storing and displaying the fractions in their initial, non-reduced state and throwing an exception if a zero denominator is encountered (which would also cover the case 0/0). Note however that the zero denominator check is not straightforward as it is possible to populate the rational array after the TIFFField is created and this is actually done in the TIFF ImageWriter. Because of this the zero denominator check might be infeasible. Also note that removing the existing fractional reduction {k*q, k} —> {q, 1} from getValueAsString() would require changing the API documentation hence a CCC request. The zero denominator check if implemented would also require an API update.
>
> Thanks,
>
> Brian
>
> On Nov 14, 2016, at 7:02 AM, Alexander Stepanov <alexander.v.stepanov at oracle.com> wrote:
>
>> P.S. please let me know if for some purposes the fractions should be stored in the initial (non-reduced) state. at a 1st glance it is not required, but maybe I'm wrong.
>>
>> If this is the case, then only some part of the changes should probably be applied:
>> 1. remove needless reduction in getValueAsString()
>> 2. check the sign for the unsigned fractions
>> 3. forbid null denominators (?)
>>
>> On 11/14/2016 5:30 PM, Alexander Stepanov wrote:
>>> Hello,
>>>
>>> Could you please review the following fix
>>> http://cr.openjdk.java.net/~avstepan/8152293/webrev.00/
>>> for
>>> https://bugs.openjdk.java.net/browse/JDK-8152293 ?
>>>
>>> Thanks,
>>> Alexander