cityjson / cityjson-qgis-plugin

A QGIS plugin that adds support for CityJSON files
Apache License 2.0
35 stars 8 forks source link

The way semantic_colors is saved in Settings breaks QGIS during startup #32

Closed rduivenvoorde closed 3 years ago

rduivenvoorde commented 3 years ago

See my thread on the dev list:

https://lists.osgeo.org/pipermail/qgis-developer/2020-July/061889.html

In short: I get a broken profile which crashes my QGIS, showing:

"Warning: QVariant::load: unknown user type with name PyQt_PyObject."

Looking into the QGIS3.ini I see

[CityJSON%20Loader]
semantic_colors\1\ambient=@Variant(\0\0\0\x7f\0\0\0\xePyQt_PyObject\0\0\0\0\x17\x80\x4\x95\f\0\0\0\0\0\0\0(K\xffK\0K\0K\xfft\x94.)
...

The object written to Settings is:

https://github.com/cityjson/cityjson-qgis-plugin/blob/main/core/settings.py#L6-L32

semantic_colors = {
    "RoofSurface": {
        "diffuse": QColor(255, 0, 0),
        "ambient": QColor(255, 0, 0),
        "specular": None
    },
...

So we could wait if this is a SIP/Python issue or a Qt-version specific one (see if one of the core devs follows up on that thread)

Or we could think about saving the colors in another way in Settings.

liberostelios commented 3 years ago

Thanks for reporting this and I am sorry for the inconvenience.

This seems a bit odd, because I actually convert the colours to integers before I stored them (and then back to QColor when loading them), for instance see:

https://github.com/cityjson/cityjson-qgis-plugin/blob/main/core/settings.py#L58-L60

settings.setValue("diffuse", get_color_int(colors["diffuse"]))
settings.setValue("ambient", get_color_int(colors["ambient"]))
settings.setValue("specular", get_color_int(colors["specular"]))

I wonder what's actually wrong here. Which version of QGIS are you using? Did this occur after an upgrade or is it typical behaviour every time?

rduivenvoorde commented 3 years ago

@liberostelios looking at the code, is it possible that the 'semantic_colors' default/initited object is written to Settings? Even when you did not do anything with the plugin or so?

https://github.com/cityjson/cityjson-qgis-plugin/blob/752175f1c0492017fe136095cf6aafe692eb41c2/core/settings.py#L6

As soon as I create a new profile, install the plugin so it writes some settings, it is done.

As said: I was hoping somebody from the core-devs would answer my question if this should or should not be possible. A quick google: https://doc.qt.io/qt-5/qsettings.html#qvariant-and-gui-types "Because QVariant is part of the Qt Core module, it cannot provide conversion functions to data types such as QColor, QImage, and QPixmap, which are part of Qt GUI. In other words, there is no toColor(), toImage(), or toPixmap() functions in QVariant."

So maybe that is the crux?

FYI: I compile master myself on Debian Testing here (but had the crash with 3.12 and 3.14 too), other versions:

QGIS version 3.15.0-Master QGIS code revision 8b5dedf625
Compiled against Qt 5.14.2 Running against Qt 5.14.2
Compiled against GDAL/OGR 3.1.2 Running against GDAL/OGR 3.1.2
Compiled against GEOS 3.8.1-CAPI-1.13.3 Running against GEOS 3.8.1-CAPI-1.13.3
Compiled against SQLite 3.32.3 Running against SQLite 3.32.3
PostgreSQL Client Version 12.3 (Debian 12.3-1+b1) SpatiaLite Version 4.3.0a
QWT Version 6.1.4 QScintilla2 Version 2.11.2
Compiled against PROJ 7.1.0 Running against PROJ Rel. 7.1.0, August 1st, 2020
OS Version Debian GNU/Linux bullseye/sid This copy of QGIS writes debugging output.
liberostelios commented 3 years ago

@liberostelios looking at the code, is it possible that the 'semantic_colors' default/initited object is written to Settings? Even when you did not do anything with the plugin or so?

I don't think this is possible. semantic_colors is just a dict to holds things in memory. Settings are saved by the save_defaults() function which saves the records one-by-one, converting QColor to int in the process. I can't see how QGIS could take the "initiative" of storing a random dict in its settings.

Nevertheless, since some QObject ends up in settings, there is obviously something I am missing here. What's even worse is that this is all happening for no reason, for now, as there is no settings editor (was planning to implement that, but haven't done so) so this whole inconvenience is practically for nothing.

So, for now I think I'll just disable the whole mechanism to fix the issue and then investigate to figure out what's wrong with it.

liberostelios commented 3 years ago

I think I figured out what's wrong. The conversion wasn't really working (I was calling a function that returns void). In my case (QGIS 3.10) the settings were just "invalid" (which I guess didn't bother QGIS, for some reason), but maybe something changed in later PyQt versions and it stores something like a QObject that breaks the loading of the profile.

Anyway, I've temporarily disabled saving of settings (it's useless anyway, as I said) so you may want to try version 0.6.1.