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

Reorganise MapLibre Qt bindings into multiple libraries #37

Merged
merged 6 commits into from
Oct 14, 2023

Conversation

ntadej
Copy link
Collaborator

@ntadej ntadej commented Oct 8, 2023

Reorganise Qt bindings into multiple libraries:

  • QMapLibre: core library
  • QMapLibreWidgets: widgets (currently OpenGL widget)
  • QMapLibreLocation: Qt Location bindings (with 2 plugins)

For now this builds on Linux and macOS. Will follow-up on other platforms later. Note that for Qt5 only desktop support is planned at this point (mainly for Windows).

API for newly added stuff is still rough, but I want a setup that builds nicely and passes tests!

Note that this PR is large, but I split it into more logical commits. Will try to merge without squashing.

Tagging also @kebekus @kamilhajdukiewicz @petricf @emericg

@emericg
Copy link

emericg commented Oct 9, 2023

Trying to build your branch on ArchLinux, Qt 6.5.3 and Qt 6.6.0, it fails while building maplibre-native. Error log is huge, let me know if you need everything.

FAILED: src/core/MapLibreCore/CMakeFiles/mbgl-core.dir/src/mbgl/gfx/shader_registry.cpp.o
/maplibre-native-ntadej/vendor/maplibre-native/src/mbgl/gfx/shader_registry.cpp: Dans le constructeur « mbgl::gfx::ShaderRegistry::ShaderRegistry() »:
/maplibre-native-ntadej/vendor/maplibre-native/src/mbgl/gfx/shader_registry.cpp:7:32: erreur: utilisation de la fonction supprimée « std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::unordered_map() [avec _Key = std::__cxx11::basic_string; _Tp = std::shared_ptrmbgl::gfx::Shader; _Hash = std::hash<std::__cxx11::basic_string >; _Pred = std::equal_to<std::__cxx11::basic_string >; _Alloc = std::allocator<std::pair<const std::__cxx11::basic_string, std::shared_ptrmbgl::gfx::Shader > >] »
7 | ShaderRegistry::ShaderRegistry() {}
/usr/include/c++/13.2.1/bits/unordered_map.h:148:7: note: « std::unordered_map<_Key, _Tp, _Hash, _Pred, _Alloc>::unordered_map() [avec _Key = std::__cxx11::basic_string; _Tp = std::shared_ptrmbgl::gfx::Shader; _Hash = std::hash<std::__cxx11::basic_string >; _Pred = std::equal_to<std::__cxx11::basic_string >; _Alloc = std::allocator<std::pair<const std::__cxx11::basic_string, std::shared_ptrmbgl::gfx::Shader > >] » est implicitement supprimé car la définition par défaut serait mal formée:
148 | unordered_map() = default;
| ^~~~~~~~~~~~~
/maplibre-native-ntadej/vendor/maplibre-native/src/mbgl/gfx/shader_registry.cpp:48:21: requis depuis ici
/usr/include/c++/13.2.1/type_traits:1048:52: erreur: l'assertion statique a échoué: template argument must be a complete class or an unbounded array
1048 | static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~

@ntadej
Copy link
Collaborator Author

ntadej commented Oct 9, 2023

I guess the compiler is too new. Will look into it.

@louwers
Copy link
Collaborator

louwers commented Oct 9, 2023

Looks unrelated to Qt to be honest.

@birkskyum
Copy link
Member

@emericg , Qt 6.6? afaik it isn't released yet - are you using the RC?

@emericg
Copy link

emericg commented Oct 9, 2023

@emericg , Qt 6.6? afaik it isn't released yet - are you using the RC?

Yes indeed, but as @louwers said I don't think it's related to Qt either.

I'm trying every beta now, and I have a lot of versions installed in parallel, because for every release of Qt6 (including point release, but these don't have beta unfortunately) it has been way (waay) too easy to find huge and breaking bugs in the most basic workflows. The sooner these can be reported the better...

@ntadej ntadej force-pushed the next-pr branch 11 times, most recently from c2c12a1 to a7f73b9 Compare October 10, 2023 21:55
@ntadej
Copy link
Collaborator Author

ntadej commented Oct 10, 2023

@emericg, you can try again, the code should now build with GCC 13.

@emericg
Copy link

emericg commented Oct 11, 2023

I've got a failure when building with Qt 6.6 (out now ^^) but that's easy to "fix" and looking at the comments you are aware of what's going on there. And it builds alright with Qt 6.5.

/maplibre-native-ntadej/src/location/stylechange.cpp:60:39: erreur: « calculatePeripheralPoints » n'est pas un membre de « QDeclarativeCircleMapItemPrivate »

But I've also got linking failure, with both Qt versions, seems to be because of ICU.

/usr/bin/ld : src/core/libQMapLibre.so.3.0.0 : référence indéfinie vers « ubidi_close_73 »
/usr/bin/ld : src/core/libQMapLibre.so.3.0.0 : référence indéfinie vers « ubidi_open_73 »

My system has ICU 73.2 and the build system try to use it, but looks like there is a copy of ICU 71 in the maplibre vendor directory, so I've enabled it and it seems to finish the build.
I found this page after this message, now I feel like I really should read the manuals :/ But still both options (icu and the install prefix) should probably be on this plugin README.

And now it loads! With both Qt 6.5 and 6.6. At first, it was... orange. The project I tried initially was loading zoomed and locked on my city, so I thought it wasn't actually working. But the default map is indeed orange here.

