[OpenJDK 2D-Dev] [9] RFR JDK-8162796: [macosx] LinearGradientPaint and RadialGradientPaint are not printed on OS X.

[Philip Race]

+1

-phil.

On 10/25/16, 11:01 PM, Prasanta Sadhukhan wrote:
> Please find the webrev with modified testcase
>
> http://cr.openjdk.java.net/~psadhukhan/8162796/webrev.03/
>
> Regards
> Prasanta
> On 10/26/2016 2:45 AM, Philip Race wrote:
>> Hi,
>>
>> This looks to work properly although I have a couple of nits and a 
>> concern that perhaps we just need to live with.
>>
>> * @requires (os.family == "mac")
>>
>> Why ? This test should be valid across OSes .. and SFAIK Mac is the 
>> only place it would fail without this fix.
>> Both tests have this issue.
>>
>> I'd like the test(s) to use  [something like]
>>   Point2D.Double p2 = new Point2D.Double(DIM/2.0, DIM/2.0);
>>
>> Otherwise the size of the shape to be filled neatly matches the span 
>> of the gradient and that can hide bugs.
>>
>> I am not sure how much to be concerned that the "raster" path relies 
>> on creating a custom paint whose
>> size matches the bounds of the shape to which the paint will be 
>> applied ..
>> 536             PaintContext context = 
>> sg2d.paint.createContext(sg2d.getDeviceColorModel(), userBounds, 
>> userBounds, sIdentityMatrix, sg2d.getRenderingHints());
>> 537             WritableRaster raster = (WritableRaster) 
>> (context.getRaster(userBounds.x, userBounds.y, userBounds.width, 
>> userBounds.height));
>>
>> If applied to a very large shape it will need a very large image.
>> But this seems the only way to get the gradient with the right 
>> reflect/repeat behaviour.
>>
>> -phil.
>>
>> On 10/24/16, 1:38 AM, Prasanta Sadhukhan wrote:
>>> Please find modified webrev with Linear/Radial Gradient NO_CYCLE 
>>> option catered through quartz api and REPEAT/REFLECT option catered 
>>> via raster path.
>>> http://cr.openjdk.java.net/~psadhukhan/8162796/webrev.02/
>>>
>>> Regards
>>> Prasanta
>>> On 10/22/2016 9:28 PM, Prasanta Sadhukhan wrote:
>>>>
>>>>
>>>> On 10/22/2016 4:50 AM, Philip Race wrote:
>>>>> Well it is confusing since in particular this exact change is in 
>>>>> both webrevs :-
>>>>> http://cr.openjdk.java.net/~psadhukhan/8162796/webrev.01/src/java.desktop/macosx/native/libawt_lwawt/awt/QuartzSurfaceData.m.sdiff.html 
>>>>>
>>>>>
>>>> Yes. This is same and I had to add this in both webrevs as 
>>>> TexturePaint patch is not yet approved.
>>>>> One thing that I think seems less ideal about this fix, at least for
>>>>> the Gradient case is that it is creating an image which shows 
>>>>> pixelisation
>>>>> effects when I zoom in using Preview ..
>>>>>
>>>>> I don't think the previous code showed those effects so it may be you
>>>>> want a hybrid, using the original code when you can.
>>>> ok. I will make NO_CYCLE option to go through quartz and other 
>>>> REPEAT/REFLECT option to go though raster texturepaint path.
>>>>
>>>> Regards
>>>> Prasanta
>>>>> That probably is not an issue for TexturePaint ..
>>>>>
>>>>> -phil
>>>>>
>>>>> On 10/20/16, 9:40 PM, Prasanta Sadhukhan wrote:
>>>>>> No, this webrev supports LinearGradient/RadialGradient for all 3 
>>>>>> options. The fix seems to be almost same as in TexturePaint but 
>>>>>> in a different else block in OSXSurfaceData.java.
>>>>>>
>>>>>> Regards
>>>>>> Prasanta
>>>>>> On 10/20/2016 9:31 PM, Phil Race wrote:
>>>>>>> Leaving aside the tests, the webrev here looks to be the one 
>>>>>>> proposed
>>>>>>> for the TexturePaint bug. Did you get things mixed up ?
>>>>>>>
>>>>>>> -phil.
>>>>>>>
>>>>>>> On 10/20/2016 02:22 AM, Prasanta Sadhukhan wrote:
>>>>>>>> Hi Phil,
>>>>>>>>
>>>>>>>> I have modified the webrev to support REFLECT and REPEAT albeit 
>>>>>>>> via raster path and not via Quartz api, as Quartz cannot handle 
>>>>>>>> REPEAT/REFLECT.
>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8162796/webrev.01/
>>>>>>>>
>>>>>>>> Have tested with different fractions, stops, color.
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Prasanta
>>>>>>>> On 9/28/2016 2:06 AM, Philip Race wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> LinearGradientPaint has three options as to how it is extended 
>>>>>>>>> beyond the end point
>>>>>>>>>
>>>>>>>>> https://docs.oracle.com/javase/8/docs/api/java/awt/MultipleGradientPaint.CycleMethod.html 
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> So far as I can see this code (and perhaps quartz) can only 
>>>>>>>>> handle NO_CYCLE ..
>>>>>>>>>
>>>>>>>>> If REFLECT or REPEAT are specified and Quartz can't handle it 
>>>>>>>>> we will need to
>>>>>>>>> fall back to the raster path.
>>>>>>>>>
>>>>>>>>> I think it would be helpful to extend the test with examples 
>>>>>>>>> that use these and
>>>>>>>>> also more than just the two colours .. add a variant that 
>>>>>>>>> tries more like 4 ...
>>>>>>>>>
>>>>>>>>> -phil.
>>>>>>>>>
>>>>>>>>> On 9/22/16, 2:03 AM, Prasanta Sadhukhan wrote:
>>>>>>>>>> Hi All,
>>>>>>>>>>
>>>>>>>>>> Please review a fix for jdk9 where it is seen that 
>>>>>>>>>> LinearGradientPaint is not getting printed on osx.
>>>>>>>>>> The below webrev is only for LinearGradientPaint
>>>>>>>>>> and RadialGradientPaint will be handled separately.
>>>>>>>>>>
>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8162796
>>>>>>>>>> webrev: 
>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8162796/webrev.00/
>>>>>>>>>>
>>>>>>>>>> Issue was LinearGradientPaint is not handled in osx from jdk7 
>>>>>>>>>> onwards. It handles simple GradientPaint.
>>>>>>>>>> Code is added as per
>>>>>>>>>> https://developer.apple.com/library/content/documentation/GraphicsImaging/Conceptual/drawingwithquartz2d/dq_shadings/dq_shadings.html#//apple_ref/doc/uid/TP30001066-CH207-TPXREF101 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> to use "CGGradientRef" quartz datatype to support 
>>>>>>>>>> LinearGradientPaint.
>>>>>>>>>> CGGradientRef is used compared to CGShadingRef because it is 
>>>>>>>>>> mentioned in CGGradientRef,  i is "Easy to define more than 
>>>>>>>>>> two locations and colors." as we set 2 or more colors and 2 
>>>>>>>>>> or more fractions in LinearGradientPaint.
>>>>>>>>>>
>>>>>>>>>> Regards
>>>>>>>>>> Prasanta
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>
>