Hi, Jim.
Thanks for review!
Thew new version of the fix:
http://cr.openjdk.java.net/~serb/7188942/webrev.02
Comments about pbuffer were changed.
CGLGC.java:
In createCompatVM() I dislike separated tests for "here is early
rejection of the list of things that I can handle" followed by a list
of tests for things it can handle. For one thing we have to test the
type more than once. But mainly it just seems like the two tests can
get out of synch over time. It just seems cleaner to only iterate a
set of allowed types once. Wouldn't it be simpler and more
straightforward to do something like:
The different ifs were merged. I think the code became more readable
after that.
CGLVSM.java:
In the constructor we claim to accelerate any surface if
CAPS_EXT_FBOBJECT is present, but CGLGC.java has a test to reject
BITMASK. Shouldn't the 2 perform the same test?
Actually this is an interesting question related to my:
- Does anybody know about AccelTypedVolatileImage, how it was planned
to use? It seems that the work on it was not completed and its usage can
be simply removed/replaced.
CGLVSM.java constructor is used in the standard codepath, when we create
a VolatileImage. And after the fix in a general case we support an
accelerated VolatileImages only when CAPS_EXT_FBOBJECT is present. On
the other hand CGLGC.java has a special method
createCompatibleVolatileImage(), which creates AccelTypedVolatileImage
only for some cases, and I do not know why(probably for performance
reasons). I do not think that this is an issue, because
createCompatibleVolatileImage() is called only on Windows for shaped
window and RT_PLAIN is used as an argument(so the whole method and
related classes became noop in OGL after the fix).
Bug: https://bugs.openjdk.java.net/browse/JDK-7188942
Webrev can be found at:
http://cr.openjdk.java.net/~serb/7188942/webrev.01
--
Best regards, Sergey.
--
Best regards, Sergey.
