LIBKML: report XML id attribute as 'id' field; implement SetFeature() and DeleteFeature() for regular layers

[rouault]

Fixes qgis/QGIS#58780

[coveralls]

coverage: 69.429% (+0.01%) from 69.417%
when pulling 518c2e2 on rouault:libkml_update_delete
into 927be3f on OSGeo:master.

[agiudiceandrea]

@rouault, I've tested this PR using QGIS 3.39.0-Master bb9a7165e6 GDAL/OGR 3.10.0dev-4e28921731 (OSGeo4W) on Windows 10: it seems to me it doesn't actually fix qgis/QGIS#58780.

To reproduce the issue:

• In QGIS, add the "world" layer, select a couple of polygons and export the selected polygons to kml format with default parameters
• import the exported kml layer, if not automatically imported
• start editing
• modify an attribute of a feature, stop editing and try to save the changes, see the error:

Could not commit changes to layer world_map

Errors: ERROR: 1 attribute value change not applied.
  
  Provider errors:
      OGR error setting feature 1: 

CRITICAL    Layer world_map : OGR error setting feature 1: 
WARNING    Commit errors : Could not commit changes to layer world_map

• select a polygon and delete it, stop editing and try to save the changes, see the error:

Could not commit changes to layer world_map

Errors: ERROR: 1 feature not deleted.
  
  Provider errors:
      OGR error deleting feature 2: 

CRITICAL    Layer world_map : OGR error deleting feature 2: 
WARNING    Commit errors : Could not commit changes to layer world_map

• select a polygon and modify a vertex, stop editing and try to save the changes, see the error:

Could not commit changes to layer world_map

Errors: ERROR: 1 geometry not changed.
  
  Provider errors:
      OGR error setting feature 2: 

CRITICAL    Layer world_map : OGR error setting feature 2: 
WARNING    Commit errors : Could not commit changes to layer world_map

• stop the editing session and discard the changes:
  WARNING Unbalanced call to leaveUpdateMode() w.r.t. enterUpdateMode()

[rouault]

it seems to me it doesn't actually fix qgis/QGIS#58780.

the issue here was rather at the step where you export a KML file from the selected polygons. This uses the "KML" driver (not the "LIBKML" one), which up to know generated Placemark without an id. But LIBKML edition requires placemarks to have id. I've addressed the issue on the KML driver side with #10991

[agiudiceandrea]

the issue here was rather at the step where you export a KML file

Thanks again! It's the same step described in the original issue report. So, I suppose the issue will still occur using any kml file exported by QGIS till now and any kml file exported by Google Earth, even with QGIS + GDAL/OGR 3.10. Isn't that so?

(I wonder why QGIS uses the KML driver to export a layer instead of / alongside LIBKML)

[jratike80]

Why? It appears first on the driver list, and the following shows a hint about what to do in the QGIS settings

[rouault]

Isn't that so?

yes. There isn't a good way of having reliable editing without Placemark.id, or would complicate implementation a bit too much for my taste.
Speaking of that, #10992 will cause the layer to not appear editable if the first placemark has no id.

(I wonder why QGIS uses the KML driver to export a layer instead of / alongside LIBKML)
Why? It appears first on the driver list, and the following shows a hint about what to do in the QGIS settings

That's not enough. For the writing side of QGIS, the list of OGR driver is hardcoded in src/core/qgsvectorfilewriter.cpp

Having both KML and LIBKML would be confusing to users.

[agiudiceandrea]

Why? It appears first on the driver list, and the following shows a hint about what to do in the QGIS settings

@jratike80, AFAIK, that is not the case:

• when both the KML and the LIBKML driver are enabled, then GDAL/OGR in QGIS always uses the LIBKML driver to import a kml file (see https://gdal.org/en/latest/drivers/vector/libkml.html), while it uses the KML driver to export a layer to the kml format
• when the KML driver is disabled and the LIBKML driver only is enabled, then GDAL/OGR in QGIS uses the KML driver to read a kml file, but it is not possible to export a layer to the kml format.

[jratike80]

Having both KML and LIBKML would be confusing to users.

In this case also reading with LIBKML and writing with KML has also made some confusion.
Does hardcoding mean that if user sets environment OGR_SKIP=KML then writing is not possible at all?

[agiudiceandrea]

Does hardcoding mean that if user sets environment OGR_SKIP=KML then writing is not possible at all?

Indeed, as written in a previous comment, when the KML driver is disabled and the LIBKML driver only is enabled in QGIS, then ... it is not possible to export a layer to the kml format (using the Save Vector Layer functionality)