[OpenJDK 2D-Dev] <AWT Dev> [9] Review Request: JDK-8029455 JLightweightFrame: support scaled painting

Anton V. Tarasov anton.tarasov at oracle.com
Thu May 22 13:58:57 UTC 2014


On 22.05.2014 15:36, Sergey Bylokhov wrote:
> On 5/22/14 11:47 AM, Anton V. Tarasov wrote:
>> Hi Sergey,
>>
>> On 22.05.2014 1:44, Sergey Bylokhov wrote:
>>> On 5/21/14 10:13 PM, Anthony Petrov wrote:
>>>> Hi Sergey,
>>>>
>>>> The original fix provides some updates and clarifications to the javadoc for the 
>>>> LightweightContent.imageBufferReset() method, but they are missing from your fix. Is this 
>>>> intentional?
>>> Nope. I just missed this update. I looked at this method closely and got a question: do we need 
>>> this scale parameter? Why we cannot use w,h + scanstride here an skip all clarification about 
>>> logical coordinates?
>>
>> Originally, Jim suggested to generalize the API:
>>
>> <<Rather than imply any parameters, I think specifying a very exact set of parameters gives the 
>> most flexibility.  Even if the relationships you characterize above are true, xywh,scan or 
>> off,wh,scan both provide the flexibility to supply the data in those formats without the client 
>> having to guess dimensions or scan size. Any API that specifies an array containing data should 
>> always provide the flexibility of specifying an offset (and x,y is a way of specifying an offset 
>> for rectangular data and using a nio Buffer can implicitly imply an offset based on its position) 
>> and when that data is a rectangle of data then it should also supply independent w,h and scan 
>> strides.  If the offset is always 0, and if the scanstride is always w in the implementation's 
>> that choose the data storage then it may seem like overkill, but it provides the flexibility of 
>> switching to a more sophisticated buffer re-use strategy later without having to track down every 
>> client and update them... >>
>>
>> and so we provide all the coordinates.
> I understand why we need x/y/w/h/scanstride but why we need scale, because our buffer is pixel 
> based anyway? In this case we have to convert w/h/x/y/scanstride from logical to pixels and back.

The reasoning for that if the following. On the client side (SwingNode), during the rendering of the 
image, there's a need to have logical bounds of the image. So, this would require conversion 
(devision)  for what the client would need to know the scale factor for what it would need to ask 
for it, separately. This would bring another code path & dependencies (for instance, b/w SwingNode 
and its prism peer). Currently, there's only one parameter of a method for that purpose.

Thanks,
Anton.

>
>>
>> Thanks,
>> Anton.
>>
>>>
>>>>
>>>> BTW, I've applied your fix and tested it with the latest version of FX changes, and everything 
>>>> works smoothly on my Mac, including the display change listener.
>>>>
>>>> -- 
>>>> best regards,
>>>> Anthony
>>>>
>>>> On 5/21/2014 7:46 PM, Sergey Bylokhov wrote:
>>>>> Hello, Everybody.
>>>>> Please review an updated version of this fix. This is a minimum possible
>>>>> fix. changes in shared code of jdk are minimized and can be enhanced in
>>>>> the future.
>>>>> The fix is covering hdpi support in SwingNode on osx + system look and
>>>>> feel(Aqua).
>>>>>
>>>>> http://cr.openjdk.java.net/~serb/8029455/webrev.01
>>>>>
>>>>> Notes:
>>>>>   - This fix depends from two other fixes: JDK- 8041129 and JDK-8041644.
>>>>> Both are under review on 2d alias.
>>>>>
>>>>> On 5/13/14 9:29 PM, Anthony Petrov wrote:
>>>>>> Hi Jim, Sergey, and Anton,
>>>>>>
>>>>>> I'd like to revive this old thread and finally push this fix, which
>>>>>> has been reviewed and approved on this mailing list back in February.
>>>>>> The only additional change that I want to introduce, is the addition
>>>>>> of default implementations for the
>>>>>> LightweightContent.imageBufferReset() methods. This allows clients of
>>>>>> the API (namely, JavaFX) to run with both the old and the new JDK w/o
>>>>>> any changes or side-effects. Here's the updated webrev:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~anthony/9-2-hiDPISwingNode-8029455.0/
>>>>>>
>>>>>> It literally only adds the default methods and doesn't make any other
>>>>>> changes to the rest of the already reviewed code. I've tested this on
>>>>>> both hiDPI and loDPI displays, with both old and hiDPI-aware JavaFX
>>>>>> builds, and it works fine in all the cases.
>>>>>>
>>>>>> The current plan is to push this fix to JDK 9, and then back-port the
>>>>>> changes to 8u20.
>>>>>>
>>>>>> -- 
>>>>>> best regards,
>>>>>> Anthony
>>>>>>
>>>>>> On 2/21/2014 2:47 AM, Jim Graham wrote:
>>>>>>> Yes, approved.
>>>>>>>
>>>>>>>          ...jim
>>>>>>>
>>>>>>> On 2/17/14 6:09 AM, Anton V. Tarasov wrote:
>>>>>>>> Jim, so this is ready for a push then.
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>> Anton.
>>>>>>>>
>>>>>>>> On 15.02.2014 5:01, Jim Graham wrote:
>>>>>>>>> I don't need to see an update for that.  I didn't read the entire
>>>>>>>>> webrev, but I looked at this one piece of code and if that was the
>>>>>>>>> only thing changed then I think that dealt with the outstanding
>>>>>>>>> issues...
>>>>>>>>>
>>>>>>>>>         ...jim
>>>>>>>>>
>>>>>>>>> On 2/13/14 11:12 PM, Anton V. Tarasov wrote:
>>>>>>>>>> On 14.02.2014 2:52, Jim Graham wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 2/13/14 5:03 AM, Anton V. Tarasov wrote:
>>>>>>>>>>>> Hi Jim,
>>>>>>>>>>>>
>>>>>>>>>>>> Please, look at the update:
>>>>>>>>>>>>
>>>>>>>>>>>> http://cr.openjdk.java.net/~ant/JDK-8029455/webrev.5
>>>>>>>>>>>>
>>>>>>>>>>>> Here I'm correcting the rect after the transform in SG2D:
>>>>>>>>>>>>
>>>>>>>>>>>> 2123         // In case of negative scale transform, reflect the
>>>>>>>>>>>> rect
>>>>>>>>>>>> coords.
>>>>>>>>>>>> 2124         if (w < 0) {
>>>>>>>>>>>> 2125             w *= -1;
>>>>>>>>>>>> 2126             x -= w;
>>>>>>>>>>>> 2127         }
>>>>>>>>>>>> 2128         if (h < 0) {
>>>>>>>>>>>> 2129             h *= -1;
>>>>>>>>>>>> 2130             y -= h;
>>>>>>>>>>>> 2131         }
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> The blit direction (dx, dy) remains transformed. Is this the right
>>>>>>>>>>>> behavior from your perspective?
>>>>>>>>>>>
>>>>>>>>>>> Yes, that looks good.  I wonder if the "w *= -1" results in a
>>>>>>>>>>> multiply
>>>>>>>>>>> byte code whereas "w = -w" would avoid the multiply?
>>>>>>>>>>>
>>>>>>>>>>>             ...jim
>>>>>>>>>>
>>>>>>>>>> Jim,
>>>>>>>>>>
>>>>>>>>>> Yes, this indeed results in different byte code instructions (imult &
>>>>>>>>>> ineg). Just for curiosity I did some measuring which showed
>>>>>>>>>> negatioation
>>>>>>>>>> worked 10% faster :)
>>>>>>>>>> Well, I'll fix it but let me please not send an update...
>>>>>>>>>>
>>>>>>>>>> Thanks!
>>>>>>>>>> Anton.
>>>>>>>>>>
>>>>>>>>
>>>>>
>>>>>
>>>
>>>
>>
>
>




More information about the 2d-dev mailing list