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

Allow to create CompoundCurve, CurvePolygon, MultiCurve, MultiSurface memory layers #37435

Merged

Conversation

agiudiceandrea
Copy link
Contributor

@agiudiceandrea agiudiceandrea commented Jun 26, 2020

Description

Currently, the New Temporary Scratch Layer allows to create, among the others, the following layer types:

  • "LineString / CompoundCurve"
  • "Polygon / CurvePolygon"
  • "MultiLineString / MultiCurve"
  • "MultiPolygon / MultiSurface"

but actually the following QgsWkbTypes layer are created respectively (see #5880 and #25491):

  • QgsWkbTypes::LineString
  • QgsWkbTypes::Polygon
  • QgsWkbTypes::MultiLineString
  • QgsWkbTypes::MultiPolygon

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 );

While Polygon memory layers can store CurvePolygon geometries, some processing tools, as "Convert to curved geometries" (qgis:converttocurves) 9939142 (PR #35875), need specific layer types (CurvePolygon for qgis:converttocurves) to work properly.

This PR gives the users more choices and allow them to create true CompoundCurve, CurvePolygon, MultiCurve and MultiSurface layers:

  • "LineString" -> QgsWkbTypes::LineString
  • "CompoundCurve" -> QgsWkbTypes::CompoundCurve
  • "Polygon" -> QgsWkbTypes::Polygon
  • "CurvePolygon" -> QgsWkbTypes::CurvePolygon
  • "MultiLineString" -> QgsWkbTypes::MultiLineString
  • "MultiCurve" -> QgsWkbTypes::MultiCurve
  • "MultiPolygon" -> QgsWkbTypes::MultiPolygon
  • "MultiSurface" -> QgsWkbTypes::MultiSurface

like it is for the new geopackage layer dialog:

mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconLineLayer.svg" ) ), tr( "Line" ), wkbLineString );
mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPolygonLayer.svg" ) ), tr( "Polygon" ), wkbPolygon );
mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPointLayer.svg" ) ), tr( "MultiPoint" ), wkbMultiPoint );
mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconLineLayer.svg" ) ), tr( "MultiLine" ), wkbMultiLineString );
mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPolygonLayer.svg" ) ), tr( "MultiPolygon" ), wkbMultiPolygon );

mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconLineLayer.svg" ) ), tr( "CompoundCurve" ), wkbCompoundCurve );
mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPolygonLayer.svg" ) ), tr( "CurvePolygon" ), wkbCurvePolygon );
mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconLineLayer.svg" ) ), tr( "MultiCurve" ), wkbMultiCurve );
mGeometryTypeBox->addItem( QgsApplication::getThemeIcon( QStringLiteral( "/mIconPolygonLayer.svg" ) ), tr( "MultiSurface" ), wkbMultiSurface );

Fixes #37406

@github-actions github-actions bot added this to the 3.16.0 milestone Jun 26, 2020
@m-kuhn m-kuhn requested a review from olivierdalang July 1, 2020 11:22
@m-kuhn
Copy link
Member

m-kuhn commented Jul 1, 2020

Cool

@olivierdalang can you have a look at this?

@olivierdalang
Copy link
Contributor

Looks good to me.

Initially I was more for keeping the entries in the menu as is, and just always create curved layers (as I can't really think of a scenario where an user would specifically require a non-curved layer, and that it's a bit confusing as non-curved memory layers actually support curves), but matching geopackages UI also makes sense, so I'm OK with either.

Thanks @agiudiceandrea :-)

@stale
Copy link

stale bot commented Jul 18, 2020

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@stale stale bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jul 18, 2020
@agiudiceandrea
Copy link
Contributor Author

@m-kuhn any chance this PR will be merged?

@stale stale bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jul 18, 2020
@m-kuhn m-kuhn merged commit 407fd19 into qgis:master Jul 18, 2020
@m-kuhn
Copy link
Member

m-kuhn commented Jul 18, 2020

Sorry that slipped through the cracks.
Thanks!

@agiudiceandrea agiudiceandrea deleted the fix-37406-memorylayer-curvepolygon branch July 21, 2020 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Temporary layers have type Polygon (Polygon) instead of Polygon (CurvePolygon)
3 participants