RFR: 8254569: Remove hard dependency on Dispman in Monocle fb rendering [v2]

Johan Vos jvos at openjdk.java.net
Wed Nov 4 11:12:08 UTC 2020


On Tue, 3 Nov 2020 21:20:49 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Johan Vos has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   process reviewer comments
>
> modules/javafx.graphics/src/main/native-glass/monocle/egl/eglBridge.c line 56:
> 
>> 54: JNIEXPORT jlong JNICALL Java_com_sun_glass_ui_monocle_EGLAcceleratedScreen_nEglChooseConfig
>> 55:     (JNIEnv *env, jclass clazz, jlong eglDisplay, jintArray attribs) {
>> 56:     jint *attrArray = (*env)->GetIntArrayElements(env, attribs, JNI_FALSE);
> 
> Should this check the return value in case it fails (returns NULL)?

I checked other places in the code, and calls to env->GetIntArrayElements are typically not checked.
However, if something fails in that call, it must be something really bad that happened. I added a fprintf to stderr, and throw an IllegalArgumentException in the java code (as there is something wrong with the attribs)

> modules/javafx.graphics/src/main/native-glass/monocle/egl/egl_ext.h line 1:
> 
>> 1: #include <jni.h>
> 
> This needs a standard copyright header.
> 
> Also should it have a guard to prevent including it more than once?

correct. fixed.

> modules/javafx.graphics/src/main/java/com/sun/glass/ui/monocle/EGLAcceleratedScreen.java line 45:
> 
>> 43:      *        implementation to get the best matching configuration.
>> 44:      */
>> 45:     EGLAcceleratedScreen(int[] attributes) throws GLException {
> 
> I presume you intended to call the (newly added) default `super` method and not `super(int[])`?

Yes, this will (implicitly) call the super() method instead of the super(int[] ) method. The problem with the latter (and actually the reason for this approach) is that the flow in the super(int[]) constructor is very specific to specific use-cases, and would require subclasses for every EGL-system that does not behave 100% similar to the specific usecase.

-------------

PR: https://git.openjdk.java.net/jfx/pull/343


More information about the openjfx-dev mailing list