[OpenJDK 2D-Dev] Review request for 8069348 SunGraphics2D.copyArea() does not properly work for scaled graphics in D3D

[Sergey Bylokhov]

On 15/12/15 03:57, Jim Graham wrote:
> Will OSXOffScreenSurfaceData.copyArea work with a scaled context?  It
> calls the drawImage on the original sg2d, but it is using transformed
> coordinates already.  On the other hand, I installed the patch and built
> on my retina MBP and didn't see any errors in SwingSet2 - is that
> because I was using a different SurfaceData by default (CGL?)?

OSXOffScreenSurfaceData and other classes are a part of the old quartz 
based pipeline. These classes were added to the jdk as part of the 
printing support on the osx. As far as I know the printing support was 
copied as is from jdk6 and I am not sure that all of these files are 
used right now.

>
> The rest looks fine...
>
>              ...jim
>
> On 12/11/15 12:09 PM, Alexander Scherbatiy wrote:
>>
>> Hello,
>>
>> Could you review the updated fix:
>> http://cr.openjdk.java.net/~alexsch/8069348/webrev.03
>>
>> On 28/11/15 02:43, Jim Graham wrote:
>>> Hi Alexandr,
>>>
>>> On 11/27/15 2:06 AM, Alexander Scherbatiy wrote:
>>>>> OSXOffscreenSD.java (and all *SD.java), line 481 - should we just make
>>>>> this part of the SD.copyArea contract, that the coordinates are in
>>>>> device space and the SD method should not concern itself with the SG2D
>>>>> transform?
>>>>      I updated the SurfaceData.copyArea() x,y,width, and height
>>>> description.
>>>
>>> And yet most of the implementations still check the transformState.
>>> Why do they do that if they are no longer concerned with transforming
>>> the inputs?
>>    Updated.
>>
>>    XRSurfaceData didn't handled translate scale transform state. I
>> removed it and checked that on Linux scaled internal frames are properly
>> moved and scroll works correctly.
>>    GDIWindowSurfaceData handles only translate state. For scale state
>> support it is necessary also to scale destination coordinates. It is
>> also requires some additional testing. I left it unchanged.
>>>
>>>>> CopyAreaTest.java, line 61 - rounding is not the same operation that
>>>>> SG2D uses, but it works anyway?
>>>
>>> The rounding still isn't the same as SG2D.  Floor() != ceil(v - 0.5).
>>>
>>> On second thought, it's probably best not to worry about the exact
>>> rounding in the test case, but just test 1 pixel inset from the
>>> coordinates that are needed.  In other words, check:
>>>
>>> scale(X + (N+1) * DX) + 1
>>> scale(Y + (N+1) * DY) + 1,
>>> scale(W) - 2
>>> scale(H) - 2
>>>
>>> and go back to just rounding...
>>     Updated.
>>>
>>>>> CopyAreaTest.java, lines 143,144 - why subtract 2DX and 2DY here? Ah,
>>>>> this may mask the error in line 94 above...?
>>>
>>> I notice that it used to check the rectangle at X+(N+1)*DX,
>>> Y+(N+1)*DY, but now it only checks X+N*DX,Y+N*DY. Why not continue to
>>> check the N+1 copy? That should be the location of the destination of
>>> the last copy, right?
>>    I believe my initial assumption was incorrect.
>>    For example, let's take N = 1. The loops below has only one iteration:
>>      -----------
>>          for (int i = 0; i < N; i++) {
>>              g.copyArea(X + i * DX, Y + i * DY, W, H, DX, DY);
>>          }
>>      -----------
>>
>>   Which is just g.copyArea(X, Y, W, H, DX, DY). So the destination
>> rectangle is (X+DX, Y+DY, W, H)
>>   which corresponds to the N = 1.
>>
>>   I also updated the test to check different scaleX and scaleY.
>>
>>   Thanks,
>>   Alexandr.
>>>
>>>             ...jim
>>


-- 
Best regards, Sergey.