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

Temporary layers have type Polygon (Polygon) instead of Polygon (CurvePolygon) #37406

Closed
olivierdalang opened this issue Jun 25, 2020 · 3 comments · Fixed by #37435
Closed

Temporary layers have type Polygon (Polygon) instead of Polygon (CurvePolygon) #37406

olivierdalang opened this issue Jun 25, 2020 · 3 comments · Fixed by #37435
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter!

Comments

@olivierdalang
Copy link
Contributor

Describe the bug

Temporary layers (line and polygons) are meant to support curved geometries, but the type of the layers is Polygon instead of CurvePolygon. While it seems to still work when digitizing curves, it causes issues with some features that rely on the layer type (e.g. the ConvertToCurves algorithm segmentizes the geometries when editing in place)

How to Reproduce

  1. Create a new temporary layer of type Polygon / CurvedPolygon
  2. Open the layer properties panel, informations tab
  3. Under geometry, it states Polygon (Polygon) instead of Polygon (CurvePolygon)

QGIS and OS versions

QGIS version
3.14.0-Pi
QGIS code revision
9f7028fd23
Compiled against Qt
5.11.2
Running against Qt
5.11.2
Compiled against GDAL/OGR
3.0.4
Running against GDAL/OGR
3.0.4
Compiled against GEOS
3.8.1-CAPI-1.13.3
Running against GEOS
3.8.1-CAPI-1.13.3
Compiled against SQLite
3.29.0
Running against SQLite
3.29.0
PostgreSQL Client Version
11.5
SpatiaLite Version
4.3.0
QWT Version
6.1.3
QScintilla2 Version
2.10.8
Compiled against PROJ
6.3.2
Running against PROJ
Rel. 6.3.2, May 1st, 2020
OS Version
Windows 10 (10.0)
Active python plugins
autocurve; 
pluginbuilder3; 
plugin_reloader; 
qgepplugin; 
QGIS3-getWKT; 
swiss_locator; 
db_manager; 
processing
@olivierdalang olivierdalang added the Bug Either a bug report, or a bug fix. Let's hope for the latter! label Jun 25, 2020
@agiudiceandrea
Copy link
Contributor

agiudiceandrea commented Jun 25, 2020

For "LineString / CompoundCurve", "Polygon / CurvePolygon", "MultiLineString / MultiCurve" and "MultiPolygon / MultiSurface" actually a LineString, Polygon, MultiLineString and MultiPolygon layer is created respectively (see #5880 and #25491).

mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconLineLayer.svg" ) ), tr( "LineString / CompoundCurve" ), QgsWkbTypes::LineString );
mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPolygonLayer.svg" ) ), tr( "Polygon / CurvePolygon" ), QgsWkbTypes::Polygon );
mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPointLayer.svg" ) ), tr( "MultiPoint" ), QgsWkbTypes::MultiPoint );
mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconLineLayer.svg" ) ), tr( "MultiLineString / MultiCurve" ), QgsWkbTypes::MultiLineString );
mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPolygonLayer.svg" ) ), tr( "MultiPolygon / MultiSurface" ), QgsWkbTypes::MultiPolygon );

So, a simple patch could be to list the geometry types without merging them and allow to create true CompoundCurve, CurvePolygon, MultiCurve and MultiSurface layers agiudiceandrea@91d157a

  mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconLineLayer.svg" ) ), tr( "LineString" ), QgsWkbTypes::LineString );
  mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconLineLayer.svg" ) ), tr( "CompoundCurve" ), QgsWkbTypes::CompoundCurve );
  mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPolygonLayer.svg" ) ), tr( "Polygon" ), QgsWkbTypes::Polygon );
  mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPolygonLayer.svg" ) ), tr( "CurvePolygon" ), QgsWkbTypes::CurvePolygon );
  mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPointLayer.svg" ) ), tr( "MultiPoint" ), QgsWkbTypes::MultiPoint );
  mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconLineLayer.svg" ) ), tr( "MultiLineString" ), QgsWkbTypes::MultiLineString );
  mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconLineLayer.svg" ) ), tr( "MultiCurve" ), QgsWkbTypes::MultiCurve );
  mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPolygonLayer.svg" ) ), tr( "MultiPolygon" ), QgsWkbTypes::MultiPolygon );
  mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPolygonLayer.svg" ) ), tr( "MultiSurface" ), QgsWkbTypes::MultiSurface );

@olivierdalang
Copy link
Contributor Author

Yes I think that would do !

Alternatively, we could just always used curved types (and just make it work as the current labels imply):

-mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconLineLayer.svg" ) ), tr( "LineString / CompoundCurve" ), QgsWkbTypes::LineString );
+mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconLineLayer.svg" ) ), tr( "LineString / CompoundCurve" ), QgsWkbTypes::CompoundCurve ); 
-mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPolygonLayer.svg" ) ), tr( "Polygon / CurvePolygon" ), QgsWkbTypes::Polygon ); 
+mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPolygonLayer.svg" ) ), tr( "Polygon / CurvePolygon" ), QgsWkbTypes::CurvePolygon ); 
-mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconLineLayer.svg" ) ), tr( "MultiLineString / MultiCurve" ), QgsWkbTypes::MultiLineString ); 
+mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconLineLayer.svg" ) ), tr( "MultiLineString / MultiCurve" ), QgsWkbTypes::MultiCurve ); 
-mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPolygonLayer.svg" ) ), tr( "MultiPolygon / MultiSurface" ), QgsWkbTypes::MultiPolygon ); 
+mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPolygonLayer.svg" ) ), tr( "MultiPolygon / MultiSurface" ), QgsWkbTypes::MultiSurface ); 

@agiudiceandrea What do you think ? Can you think of cases where one would NOT want layers supporting curves ? Also worth noting that currently, despite being linear layers, the memory layers still support curved geometries (e.g. digitizing arcs work). It's just some tools that seem to rely on the layer type that get confused. IMO, because of this, keeping the current entries but just fixing the layer types will be less confusing for users.

@agiudiceandrea
Copy link
Contributor

agiudiceandrea commented Jun 26, 2020

I prefer to have more options than less, and to have the possibility to choose according to my needs if it's possible.

Anyway, wouldn't it be possible to have the converttocurves algorithm #35875 handle the Polygon memory layer as well as CurvePolygon, since Polygon memory layer can store CurvePolygon geometries?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter!
Projects
None yet
2 participants