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

[Alexandr Scherbatiy]

  Hello,

  Could you review the updated fix:
   http://cr.openjdk.java.net/~alexsch/8069348/webrev.04/

  - TRANSFORM_TRANSLATESCALE check is restored in the 
OSXOffScreenSurfaceData
  - TRANSFORM_TRANSLATESCALE check is removed from X11SurfaceData and 
GDIWindowSurfaceData

   Thanks,
   Alexandr.

On 12/15/2015 11:22 AM, Jim Graham wrote:
> Also, X11SD and GDISD still reject TRANSLATESCALE.
>
> I think it may make sense for OSXOffscreen to return false for 
> TRANSLATESCALE and punt back to the general implementation?  But, X11 
> and GDI should be able to allow that condition...
>
>             ...jim
>
> On 12/14/15 4:57 PM, 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?)?
>>
>> 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
>>>