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

Alexander Scherbatiy alexandr.scherbatiy at oracle.com
Fri Nov 27 10:06:06 UTC 2015


   Could you review the updated fix:

On 11/25/2015 12:30 AM, Jim Graham wrote:
> Hi Alexandr,
> OSXOffScreenSD.java - what are these "rdar:" references?
     These are references to the internal Apple bug system. They have 
been contributed with the Mac OS X port:

> OSXOffscreenSD.java - since this method just refers back to the SG2D, 
> how is it any better than just returning false and letting SG2D's 
> general copyArea deal with the issue?  Also, SG2D.drawImage() is not 
> guaranteed to handle overlapping copyareas so arguably it could fail - 
> but I suppose the assertion is that the drawImage to such destinations 
> happens to deal with overlapping areas?  Is that asserted or tested 
> anywhere?  Also, this method claims that the Windows version ignores 
> the "light clip", but it doesn't.  In fact, if there is a clip then 
> GDIcopyarea punts to the general version.  I find a lot to worry about 
> in this implementation and I wonder how well it was tested...?

     I create a separate issue on it: JDK-8144181 
OSXOffScreenSurfaceData.copyArea() method improvement
> (Note that other than the two issues I point out below with this file, 
> those concerns were not raised by this fix - it looks like a fairly 
> faithful translation of the questionable implementation to the new 
> scheme (ignoring the x/y bug below))
> 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.
> OSXOffscreenSD.java, line 511 - double check your x/y's
> OSXSD.java, line 1097 - typos: heavyweight, clipped
> X11SD.java & XRSD.java - (not an issue with this fix, but a question 
> for future work) needExposures?  Do any of the other copyarea 
> implementations send repaint events?  Shouldn't this be hard-coded to 
> false?  More investigation is likely needed here.
     I moved it to the separate issue:  JDK-8144179 GraphicsExposure 
event generation investigation in XCopyArea
> CopyAreaTest.java, line 52 - typo "BACKGROUND_COLOR"
> CopyAreaTest.java, line 61 - rounding is not the same operation that 
> SG2D uses, but it works anyway?
> CopyAreaTest.java, line 86 - perhaps move the scale to after you fill 
> the background?
> CopyAreaTest.java, line 94 - "Y + i * DX" => DY  (that's odd, how did 
> the test pass?)
> CopyAreaTest.java, lines 143,144 - why subtract 2DX and 2DY here? Ah, 
> this may mask the error in line 94 above...?

     The test is updated according to the comments.

>             ...jim
> On 11/24/15 5:07 AM, Alexander Scherbatiy wrote:
>>   Hello,
>>   Could you review the updated fix:
>> http://cr.openjdk.java.net/~alexsch/8069348/webrev.01/
>>   - The given image sizes are transformed in the SG2D.doCopyArea method.
>> The copyArea() methods are updated accordingly.
>>   - OGLSurfaceData.copyArea() method is updated to process scaled
>> transforms.
>>   - The test which uses copyArea() method for VolatileImage is added. I
>> left the test which moves internal frames for the Swing testing 
>> purposes.
>>   Thanks,
>>   Alexandr.
>> On 11/20/2015 6:59 PM, Jim Graham wrote:
>>> In terms of consolidating the code...
>>> Since SD.copyArea is an internal API, we could simply specify that it
>>> takes pixel coordinates and assumes COPY semantics so it is up to SG2D
>>> to verify the transform and compState and perform appropriate
>>> coordinate transformations before asking the SD to do the dirty work.
>>> I'm not sure why we left so much veto power up to the SD class there...
>>>             ...jim
>>> On 11/20/15 7:38 AM, Sergey Bylokhov wrote:
>>>> On 20.11.15 14:49, Alexander Scherbatiy wrote:
>>>>> Hello,
>>>>> Could you review the fix:
>>>>>    bug: https://bugs.openjdk.java.net/browse/JDK-8069348
>>>>>    webrev: http://cr.openjdk.java.net/~alexsch/8069348/webrev.00
>>>>>    For some reasons Blit operation does not properly work in the
>>>>> SunGraphics2D.copyArea() method
>>>>>    with scaled transform neither on Windows nor on Mac OS X.
>>>>>    I have filled an issue on it: JDK-8143392 SunGraphics2D.copyArea()
>>>>> does not properly handle Blit operation
>>>> Interesting.
>>>>>    The current solution updates D3DSurfaceData.copyArea() to handle
>>>>> scaled graphics in the same way
>>>>>    as it has been already done for the CGLSurfaceData (see 
>>>>> JDK-8000629
>>>>> [macosx] Blurry rendering with Java 7 on Retina display).
>>>> Note that this fix does not help ogl on windows right? because on
>>>> windows the usual ogl blit should be used since the fix for retina was
>>>> osx specific. Or it works properly?
>>>>>    May be there is a way to avoid the code duplication: put it to 
>>>>> some
>>>>> SurfaceDataUtils class (it also
>>>>>    requires adding parent copyArea() method to BufferedRenderPipe) or
>>>>> something else. I am not sure about the best option.
>>>> This is up to the 2d team, but I think yes, it will be better to 
>>>> move it
>>>> somewhere because after the current fix the different pipelines will
>>>> behave differently on different platforms for a different
>>>> transformations =(
>>>>>    Thanks,
>>>>>    Alexandr.

More information about the 2d-dev mailing list