Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Qt virtual QObject #419

Open
drieks opened this issue Aug 10, 2020 · 10 comments
Open

Qt virtual QObject #419

drieks opened this issue Aug 10, 2020 · 10 comments

Comments

@drieks
Copy link

drieks commented Aug 10, 2020

Hi,

I'm trying to implement a custom event function in a class extending QApplication.

public native @Cast("bool") boolean eventFilter(QObject watched, QEvent event);

When overriding this function in java, the new implementation is not called.

Also, this function is missing on the java side:

@Virtual protected native void customEvent(QEvent event);

To fix this, I added this function to qt/src/main/java/org/bytedeco/qt/presets/Qt5Core.java:

@Override
protected String[] virtual() {
return new String[]{"QObject"};
}

I'm using mvn install -DskipTests=true -Djavacpp.cppbuild.skip to update the generated files.
This changes will be created in qt/src/gen/java/org/bytedeco/qt/Qt5Core/QObject.java:

-    public native @Cast("bool") boolean event(QEvent event);
-    public native @Cast("bool") boolean eventFilter(QObject watched, QEvent event);
+    @Virtual public native @Cast("bool") boolean event(QEvent event);
+    @Virtual public native @Cast("bool") boolean eventFilter(QObject watched, QEvent event);

+    @Virtual protected native void timerEvent(QTimerEvent event);
+    @Virtual protected native void childEvent(QChildEvent event);
+    @Virtual protected native void customEvent(QEvent event);

But compiling fails:

/(...)/javacpp-presets/qt/target/native/org/bytedeco/qt/linux-x86_64/jniQt5Core.cpp: In function 'jboolean Java_org_bytedeco_qt_Qt5Core_QCoreApplication_event(JNIEnv*, jobject, jobject)':
/(...)/javacpp-presets/qt/target/native/org/bytedeco/qt/linux-x86_64/jniQt5Core.cpp:9555:152: error: 'virtual bool QCoreApplication::event(QEvent*)' is protected within this context
 9555 |         bool rval = (bool)(dynamic_cast<JavaCPP_QCoreApplication*>(ptr) != NULL ? ((JavaCPP_QCoreApplication*)ptr)->super_event(ptr0) : ptr->event(ptr0));
      |                                                                                                                                                        ^
In file included from /(...)/javacpp-presets/qt/target/native/org/bytedeco/qt/linux-x86_64/jniQt5Core.cpp:103:
/(...)/javacpp-presets/qt/cppbuild/linux-x86_64/include/QtCore/qcoreapplication.h:190:10: note: declared protected here
  190 |     bool event(QEvent *) override;
      |          ^~~~~

qobject.h:

public:
  virtual bool event(QEvent *event);

qcoreapplication.h:

protected:
    bool event(QEvent *) override;

Because it is protected in QCoreApplication, it can not be called here (jniQt5Core.cpp):

JNIEXPORT jboolean JNICALL Java_org_bytedeco_qt_Qt5Core_QCoreApplication_event(JNIEnv* env, jobject obj, jobject arg0) {
    QCoreApplication* ptr = (QCoreApplication*)jlong_to_ptr(env->GetLongField(obj, JavaCPP_addressFID));
    if (ptr == NULL) {
        env->ThrowNew(JavaCPP_getClass(env, 6), "This pointer address is NULL.");
        return 0;
    }
    jlong position = env->GetLongField(obj, JavaCPP_positionFID);
    ptr += position;
    QEvent* ptr0 = arg0 == NULL ? NULL : (QEvent*)jlong_to_ptr(env->GetLongField(arg0, JavaCPP_addressFID));
    jlong position0 = arg0 == NULL ? 0 : env->GetLongField(arg0, JavaCPP_positionFID);
    ptr0 += position0;
    jboolean rarg = 0;
    jthrowable exc = NULL;
    try {
        bool rval = (bool)(dynamic_cast<JavaCPP_QCoreApplication*>(ptr) != NULL ? ((JavaCPP_QCoreApplication*)ptr)->super_event(ptr0) : ptr->event(ptr0));
        rarg = (jboolean)rval;
    } catch (...) {
        exc = JavaCPP_handleException(env, 9);
    }

    if (exc != NULL) {
        env->Throw(exc);
    }
    return rarg;
}

In this line: bool rval = (bool)(dynamic_cast<JavaCPP_QCoreApplication*>(ptr) != NULL ? ((JavaCPP_QCoreApplication*)ptr)->super_event(ptr0) : ptr->event(ptr0));,
when ptr is of type QCoreApplication, ptr->event is not valid, because it is protected.

I tried this changes:
In file org.bytedeco.qt.presets.Qt5Core, in function public void map(InfoMap infoMap) I added

.put(new Info("QObject::event").javaNames("someExampleName"))

this will rename the function as expected. But changing it to:

.put(new Info("QCoreApplication::event").javaNames("someExampleName"))

nothing will be changed. Also

.put(new Info("QCoreApplication::event").skip())

and adding "QCoreApplication::event" to protected String[] skip() is not possible.

Maybe changing ptr->event(ptr0) to ((QObject*)ptr)->event(ptr0) will work?

Is there a way to fix this?

@saudet
Copy link
Member

saudet commented Aug 11, 2020

Maybe changing ptr->event(ptr0) to ((QObject*)ptr)->event(ptr0) will work?

Wow, yes, that does work! We'll just need to figure out where to plug this in here:
https://github.com/bytedeco/javacpp/blob/master/src/main/java/org/bytedeco/javacpp/tools/Generator.java
And make sure it doesn't break anything else, but you can leave that to me. :)

