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

qmlRegisterType<T> under MSVC goes nuts #7

Closed
vltr opened this issue Jun 27, 2016 · 10 comments
Closed

qmlRegisterType<T> under MSVC goes nuts #7

vltr opened this issue Jun 27, 2016 · 10 comments

Comments

@vltr
Copy link
Contributor

vltr commented Jun 27, 2016

Hi! First, I must say that this is a great project and I'm really interested to work with. The only problem I found so far is ... Windows. Under Linux? Smooth. Under MSVC? Things got bit crazy.

Trying to compile the photoalbum example, using Qt 5.6 or Qt 5.7, MSVC 2013u5 x86, everything went fine, as expected. But not so fine when I hit run. The stacktrace goes to QFQmlTypes, where the QML types are registered. Debugging a little (I'm no Qt expert or CPP guru), things blows up in Qt macro QML_GETTYPENAMES, from qqml.h, something like this:

#define QML_GETTYPENAMES \
    const char *className = T::staticMetaObject.className(); \
    const int nameLen = int(strlen(className)); \
    [more code]

Well, thing is that exploring what I can read in memory, it looks like the className is returning what looks like memory garbage to me: \203Ä\010. I came to this conclusion because nameLen is even more odd: 18073424.

Of course, before opening this issue I went looking around for this, this, this, this, docs, docs and this example.

The documented example is from Qt 5.7. From travis, I see that only Qt 5.3 is used to test. I presume this is not relevant, since it worked on my linux using Qt 5.7 without a glitch but, closely looking into docs and answers, there are some differences to QuickFlux. I don't know if I'm in the right direction, but I expect to help. If there's any other information I could give you, please ask! 😄

Thanks again for this great lib!

My best regards,
Richard.

@vltr vltr changed the title qmlRegisterType<T> under MSVC goes nuts qmlRegisterType<T> under MSVC goes nuts Jun 27, 2016
@vltr
Copy link
Contributor Author

vltr commented Jun 28, 2016

Alright, quick update: I created two more scenarios to test this:

  1. Qt 5.3.2 (as seen from Travis configuration) and MSVC 2013u5 x86: the same error. F* ...
  2. Qt 5.7.0 and MinGW 5.3.0 x86: worked like a charm just out of the box.

Oh the internals of MSVC ... I'll keep digging on this and keep this issue updated.

Thanks a lot again!

PS: the F* word is for MSVC 😄

@benlau
Copy link
Owner

benlau commented Jun 28, 2016

hi @vltr ,

Thanks for reporting the issue. I could reproduce the problem in Qt 5.7 + MSVC environment (Setup the environment is very time consuming..)

Just a quick glance, the problem is qmlRegisterType is called before the construction of QGuiApplication. If I run those calls after the construction of QGuiApplication, that it won't crash.

I am still finding a way to solve it. The simplest way is to export a function to let window user to register qml types by themself. But that is what I want to avoid.

@vltr
Copy link
Contributor Author

vltr commented Jun 28, 2016

Thanks @benlau ! You bet the environment setup is time consuming, in fact I created a repository to tell-tale my adventures under Windows / MSVC. I was suspecting that some null pointer were being "initialized" (or called) in this situation. The order of it never came to my mind! I did called it in a function after QGuiApplication, but I didn't quite get it at the time (I was hammering in my head: "Why just in MSVC this doesn't works? Is it Qt?").

Well, of course it is not intended to the application to behave like that (registering before QGuiApplication), but perhaps a safety of some kind, even a macro and a one time check could be useful to prevent this. I know you want to avoid this, but several APIs comes with some kind of checking on runtime. SQLite as an example: you may want it to be faster and compromise your data by sending the journal to memory or if you must at all costs have integrity over performance, then make your journal synchronous and written to the disk. What I mean is: why not let this as an option for the user?

I can make a quick example in the API if you're open to suggestions 😃

My best regards,
Richard.

@vltr
Copy link
Contributor Author

vltr commented Jun 28, 2016

Hello again! I've been reading Qt's documentation about QML types, how to register and everything else. I personally think that you should keep it simple, so it could work everywhere. Yes, Qt says that the registration needs to be invoked, which I read as "the coder needs to tell Qt that, in the Qt way" - not a quick hack that have no default behaviour independently the architecture, OS or compiler. Or, yes, you can "automagically" instantiate custom QML types as plugins, because that's why the plugin system in Qt was created for ("automagically" loading things, not just QML types).

Of couse, one could always try a hack here or there to make it work, but that explains why the cpp developer will not read any docs on how to use a lib if such a hack is not available? Please don't get me wrong, I think this is me arguing with myself 😄

I'm sorry if any of this somehow does not pleases you, it is not my intention.

My best regards,
Richard.

@vltr
Copy link
Contributor Author

vltr commented Jun 28, 2016

Ok, so, after sweeping Qt's 5.7.0 source code on QML types, the solution came much simpler than I thought - besides that all QML types are either exported as a plugin (or created in QQmlEngine constructor), so I'm still a bit resilient by using this solution (after all, it's not documented to work with QML specifically and I didn't found any usage on Qt's source code indicating this), but it works fine on Windows and Linux (if someone could test is OSX ...). Perhaps some Qt core hacker could give a word about it?

Well, basically, file qfqmltypes.cpp becomes this:

/// QML Type Registration Helper
#include <QtQml>
#include "qfappdispatcher.h"
#include "qfapplistener.h"
#include "qfappscript.h"
#include "qfapplistenergroup.h"
#include "qfappscriptgroup.h"
#include "priv/qfappscriptrunnable.h"
#include "qffilter.h"
#include "qfkeytable.h"
#include "qfactioncreator.h"

static QObject *appDispatcherProvider(QQmlEngine *engine, QJSEngine *scriptEngine) {
    Q_UNUSED(engine);
    Q_UNUSED(scriptEngine);

    QFAppDispatcher* object = new QFAppDispatcher();
    object->setEngine(engine);

    return object;
}

static void registerTypes()
{
    qmlRegisterSingletonType<QFAppDispatcher>("QuickFlux", 1, 0, "AppDispatcher", appDispatcherProvider);
    qmlRegisterType<QFAppListener>("QuickFlux", 1, 0, "AppListener");
    qmlRegisterType<QFAppScript>("QuickFlux", 1, 0, "AppScript");
    qmlRegisterType<QFAppListenerGroup>("QuickFlux", 1, 0, "AppListenerGroup");
    qmlRegisterType<QFAppScriptGroup>("QuickFlux", 1, 0, "AppScriptGroup");
    qmlRegisterType<QFAppScriptRunnable>();
    qmlRegisterType<QFFilter>("QuickFlux", 1, 0, "Filter");
    qmlRegisterType<QFKeyTable>("QuickFlux", 1, 0, "KeyTable");
    qmlRegisterType<QFActionCreator>("QuickFlux", 1, 0, "ActionCreator");
}

Q_COREAPP_STARTUP_FUNCTION(registerTypes)

I hope this solve this issue the way you planned to 😉

My best regards,
Richard.

@benlau
Copy link
Owner

benlau commented Jun 29, 2016

Hi Richard,

It is awesome!! Would you like to contribute a patch?

@vltr
Copy link
Contributor Author

vltr commented Jun 29, 2016

@benlau it would be my pleasure! I'll run the test units to make sure it's everything working before making a pull request 😉 thanks a lot!!

Best regards,
Richard.

@vltr
Copy link
Contributor Author

vltr commented Jun 29, 2016

Tested (quickfluxunittests) OK on:

  • Linux:
    • Qt 5.7.0 (from repository (Arch Linux) and another version I compiled);
    • Qt 5.6.1;
  • Windows:
    • Qt 5.3.2 MSVC 2013 x86;
    • Qt 5.6.1 MSVC 2013 x86;
    • Qt 5.7.0 MSVC 2013 x86;
    • Qt 5.7.0 MinGW 530 x86.

I couldn't test on Qt 5.3.2 in my Linux because I'm already using GCC 6.1 and all compatibility flags were put in place but no success on compiling Qt. Well, judging 5.3.2 passed on Windows I think it will pass on Linux too ...

I'm sending the pull request right away! 😄

Best regards,
Richard.

@benlau benlau closed this as completed in 481b216 Jun 29, 2016
benlau added a commit that referenced this issue Jun 29, 2016
@benlau
Copy link
Owner

benlau commented Jun 29, 2016

You already did more than I expect. Excellent. thx!

@vltr
Copy link
Contributor Author

vltr commented Jun 29, 2016

I was just checking if this fix is acceptable to the tests and other versions of Qt :)

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

No branches or pull requests

2 participants