Geopackage: handle raster drop in browser #5057
Conversation
This is being reused in gpkg data items
| } | ||
| } | ||
| GDALClose( hSrcDS ); | ||
| GDALTranslateOptionsFree( psOptions ); |
There was a problem hiding this comment.
missing GDALClose( hOutDS )
|
|
||
| const char *args[] = { "-of", "gpkg", "-co", QStringLiteral( "RASTER_TABLE=%1" ).arg( u.name ).toUtf8().constData(), "-co", "APPEND_SUBDATASET=YES", nullptr }; | ||
| GDALTranslateOptions *psOptions = GDALTranslateOptionsNew( ( char ** )args, nullptr ); | ||
| GDALDatasetH hSrcDS = GDALOpen( u.uri.toUtf8().constData(), GA_ReadOnly ); |
There was a problem hiding this comment.
It would be prudent to check hSrcDS != NULL just in case
| GDALTranslateOptions *psOptions = GDALTranslateOptionsNew( ( char ** )args, nullptr ); | ||
| GDALDatasetH hSrcDS = GDALOpen( u.uri.toUtf8().constData(), GA_ReadOnly ); | ||
| CPLErrorReset(); | ||
| GDALDatasetH hOutDS = GDALTranslate( mPath.toUtf8().constData(), hSrcDS, psOptions, NULL ); |
There was a problem hiding this comment.
GDALTranslate() requires GDAL >= 2.1. Currently only >= 2.0 is required : https://github.com/qgis/QGIS/blob/master/cmake/FindGDAL.cmake#L104
There was a problem hiding this comment.
In the Debian/Ubuntu world, the jump was from 1.11 to 2.1, and people using for example Ubuntu 16.04 have to use UbuntuGIS to build QGIS that ship with at least 2.1 as well. OSGeo4W has GDAL 2.2. So would seem to be OK to me.
|
Thanks for your efforts on supporting geopackage @elpaso ! Is-it possible to implement some kind of helpers, especially for python plugin coders for importing vectors and rasters easily, without doing the same kind of R&D that you are doing by trying It would be very nice if we can have a set of tool for plugins to create/update/delete layers in a geopackage, without switching between the QGIS and GDAL/OGR API. I was writing a small wrapper https://github.com/Gustry/geopackage-qgis but it could be nice to have something in QGIS. |
|
@Gustry yes, my preferred way would be to rely on QGIS API instead of going upstream with GDAL. If you want to go that route (that would probably be ~ 10 times slower anyway), I think you need some help from the raster experts ( @rouault , @blazek etc. ), btw, I've not yet given up completely, I want to spend some more time to see if I can make it work. If I don't succeed (that is very likely), I would go with this GDAL implementation and wrap it into a generic re-usable translate class (and a companion task), I think that it would be very handy for many use cases. For the gpkg layer handling wrapper, in theory all needed functionality should be in the provider: I think we should implement raster delete in GDAL, I've added the |
I don't know how QgsRasterFileWriter works, but the GDAL GPKG driver implements the GDAL Raster API, so in theory QgsRasterFileWriter should work with it. Well that's the theory. There must be some grain of sand somewhere...
A related question was asked this morning on GDAL ML. An enhancement ticket has been filed https://trac.osgeo.org/gdal/ticket/7013 |
Because we need GDALTranslate()
This implementation uses GDALTranslate and a QgsTask.
elpaso
changed the title
|
@nyalldawson I've wrapped the importer into a QgsTask, can you please have a look? |
There was a problem hiding this comment.
Looks good to me! @rouault could you also take a look here?
Now that 2.1 is required
|
@nyalldawson I've removed some ifdefs, there is one inside test that cannot be removed because the build will error: https://github.com/qgis/QGIS/blob/master/tests/src/core/testqgsinvertedpolygonrenderer.cpp#L162 ( @mhugo would you mind having a look?) |
|
@elpaso that test is broken - I tried removing the ifdef a while back with no luck there. |
Description
This implementation is using
GDALTranslatedirectly, my previous attempt to useQgsRasterFileWriterwas not working because the originated gpkg table was created ok but it was completely empty.As a side note, the failed attempt to import a sample raster with the
QgsRasterFileWritertask was at least 10 times slower than usingGDALTranslatedirectly.