I've tried this from an open issue, it's not doing anything. The supported map type is stuck on QGeoMapType(1, maplibre://maps/style, Basic, false, false, , )

PluginParameter { name: "maplibre.mapping.additional_style_urls"; value: "https://tiles.versatiles.org/styles/colorful.json" }
PluginParameter { name: "maplibre.api_base_url"; value: "https://tiles.versatiles.org:8080/api" }

I've tried other things, but long story short, I'm not sure if it is a bug or if I have absolutely no idea on how to set another map style or provider. It's my first time using maplibre, but still there should be an example or plugin parameters documentation on how to change the style?

@ntadej
Copy link
Collaborator Author

ntadej commented Oct 11, 2023

Yes, I need to add all the compile options here.

I also plan to provide some documentation, although I have problems with QDoc as it does not really look like being suited for external Qt projects (or maybe I'm doing something wrong there).

As for changing the style, only "maplibre.mapping.additional_style_urls" is needed, but then you also need to choose the style in QML Map object (something like activeMapType: supportedMapTypes[someIndex]).

@emericg
Copy link

emericg commented Oct 11, 2023

Yes, I need to add all the compile options here.

I also plan to provide some documentation, although I have problems with QDoc as it does not really look like being suited for external Qt projects (or maybe I'm doing something wrong there).

Glad to hear that.

As for changing the style, only "maplibre.mapping.additional_style_urls" is needed, but then you also need to choose the style in QML Map object (something like activeMapType: supportedMapTypes[someIndex]).

Yes I can already switch styles with the osm plugin that way, but with the maplibre plugin like I said the supported map type is stuck on QGeoMapType(1, maplibre://maps/style, Basic, false, false, , ) whatever I try.

@ntadej
Copy link
Collaborator Author

ntadej commented Oct 11, 2023

OK, can you maybe open an issue for the map switch? I'll work on it next after this is merged also adding some examples.

@petricf
Copy link

petricf commented Oct 11, 2023

Built on Debian 12 (linux amd64).

Compiles well (with some warnings). At the end i got
/usr/bin/ld: warning: libicuuc.so.56, needed by /home/petric/Qt/6.5.2/gcc_64/lib/libQt6Core.so.6.5.2, may conflict with libicuuc.so.72

Reason: Debian delivers v72 whereas Qt online installer provides v56. I assume i can ignore this warning if i use LD_LIBRARY_PATH to override the location.

Linkage option "-Wl,rpath,..." may help (untested) on Linux platform, others unknown.

Functional test is ongoing.

Environment:
OS Debian Linux 12.2 (Bookworm)
gcc version 12.2.0 (Debian 12.2.0-14)
GNU ld (GNU Binutils for Debian) 2.40
QMake version 3.1
ninja 1.11.1
Qt 6.5.2 (by Qt online installer placed to directory /home/petric/Qt/6.5.2/...)

Debian stable does not deliver Qt6Location. But qt6positioning(-dev)/libqt6positioning is part of stable debian. Is that enough to compile/use this plugin ?

If yes i will try to use stock debian Qt6 to build it. Advantage: the *.deb can be installed w/o additional Qt libs from outside debian. It perhaps solves the icu problem also.

@emericg
Copy link

emericg commented Oct 11, 2023

OK, can you maybe open an issue for the map switch? I'll work on it next after this is merged also adding some examples.

No problem, I just opened an issue. And thank you for working on this plugin, this is probably the only viable future for QtLocation...

A while ago I was using the mapbox plugin for Qt5, with great results. But even in "the end" of Qt 5, support was starting to show some serious signs of weaknesses.
I only discovered MapLibre last month, after realizing the osm plugin (now the only plugin) for Qt Location 6 wasn't even able to correctly display a simple (and poor looking) map anymore.

Also it would be helpful to quickly recap what this plugin can do in the README, in relation to the larger MapLibre ecosystem. Does the plugin only support tiles? What about vector maps? The renderer is OpenGL or software? Or can do both? Will the metal implementation be a part of the plugin eventually? Will the terrain and globe view ever be supported?

@emericg
Copy link

emericg commented Oct 11, 2023

Debian stable does not deliver Qt6Location. But qt6positioning(-dev)/libqt6positioning is part of stable debian. Is that enough to compile/use this plugin ?

@petricf

QtPositioning is only to get your position from various sources like GPS on mobile or wifi on desktop.
QtLocation is the mapping infrastructure. You can use QtPositioning to draw an icon on the map to visualize your position, but the two modules are independent.

@petricf
Copy link

petricf commented Oct 11, 2023

Thanks for clarifying.

Qt Maintenance Tool starts providing QtLocation from 6.5 onwards as technical preview. I assume Debian as result does not deliver it because here the Qt version in use is 6.4.x.

Standard Qt on Debian is 5.15.8. Here they provide a *.debs for QtLocation.

@ntadej
Copy link
Collaborator Author

ntadej commented Oct 11, 2023

Thanks for clarifying.

Qt Maintenance Tool starts providing QtLocation from 6.5 onwards as technical preview. I assume Debian as result does not deliver it because here the Qt version in use is 6.4.x.

Standard Qt on Debian is 5.15.8. Here they provide a *.debs for QtLocation.

For 6.4 I provide a patched version of QtLocation that you could use (https://github.com/ntadej/qtlocation), but I'm not sure we should support that moving forward, depending on the need of the community.

@petricf
Copy link

petricf commented Oct 11, 2023

As i have seen that ntadej made Qt6Location releases under https://github.com/ntadej/qtlocation.

I try to repackage the release (6.4.2) as binary *.deb. Hopefully this would enable to release a maplibre-qt package for stable Debian. As long i get it compiling - see issue #33.

@ntadej ntadej merged commit df4582b into maplibre:main Oct 14, 2023
7 checks passed
@ntadej ntadej deleted the next-pr branch October 14, 2023 14:05
@ntadej ntadej added this to the 3.0 milestone Oct 11, 2024
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

Successfully merging this pull request may close these issues.

5 participants