[OpenJDK 2D-Dev] <AWT Dev> [9] Review Request: JDK-8029455 JLightweightFrame: support scaled painting
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Thu May 22 11:36:47 UTC 2014
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.
>
> 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.
>>>>>>>>>
>>>>>>>
>>>>
>>>>
>>
>>
>
--
Best regards, Sergey.
More information about the 2d-dev
mailing list