[OpenJDK 2D-Dev] [9] Review Request: 7188942 Remove support of pbuffers in OGL Java2d pipeline

[Jim Graham]

Hi Sergey,

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:

if (BITMASK) { return null }
if (type = FBO) {
     if (!CAPS_EXT_FBOBJECT) return null;
} else if (type = TEXTURE) {
} else {
     return null;
}

which is a list of positive tests for what can be done rather than a 
parallel list of tests - one set of tests negative and the other positive.

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?

CGLSD.m, line 114: The comment still refers to "pbuffer" in the new 
version.  You changed one reference to WINDOW, but not the other reference.

GLXGC.java - same comment as CGLGC.java

GLXVSM.java - same comment as CGLVSM.java

GLXSD.c - In DestroyOGLSurface() - should we leave the Traceln in even 
though the method is empty?

WGLGC.java - same comment as CGLGC.java

WGLVSM.java - same comment as CGLVSM.java

WGLSD.c - In DestroyOGLSurface() - should we leave the Traceln in even 
though the method is empty?

			...jim

On 6/16/15 6:01 AM, Sergey Bylokhov wrote:
> Hello.
> Please review the fix for jdk9.
>
> The reason of the removing is a general deprecation of pbuffers, and a
> lack of their support in the java2d, because for a long time pbuffers
> were not used by default.
> Attempts to disable FBO(which are used by default), and enable the
> pbufferes will cause a different crashes in different places on all our
> platforms(mac,lin,win).
>
> Change description:
>   - Possibility to create a VolatileImage on top of pbuffers was
> removed. FBO and Textures are supported only.
>   - "-Dsun.java2d.opengl.fbobject" now totally disable accelerated
> volatile images(BufSurfaces will be used instead). But it does not
> affect manageable images.
>   - CAPS_STORED_ALPHA is removed because it was used to check is pbuffer
> is able to store alpha or not.
>
> Note that pbuffers are still used to get some information from the
> opengl on the start. I plan to replace these usage later.
>
> Some related questions:
>   - 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.
>
>
> 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.
>