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

Phil Race philip.race at oracle.com
Wed Jul 9 18:44:46 UTC 2014


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.
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>




More information about the 2d-dev mailing list