@saudet saudet added the bug label Aug 11, 2020
saudet added a commit that referenced this issue Aug 18, 2020
saudet added a commit to bytedeco/javacpp-presets that referenced this issue Aug 18, 2020
@saudet
Copy link
Member

saudet commented Aug 18, 2020

I've fixed this issue and virtualized QObject.
Please give it a try with 1.5.4-SNAPSHOT: http://bytedeco.org/builds/
And thanks for reporting and for investigating!

@drieks
Copy link
Author

drieks commented Aug 18, 2020

Hi @saudet,

thank you for the patch!

I already tried a manually patched version and I found another problem. My Plan is to write a class MyApplication that extends QApplication and override customEvent. main will call QApplication.exec() (It's a static function). exec will call event from C++ on the custom QApplication instance. The Problem here is, that the Pointer to MyApplication is no instance of JavaCPP_QObject, so the dynamic cast is not possible. But when casting to QObject and invoking event there, the same JNI function is called again - stack overflow.

A way to fix this is extending all JavaCPP_* classes by the JavaCPP-Class of the base class:

-class JavaCPP_hidden JavaCPP_QCoreApplication : public QCoreApplication {
+class JavaCPP_hidden JavaCPP_QCoreApplication : public QCoreApplication, public JavaCPP_QObject {

In this case, my MyApplication extending (java-)QApplication will point to JavaCPP_QCoreApplication, but the dynamic cast to JavaCPP_QObject is possible.
This can be implemented by adding @Virtual to all classes having virtual functions (currently, @Virtual is only allowed for functions). When generating the code from the diff, we can check if (java-)QApplication (or one of this super classes) is annotated with @Virtual. Because in this case, it is annotated, we can just add a second base class pointing to the JavCPP version of the superclass (JavaCPP_QObject in the diff).

@saudet
Copy link
Member

saudet commented Aug 19, 2020

If you're subclassing QApplication, then we need to virtualize() that one as well, and it should work fine.

drieks added a commit to drieks/javacpp-presets that referenced this issue Aug 19, 2020
@drieks
Copy link
Author

drieks commented Aug 19, 2020

Hi @saudet ,

I added an example, please have a look. It should just print "customEvent", but it will crash with a stack overflow.

The Event Loop QApplication::exec() will deliver the new Event() to application by invoking event.

Because event is virtual, this JNI function will be called:

JNIEXPORT jboolean JNICALL Java_org_bytedeco_qt_Qt5Core_QCoreApplication_event(JNIEnv* env, jobject obj, jobject arg0) {
    QCoreApplication* ptr = (QCoreApplication*)jlong_to_ptr(env->GetLongField(obj, JavaCPP_addressFID));
    if (ptr == NULL) {
        env->ThrowNew(JavaCPP_getClass(env, 6), "This pointer address is NULL.");
        return 0;
    }
    jlong position = env->GetLongField(obj, JavaCPP_positionFID);
    ptr += position;
    QEvent* ptr0 = arg0 == NULL ? NULL : (QEvent*)jlong_to_ptr(env->GetLongField(arg0, JavaCPP_addressFID));
    jlong position0 = arg0 == NULL ? 0 : env->GetLongField(arg0, JavaCPP_positionFID);
    ptr0 += position0;
    jboolean rarg = 0;
    jthrowable exc = NULL;
    try {
        bool rval = (bool)(dynamic_cast<JavaCPP_QCoreApplication*>(ptr) != NULL ? ((JavaCPP_QCoreApplication*)ptr)->super_event(ptr0) : ((QObject*)ptr)->event(ptr0));
        rarg = (jboolean)rval;
    } catch (...) {
        exc = JavaCPP_handleException(env, 9);
    }

    if (exc != NULL) {
        env->Throw(exc);
    }
    return rarg;
}

Here, QCoreApplication* ptr is a Pointer to a instance of JavaCPP_QCoreApplication (because JavaCPP_QCoreApplication extends QCoreApplication). The dynamic cast to JavaCPP_JObject will not work, beause JavaCPP_QCoreApplication implements QObject, but not JavaCPP_JObject. So the fallback cast of ptr to QObject* will be used, this is valid. But invoking this function will lead in a stack overflow, because it is a pointer to the actually running function.

saudet added a commit that referenced this issue Aug 20, 2020
saudet added a commit to bytedeco/javacpp-presets that referenced this issue Aug 20, 2020
@saudet
Copy link
Member

saudet commented Aug 20, 2020

Actually, there was another issue with JavaCPP not picking up virtual methods specified with override instead of virtual. I've fixed that in the latest commit, and I've also updated the presets for Qt in commit bytedeco/javacpp-presets@0b485f5 to virtualize all subclasses of QObject.

Your ExampleApplication seems to work fine with those changes. Could you confirm and let me know if you encounter any other problems? Thanks!

@drieks
Copy link
Author

drieks commented Aug 20, 2020

Hi @saudet, thank you very much, the virtual stuff is working now!

But there is still another problem. When you set a breakpoint in function customEvent in my example application, you can see, that the event argument is of type QEvent, not of expected type Event. But the address is the same, so the passed pointer seems to be fine. (I updated my example code)

@saudet
Copy link
Member

saudet commented Aug 21, 2020 via email

@saudet
Copy link
Member

saudet commented Aug 22, 2020

The easiest thing to do for now is to use the address as a key to a HashMap, like this:
https://github.com/bytedeco/javacv/blob/master/src/main/java/org/bytedeco/javacv/FFmpegFrameRecorder.java#L311-L327

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants