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