Hello,
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8069348/webrev.02/
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:
http://hg.openjdk.java.net/macosx-port/macosx-port/jdk/rev/ee9c8c9b5c2e
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
Updated.
OSXSD.java, line 1097 - typos: heavyweight, clipped
Updated.
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.
Thanks,
Alexandr.
...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.
