Hi Oleg,
That looks good. I am vaguely starting to recall that the exception can
sometimes happen when the native layer wants the upper layer to pick up
on some device changes, but the SD may not actually be in line for a
replacement, so this makes sense...
...jim
On 9/5/2012 11:23 AM, Oleg Pekhovskiy wrote:
Hi Jim,
there are several places (e.g. in D3DRenderer and D3DSurfaceData) where
InvalidPipeException could be thrown regardless the validity of
SurfaceData.
As all SunGraphics2D draw-methods call revalidateAll() on IPE catch,
getReplacement() could be potentially called for the valid SurfaceData.
Anyway, I made changes you proposed:
http://cr.openjdk.java.net/~bagiras/8/7153339.3/
Thanks,
Oleg
9/4/2012 10:50 PM, Jim Graham wrote:
Hi Oleg,
That looks much better. One question - is there ever a case where that
method is called on a peer whose surface data is still valid? (i.e.
won't the first test always cause a replacement to be made?). It's
been a while since I wandered through this code so I don't remember
all of the conditions.
If it is called a bit even for valid SDs then could you refactor it so
that we don't call getSurfaceData() twice in the "already valid" case?
...jim
On 9/4/12 11:32 AM, Oleg Pekhovskiy wrote:
Hi all,
please review the second version of fix for CR:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7153339
Webrev:
http://cr.openjdk.java.net/~bagiras/8/7153339.2/
To avoid influence to the platforms other than Windows and guarantee
SurfaceData.getReplacement() returns valid surface
I made changes in ScreenUpdateManager.getReplacementScreenSurface()
where WComponentPeer.replaceSurfaceData()
is called to make peer's surfaceData valid.
Thanks,
Oleg
8/25/2012 3:07 AM, Jim Graham wrote:
Hi Oleg,
Let me clarify, I'm not referring to a "better way" to fix this.
Actually, the fix you have prepared has a problem. Once the SD is
"invalid due to switching to software" and the SG2D decides to install
a null SD instead, then it will never recover. But, no change to a
drawable should happen (other than it going away) to cause that state.
The SG2D's were designed to survive changes in state to the underlying
drawable, as long as it remains viable as a drawing destination, but
here you are giving up on it when that (externally not really very
obvious) state change occurs. Thread A could be happily rendering to
the surface on the screen, thread B decides to use XOR thereby
changing the state, but thread A's G2D now stops working? Thread A
would be very very confused.
So, while there may be a fix we could add to SG2D to work around the
problem, your fix isn't the right solution. I believe that the right
solution is to have the D3DSurface return the correct new surface, but
it being "what I think is the right solution" is just part of the
issue with your currently proposed fix - the proposed fix violates a
long-standing behavior of G2D objects to remain viable until the
surface is gone...
...jim
On 8/24/2012 3:01 AM, Oleg Pekhovskiy wrote:
Hi Jim,
first of all I should say, that I prepared that fix for 7u as the most
safe, with minimum changes.
I agree that getReplacement() should return a valid sufrace or
null, but
it doesn't happen during that switch.
That CR was assigned to me because my previous changes for:
7112642 Incorrect checking for graphics rendering object
7121482 some sun/java2d and sun/awt tests failed with
InvalidPipeException
discovered the problem with getReplacement() and were somehow related.
But maybe it would be better to create a separate CR for
getReplacement() issue and assign it to Java2d Team?
I also could add a comment for "!surfaceData.isValid()" reffering to
CR7153339.
What do you think?
Thanks,
Oleg
24.08.2012 4:04, Jim Graham wrote:
Hi Oleg,
getReplacement should not be returning an invalid surface. If the
current D3DSD is invalid, then it should return a valid replacement.
The only time it should return null is when the window is gone. It
sounds like the window isn't gone here, it has just switched over
to a
non-D3D type of SurfaceData and return that...
...jim
On 8/23/2012 4:37 PM, Oleg Pekhovskiy wrote:
Hi Phil, Jim,
thank you for pointing out the testing work that should be
performed.
I tested my fix with the following regression tests:
test/java/awt/Graphics
test/java/awt/Graphics2D
test/java/awt/GraphicsDevice
test/java/awt/GraphicsEnvironment
test/sun/java2d
Plus I tested performance differences on:
demo/jfc/Java2D/Java2Demo.jar
Testing was done on Windows 7 & Ubuntu 12.04 LTS.
No differences were found.
I also asked Yuri Nesterenko to test all that stuff on Mac.
Thanks,
Oleg
11.08.2012 3:10, Phil Race wrote:
Oleg,
This looks OK to me but since this is a shared code change I have
to ask what testing you've done ?
Also why not provide a regression test ? The provided test was
interactive but I think it can be automated.
You need another review and I'd like Jim to take a look.
-phil.
On 8/10/2012 2:26 PM, Oleg Pekhovskiy wrote:
Hi,
Please review the fix for CR:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7153339
Webrev:
http://cr.openjdk.java.net/~bagiras/8/7153339.1/
Comments:
XOR is not supported for D3D (see comments inside
D3DSurfaceData.validatePipe()) and
software rendering is used invalidating current D3DSurfaceData.
So we have situation when component's peer has invalid SurfaceData
and it's retrieved in
SunGraphics2D.revalidateAll() through SurfaceData.getReplacement()
without any check.
That's why I added validity check there.
Thanks,
Oleg
<http://cr.openjdk.java.net/%7Ebagiras/8/7153339.1/>
