Zip for compiling mapbox-gl-native with MSVC 2017 (15.6.3)[qt]

[PeterMJ]

I have managed to download the qt-v1.3.0.zip release from your release page and 'port' it to the Microsoft compiler, but I cannot see how the release zip maps to the repository you maintain here? I have 'ported' both the mapbox-gl-native and the external Qt plugin, and some of the 'deps' that had mapbox specific folders I have had to alter slightly, e.g. to point to mbgl/util/optional.hpp rather than the std::experimental that does not exist in MSVC. MSVC 2017 also no longer needs tao_tuple and tao_sequence.

https://github.com/mapbox/mapbox-gl-native/releases/tag/qt-v1.3.0

I have had to make a number of changes, but none I believe should interfere with GCC compilation. I have had to change from using Boost 1.65.1 to Boost 1.66.0, but that should also be a non-issue.

How can I contribute these changes?

I am compiling inside QtCreator 4.5.1, qmake, Qt 5.10.1, MSVC 2017 15.6.3. As a note I have to disable jom because compilation of some of the heavily templated classes consumes 4 GiRAM during compilation.

I have zipped up and attached qt-v1.3.0-msvc2017.zip, so you could commit the changes, I have left boost 1.66.0 out, which I installed locally have at "C:/CppLib/boost_1_66_0" as you will see inside the .pro file.

mapbox-gl-native-qt-v1.3.0-msvc2017.zip

---

As an overview, the changes I made involved changing using directives to typedefs, and using forwarding constructors. I also had to implement some template workarounds to enable template parameter packs to expand with MSVC. MSVC also seems to fail to promote 0 -> float, float -> double during template resolution, and is more picky about some other conversions. I also use boost's optional.hpp not std::experimental::optional, which is controlled from util/mbgl/optional.hpp. There is also a weird type resolution failure I have worked around in the expression value get and is functions...

---

I would love this to be included and MSVC to be mainlined so that I don't have to do this work again for the next release.

Thank you,

Peter Myerscough-Jackopson

Ps. I enjoyed learning and reading the code you have developed, particularly IndexedTuple and your use of it in the PaintPropertyBinders which was the trickiest section of code to port.

[tobrun]

cc @tmpsantos

[tmpsantos]

Hi @PeterMJ,

These Qt releases are stripped down version of a stable version of Mapbox GL Native that we make to use at the Qt Location plugin on Qt upstream. They always map to hash on the master branch and you will find them here https://github.com/mapbox/mapbox-gl-native/commits/qt-staging.

We don't support MSVC compiler as you found out and so far the easiest way to get Mapbox GL linking with a MSVC compiled app was to use Clang-cl like we do on the AppVeyeor bot:

https://ci.appveyor.com/project/Mapbox/mapbox-gl-native/build/1531

I'm very interested on seeing your changes to get native MSVC support and the best way would be make a pull request against out master and tag me.

I appreciate you took your time working on this and thanks for reaching us willing to contribute the changes back.

[PeterMJ]

So are you suggesting I fork https://github.com/mapbox/mapbox-gl-native/commits/qt-staging,
apply my changes and then make a pull request? or are you suggesting I fork https://github.com/mapbox/mapbox-gl-native/ and then try and find all the files? because I have changed some of the files in the deps folder and I could not see them in the master branch.

Or do you want a hybrid of a pull for the files in src from master and a separate pull for the qt-staging branch?

[tmpsantos]

@PeterMJ doing it on qt-staging sounds easier right? I think it is probably better because we can also see the changes on the deps.

[tmpsantos]

/cc @brunoabinader

[PeterMJ]

I have posted a pull request as asked.

[tmpsantos]

@PeterMJ saw it, thank you very much. I will have a 🔍 on it!

[PeterMJ]

Thanks, just wanted to know it had been seen, any questions do feel free to ask, In reviewing the guidelines I see that the use of a constexpr auto EXTENTS = 8192 may be better than my current alteration to cast the constant to uint16 and int16, but I have not checked if MSVC would like that formulation.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

[mauricek]

Is there any update on this? Would be highly appreciated to have MSVC support.

[PeterMJ]

I have since resorted to using the LLVM compiler(7.0) using its msvc compatible mode. If you set it to compile with -std:c++17 as well, and add a default copy move constructor to the Segment class it compiles with ease. The copy constructor is already present in the main mapbox-gl-native library, but wasn't in the Qt 5.12 release.