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 dependency ? #1

Closed
rouault opened this issue Apr 10, 2018 · 12 comments · Fixed by #2
Closed

QT dependency ? #1

rouault opened this issue Apr 10, 2018 · 12 comments · Fixed by #2
Assignees
Labels
question Further information is requested

Comments

@rouault
Copy link
Contributor

rouault commented Apr 10, 2018

I'm aware the current code is just a stub, but it uses QT. Is it really needed ? Plain C++11 with STL is not enough ?

@PeterPetrik PeterPetrik added the question Further information is requested label Apr 10, 2018
@wonder-sk
Copy link
Member

We will be porting some existing code from Crayfish [1] that makes use of Qt, so having it as a dependency will help us in the early stages (and we only want to make use of QtCore which has modest dependencies [2]). As we progress we would like to move away from Qt if possible.

[1] https://github.com/lutraconsulting/qgis-crayfish-plugin/tree/master/corelib

[2] https://packages.ubuntu.com/artful/libqt5core5a

@hobu
Copy link

hobu commented Apr 10, 2018

I am also 👎 on the QT dependency. The fewer dependencies the better, and Plain C++11 with STL is the preferable platform in my opinion.

@wonder-sk
Copy link
Member

I totally agree that fewer dependencies = better. But I would also like to understand what is the problem that people see with the usage of QtCore library. From my point of view, it is available on all platforms I can think of, it has permissive license (LGPL) and very convenient API.

Stuff like unicode string handling still seems to be a pain with plain c++11: https://stackoverflow.com/questions/17103925/how-well-is-unicode-supported-in-c11

@hobu
Copy link

hobu commented Apr 10, 2018

IMO, a core functionality library like GDAL, PDAL, and MDAL needs to have few "application-level" dependencies to ease its porting across platforms and attempt to limit the layers of complexity (which it already adds one itself). Using something like QtCore means that a developer must also learn that to be productive with the library, which adds burden. The more standard and boring the better.

@wonder-sk
Copy link
Member

I would not call QtCore an "application-level" dependency - it adds missing features to C++ just like Boost libs do. If we didn't want to use QtCore then we could opt for Boost instead, but the same concerns apply there (another layer of complexity + need to learn it). I don't think we can live with just plain C++ and STL.

From the practical point of view... if a library comes with a solid C and Python API, and depends on a widely supported multi-platform library, is the fact that QtCore is used under the hood going to deter anyone from using it?

@hobu
Copy link

hobu commented Apr 10, 2018

Both PDAL and libLAS were originally built on Boost to get the kind of functionality you desire from QT. PDAL still has a couple of embedded Boost libraries (mostly just filesystem, which will be available in C++17). Over time, we migrated PDAL away from Boost as C++11 support matured. The Boost dependency was a hurdle as you've maybe experienced, and it caused potential users to shy away from PDAL because it is imposing and poorly managed from a release standpoint.

As for QT, do you expect to have QT objects in the public API? This means every application that uses your library must also use QT. Many will chose to do their own thing rather than use your tool if it has a dependency they don't like. We saw this behavior with libLAS, which is a simple enough library that people can go write their own thing.

What specific functionality other than "UTF support" does QtCore provide you that is a hard requirement?

@wonder-sk
Copy link
Member

As for QT, do you expect to have QT objects in the public API?

No - Qt objects would not appear in public API.

What specific functionality other than "UTF support" does QtCore provide you that is a hard requirement?

Initially mainly filesystem access (c++17 only) and unicode strings. Later possibly also file watching, JSON support, threading/synchronization primitives, regular expressions, text file I/O, date/time handling/parsing.

I know c++11 has support for several of the items above. Don't get me wrong - I would love to have the whole library in just c++ and STL without any extra libraries. I am also trying to be practical - if there is already code several drivers for formats (developed over a couple of years) that make use of QtCore, we can either use them as they are (introducing QtCore as an internal dependency) or we can discard them (and wait until someone will have time/funding to port those). In theory we could have a configuration option to build QtCore-free MDAL with fewer drivers.

@hobu
Copy link

hobu commented Apr 10, 2018

If QT is available, contributors will pull more and more of it in because of its convenience. PDAL does all of the things you've listed with a bit of its own utility API, embedding some libraries (like nanoflann, jsoncpp, and boost bits), and plain old C++11. In exchange, PDAL's users know who to blame 😄 when something blows up, and our hard deployment requirements are limited. I agree it would be easier and faster if we depended upon all of Boost or all of QtCore, but I'm skeptical we (or GDAL) would have the uptake if we did.

In theory we could have a configuration option to build QtCore-free MDAL with fewer drivers.

Seems like a lot of extra work.

@vpicavet
Copy link

Please please please favor medium/long-term code quality over speed of development !

I 100% agree to what @hobu said. And we even ( no pun intended here) should integrate MDAL into GDAL's framework : it would be better, lower level, more versatile, attract more developers, facilitate ports over multiple architectures, ease development.

As for Oslandia, not sure if we would like to contribute to this if starting conditions are not done right.

@hobu
Copy link

hobu commented Apr 10, 2018

MDAL into GDAL's framework

I don't think so, just as I think PDAL's capability shouldn't be embedded in GDAL either. They are separate problem spaces, and there is a lot of value in each being able to mature it its own pace.

@vmora
Copy link

vmora commented Apr 10, 2018

Not sure about the separate problem spaces. I really see mesh facets as concepts already present in gdal, not sure it's the same with point clouds.

A dependency on GDAL (or beeing part of GDAL) would also solve some of the missing functionalities in c++11.

@PeterPetrik PeterPetrik mentioned this issue Apr 12, 2018
Merged
PeterPetrik added a commit that referenced this issue Apr 12, 2018
Remove QT dependency (fixes #1), copied simple 2dm support
@PeterPetrik
Copy link
Contributor

Decided to remove QT dependency for now. Closing the issue

This was referenced Apr 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants