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

[Jim Graham]

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?)?

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
>