[OpenJDK 2D-Dev] [9] Review request for 8041460, PNGImageWriter creates metadata with incorrect bitDepth

[Andrew Brygin]

The updated fix looks fine to me.

Thanks,
Andrew

On 7/9/2014 10:44 PM, Phil Race wrote:
> This now looks fine. Since the fix changed since Andrew approved,
> make sure you get a 2nd reviewer.
>
> Also the putback comment must use bug ID 4991647
>
> -phil.
>
> On 7/9/2014 9:53 AM, mikhail cherkasov wrote:
>> Hi Phil,
>>
>> I updated testcase: 
>> http://cr.openjdk.java.net/~mcherkas/8041460/webrev.04/
>> I replaced it with testcase from 499164
>>
>> Thanks,
>> Mikhail.
>>
>> On 7/8/2014 5:30 PM, Phil Race wrote:
>>> This is now bug 4991647 so the test needs to reflect that ..
>>>
>>> -phil.
>>>
>>> On 7/8/14 5:34 AM, mikhail cherkasov wrote:
>>>> Hello Phil,
>>>>
>>>> Please review a new version:
>>>> http://cr.openjdk.java.net/~mcherkas/8041460/webrev.03/
>>>>
>>>> I applied changes that you advised, also I removed the comment at 
>>>> all, because
>>>> the new code isn't required clarification.
>>>>
>>>> Thanks,
>>>> Mikhail.
>>>>
>>>> On 7/8/2014 8:43 AM, Phil Race wrote:
>>>>> PS the comments referring to bug id et al seem too much.
>>>>> I am not sure we need a new comment here just because there was a 
>>>>> bug.
>>>>> That may be something sustaining may want to do in 'old' releases but
>>>>> the ongoing clutter for the mainstream release isn't something we 
>>>>> want.
>>>>>
>>>>> -phil.
>>>>>
>>>>> On 7/7/14 2:45 PM, Phil Race wrote:
>>>>>> Since its returning an array index into theIHDR_bitDepths array I 
>>>>>> think this should be written as
>>>>>>
>>>>>> IHDR_bitDepth =IHDR_bitDepths[getEnumeratedAttribute(node, 
>>>>>> "bitDepth",  IHDR_bitDepths)];
>>>>>>
>>>>>> .. but ensuring the line <=80 chars long
>>>>>>
>>>>>> Also note that this is a complete duplicate of
>>>>>> https://bugs.openjdk.java.net/browse/JDK-4991647
>>>>>>
>>>>>> Always remember to look for duplicates !
>>>>>> So I have closed 8041460 and you should push under 4991647 - once
>>>>>> the code is fixed and reviewed.
>>>>>>
>>>>>> I have assigned 4991647 to you.
>>>>>>
>>>>>>
>>>>>>  -phil.
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 6/23/2014 6:09 AM, Andrew Brygin wrote:
>>>>>>> Hello Mikhail,
>>>>>>>
>>>>>>>  the fix looks fine to me.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Andrew
>>>>>>>
>>>>>>> On 6/9/2014 10:32 PM, mikhail cherkasov wrote:
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> please review the fix:
>>>>>>>> http://cr.openjdk.java.net/~mcherkas/8041460/webrev.02/
>>>>>>>> for the following bug:
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8041460
>>>>>>>>
>>>>>>>> The problem is that during merging of metadata trees we take 
>>>>>>>> index of IHDR_bitDepths
>>>>>>>> and set this value to IHDR_bitDepth, but IHDR_bitDepth requires 
>>>>>>>> a real number of bits.
>>>>>>>> IHDR_bitDepths - store all valid values: "1", "2", "4", "8", 
>>>>>>>> "16", so to get
>>>>>>>> valid value we need: 1 << index.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Mikhail.
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>