[graphics] [9] Review request for 8155903: Crash while running imported/w3c/canvas/2d.gradient.interpolate.overlap2.html

[Arunprasad Rajkumar]

Hello Jim,

Thank you. Incorporated your review comments in 
http://cr.openjdk.java.net/~arajkumar/8155903/webrev.03

Please take a look.

--
Arun

On 5/9/2016 11:51 PM, Jim Graham wrote:
> That looks good for the case where Imin is zero, but it appears that 
> we could also have overflow as well, with a single very tiny Imin the 
> accumulation of estimatedSize with an "int" type could easily overflow 
> and become essentially a random number.  Changing the estimatedSize 
> variable to a float should prevent that related issue...
>
>             ...jim
>
> On 5/9/16 6:16 AM, Arunprasad Rajkumar wrote:
>> Hello Jim,
>>
>> Thanks for your suggestions. As of now I taking an easy way to fix 
>> the issue, New changes are available at
>> http://cr.openjdk.java.net/~arajkumar/8155903/webrev.02
>>
>> I couldn't write a reliable test case using public javafx APIs, the 
>> behavior is intermittent. However I could
>> consistently produce the issue using our DRT internal test case which 
>> is based on W3C functional test[1].
>>
>> [1] 
>> http://w3c-test.org/2dcontext/fill-and-stroke-styles/2d.gradient.interpolate.overlap2.html
>>
>> Thanks,
>> Arun
>>
>> On 5/5/2016 11:51 PM, Jim Graham wrote:
>>> Hi Arun,
>>>
>>> The change you made to the calculateSingleArray method looks like it 
>>> produces a bad array of color stops for the case
>>> where Imin is 0.  You should fall into the calculateMultipleArray 
>>> method instead which should not have any trouble
>>> with zero length intervals.  At that point you don't have to have 
>>> any code in calculateSingleArray that deals with
>>> Imin being zero because that can be part of its calling contract.
>>>
>>> That is the quick fix.
>>>
>>> The harder fix that would let us maintain speed when there is a zero 
>>> interval would be to teach the code what to do in
>>> that special case. Basically a zero interval means that we have a 
>>> case where approaching the interval point from the
>>> left is interpolating towards colorA, but leaving that point from 
>>> the right is interpolating from colorB, with a
>>> sudden transition between those 2.  In that case, a zero interval 
>>> shouldn't affect the Imin, since the Imin is trying
>>> to determine the size of each interpolated region and we don't 
>>> interpolate across a zero-sized interval.  So, the scan
>>> for Imin would simply ignore zero length intervals.  Later, the code 
>>> that populates the array in the
>>> calculateSingleArray function would know that the zero length 
>>> interval simply means swap in a new "from color" without
>>> any actual interpolation.
>>>
>>> Getting that harder fix right would require a lot of testing, so it 
>>> may be better to do the quick fix now to stop the
>>> exceptions and then deal with the optimization as a tweak filed for 
>>> later...
>>>
>>>             ...jim
>>>
>>> On 05/05/2016 01:57 AM, Arunprasad Rajkumar wrote:
>>>> Hello Jim,
>>>>
>>>> Please review the below patch.
>>>>
>>>> JIRA: https://bugs.openjdk.java.net/browse/JDK-8155903
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~arajkumar/8155903/webrev.01
>>>>
>>>> Issue: Divide by zero while adding multiple gradient stops at same 
>>>> offset.
>>>>
>>>> Regards,
>>>> Arun
>>>>
>>>>
>>>>
>>