I'm OK with the removal of the test, but I didn't look in more depth.
Phil already approved from his end, though, so I think you can take this
as a concurrence...
...jim
On 11/30/12 1:37 AM, Andrew Brygin wrote:
Hi Jim,
I have verified that we use either the imageComp field from SunGraphics2D
(which can not be null), or some explicit composite type in this method.
So, according to your suggestion, I have removed the check for null.
Please take a look to updated webrev:
http://cr.openjdk.java.net/~bae/7124347/8/webrev.01/
Phil, could you please take a look the the updated fix also?
Thanks,
Andrew
On 30.11.2012 4:48, Jim Graham wrote:
Hi Andrew,
My belief is that comp should never be null. A simple grep should
confirm that belief. While I do agree that it can be safer to check
for null if you aren't sure about the code base, once one piece of
code does it then it spreads and weakens the API contract so it would
be better to check and keep the API contract stronger if we can...
...jim
On 11/29/12 12:18 AM, Andrew Brygin wrote:
Hello Jim,
I do not think that suggested change allows a null composite per se:
the check for null was introduced just to make sure that we can safely
call isDerivedFrom method. However, it does not change the method
behavior for the case of null composite.
If it is impossible to get null composite here for some reason
(unfortunately
I can not say this for sure), then this check is not required.
However, I do not
think that the extra check for null may cause any harm.
Thanks,
Andrew
On 29.11.2012 4:07, Jim Graham wrote:
Is there a reason for allowing a null comp in the isSupported method?
...jim
On 11/26/2012 4:38 AM, Andrew Brygin wrote:
Hello,
could you please review a fix for 7124347?
This fix does not implement getRater() in ogl surfaces.
Instead, it provides a blit for custom composite, which
prepares a snapshot of the destination surface (which
is one of ogl surfaces) and delegates the work to the
general blit, which now extracts raster from the snapshot.
A result of general blit's work is transferred to the
original destination.
Changes in OGLSurfaceDataProcy are required in order to prevent
getting an accelerated copy of original source image as an
operand of the blit. Now we first check for composite type,
and only then (if composite is not specified, or is a kind
of alpha composite) we take into account other conditions.
So this fix does not change anything for the case of alpha
composites
but affects only XOR and custom composites case.
Note that the fix solves the problem only for the case of blits
(i.e. drawImage()). Other operations may result into the same
problem if custom composite is used. However, in case of XOR,
this change solves the problem completely, because other rendering
operations in the ogl pipeline are ready for XOR composite.
Supplied regression test verifies that the fix makes the problem
gone.
I have verified both windows and macosx platforms.
Please take a look and share your comments.
Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7124347
Webrev: http://cr.openjdk.java.net/~bae/7124347/8/webrev.00/
Thanks,
Andrew
