Avoid dependency to gui when not needed #66

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

Projects

None yet

6 participants

@mhugo
mhugo commented Oct 26, 2016 edited

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)

@rldhont rldhont referenced this issue in qgis/QGIS-Enhancement-Proposals Oct 26, 2016
Open

QEP 74: QGIS server code refactoring for QGIS 3.0 #74

1 of 8 tasks complete
@m-kuhn
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
Member
jef-n commented Oct 26, 2016 edited

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
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
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
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
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
Member
m-kuhn commented Oct 26, 2016

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

@rldhont
rldhont commented Oct 26, 2016

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

@vmora
vmora commented Oct 26, 2016

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

@m-kuhn
Member
m-kuhn commented Oct 26, 2016 edited

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

They don't. Use xfvb-run

@mhugo
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
Member
m-kuhn commented Oct 26, 2016 edited

Does anybody know why QgsWebView has to be in core ?

I think composerlabel/composerhtml

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