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

Avoid dependency to gui when not needed #66

Open
mhugo opened this Issue Oct 26, 2016 · 16 comments

Comments

Projects
None yet
7 participants
@mhugo

mhugo commented Oct 26, 2016

Some classes depend on the gui part of qgis when it is not needed.

Identified so far :

  • Data providers: selectWidget() and similar. Proposition: split each data providers into:
    • the core provider module for a layer
    • the core provider that deals with layer management (deletion, creation as empty, etc.)
    • the module with GUI widgets
  • QgsEditorWidgetFactory : all metadata and data access functions of an editor widget should be kept in core only (e.g. QgsEditorWidgetFactory::representValue)
  • QgsProject (see qgis/QGIS-Enhancement-Proposals#76)

Ideally, we should also get rid of dependencies to a QtGui components when not needed, and especially when they need an X server :

  • Abstract the use of QDialogProgress in core: a base class that outputs to stdout or /dev/null for console application. Found in qgsgml.cpp qgsvectorlayer.cpp qgsvectorlayerimport.cpp and qgsrasterfilewriter.cpp
  • QPixmap (X server needed) should be replaced by QImage. Apparently, most of them are introduced by methods that return QIcon. Proposition: modify them to return a QImage (or a QgsIcon that converts to QIcon when needed by the app)

(original focus: make the server not dependent on a X server anymore)

@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn Oct 26, 2016

Member

Thanks for opening this.

Splitting data providers in two modules sounds good to me.

I will probably be looking into editor widgets soon.

Member

m-kuhn commented Oct 26, 2016

Thanks for opening this.

Splitting data providers in two modules sounds good to me.

I will probably be looking into editor widgets soon.

@jef-n

This comment has been minimized.

Show comment
Hide comment
@jef-n

jef-n Oct 26, 2016

Member

I think there was also a problem with QPixmap/QImage introducing a dependency to gui (some room for spliting is probably intoduced because QtWidgets are now separate). Printing/PDF output used to be another (at least on *x).

The providers should also be split into "management" and "layer" - ie. create, drop, rename, export, import of tables in one part and the current provider part in the other. The current api to access the first part of the provider functionality is quite clumsy.

Member

jef-n commented Oct 26, 2016

I think there was also a problem with QPixmap/QImage introducing a dependency to gui (some room for spliting is probably intoduced because QtWidgets are now separate). Printing/PDF output used to be another (at least on *x).

The providers should also be split into "management" and "layer" - ie. create, drop, rename, export, import of tables in one part and the current provider part in the other. The current api to access the first part of the provider functionality is quite clumsy.

@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn Oct 26, 2016

Member

Are you thinking of one "management" provider as backend for several "layer" providers (i.e. "management" corresponds to a database while "layer" corresponds to a table)?

I think the most important one is lifting the dependency on the QGIS gui module. Lifting the dependency on QtWidgets would be even better.

Member

m-kuhn commented Oct 26, 2016

Are you thinking of one "management" provider as backend for several "layer" providers (i.e. "management" corresponds to a database while "layer" corresponds to a table)?

I think the most important one is lifting the dependency on the QGIS gui module. Lifting the dependency on QtWidgets would be even better.

@mhugo

This comment has been minimized.

Show comment
Hide comment
@mhugo

mhugo Oct 26, 2016

@jef-n yeah avoiding dependencies to an X server would even be better. That means QPixmap and maybe lots of other classes (QFont ? QPrinter ?, etc.)

mhugo commented Oct 26, 2016

@jef-n yeah avoiding dependencies to an X server would even be better. That means QPixmap and maybe lots of other classes (QFont ? QPrinter ?, etc.)

@rouault

This comment has been minimized.

Show comment
Hide comment
@rouault

rouault Oct 26, 2016

Tiny detail: there's a call to QMessageBox in the saveStyle() function of at least Postgre, Spatialite and OGR providers to confirm if an existing style must be overwritten.

rouault commented Oct 26, 2016

Tiny detail: there's a call to QMessageBox in the saveStyle() function of at least Postgre, Spatialite and OGR providers to confirm if an existing style must be overwritten.

@jef-n

This comment has been minimized.

Show comment
Hide comment
@jef-n

jef-n Oct 26, 2016

Member

Yes. Currently QgsVectorDataProvider handles one layer (w/ sublayers, but those are IMHO clumsy too) and there is createEmptyLayer, deleteLayer etc. you have to use QLibrary::resolve to get to.

Member

jef-n commented Oct 26, 2016

Yes. Currently QgsVectorDataProvider handles one layer (w/ sublayers, but those are IMHO clumsy too) and there is createEmptyLayer, deleteLayer etc. you have to use QLibrary::resolve to get to.

@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn Oct 26, 2016

Member

@jef-n are you interested in working on this?

Member

m-kuhn commented Oct 26, 2016

@jef-n are you interested in working on this?

@rldhont

This comment has been minimized.

Show comment
Hide comment
@rldhont

rldhont Oct 26, 2016

@mhugo an other point: QDialogProgress is used in QgsVectorLayer::countSymbolFeatures qgis/QGIS#2780

rldhont commented Oct 26, 2016

@mhugo an other point: QDialogProgress is used in QgsVectorLayer::countSymbolFeatures qgis/QGIS#2780

@vmora

This comment has been minimized.

Show comment
Hide comment
@vmora

vmora Oct 26, 2016

Anybody knows if tests under ../../tests/src/core/ pass without an X server ?

vmora commented Oct 26, 2016

Anybody knows if tests under ../../tests/src/core/ pass without an X server ?

@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn Oct 26, 2016

Member

Anybody knows if tests under ../../tests/src/core/ pass without an X server ?

They don't. Use xfvb-run

Member

m-kuhn commented Oct 26, 2016

Anybody knows if tests under ../../tests/src/core/ pass without an X server ?

They don't. Use xfvb-run

@mhugo

This comment has been minimized.

Show comment
Hide comment
@mhugo

mhugo Oct 26, 2016

@rldhont @rouault thanks for your "tiny" details (!) I've added some more in the ticket description.

Does anybody know why QgsWebView has to be in core ?

mhugo commented Oct 26, 2016

@rldhont @rouault thanks for your "tiny" details (!) I've added some more in the ticket description.

Does anybody know why QgsWebView has to be in core ?

@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn Oct 26, 2016

Member

Does anybody know why QgsWebView has to be in core ?

I think composerlabel/composerhtml

Member

m-kuhn commented Oct 26, 2016

Does anybody know why QgsWebView has to be in core ?

I think composerlabel/composerhtml

@rldhont

This comment has been minimized.

Show comment
Hide comment
@rldhont

rldhont Oct 4, 2017

For server side does-it mean that gui dependencies has been remove ?

rldhont commented Oct 4, 2017

For server side does-it mean that gui dependencies has been remove ?

@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn Oct 4, 2017

Member

For server side does-it mean that gui dependencies has been remove ?

Yes qgis/QGIS#4824

Member

m-kuhn commented Oct 4, 2017

For server side does-it mean that gui dependencies has been remove ?

Yes qgis/QGIS#4824

@wonder-sk

This comment has been minimized.

Show comment
Hide comment
@wonder-sk

wonder-sk Oct 14, 2017

Member

Can we close this one? Server now compiles without qgis_gui library, and it is possible to compile providers without select widgets (using WITH_GUI=FALSE cmake option).

Removal of dependency of X server in qgis_core is not going to be easy (or even possible? - isn't X server required for text rendering?) - and I am not sure if there is any added value if we did such effort...

Member

wonder-sk commented Oct 14, 2017

Can we close this one? Server now compiles without qgis_gui library, and it is possible to compile providers without select widgets (using WITH_GUI=FALSE cmake option).

Removal of dependency of X server in qgis_core is not going to be easy (or even possible? - isn't X server required for text rendering?) - and I am not sure if there is any added value if we did such effort...

@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn Oct 16, 2017

Member

The only thing left on my wish-list is to really decouple provider and provider_gui. While it's in practice possible to compile server without a QtGui dependency, in practice no distribution deploys such binaries because normally the whole stack is created with one set of cmake options and then packaged.

Member

m-kuhn commented Oct 16, 2017

The only thing left on my wish-list is to really decouple provider and provider_gui. While it's in practice possible to compile server without a QtGui dependency, in practice no distribution deploys such binaries because normally the whole stack is created with one set of cmake options and then packaged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment