Skip to content

add support for SFCGAL library #62611

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

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

benoitdm-oslandia
Copy link
Collaborator

This PR is the implementation of this QEP and follow this closed PR.

We add 2 main classes:

  • QgsSfcgalEngine: exposes SFCGAL C API to QGIS. Also adds memory and error management.
  • QgsSfcgalGeometry: keep an handle to SFCGAL geometry representation to reduce geometry conversion between QGIS and SFCGAL

The integration in QgsGeometry class will be done in a future PR after the geometry backend support is merged.

Sponsored by:

@github-actions github-actions bot added this to the 3.46.0 milestone Jul 17, 2025
Copy link
Contributor

github-actions bot commented Jul 17, 2025

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 066dfaf)

🍎 MacOS Qt6 builds

Download MacOS Qt6 builds of this PR for testing.
This installer is not signed, control+click > open the app to avoid the warning
(Built from commit 066dfaf)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 066dfaf)


find_package(SFCGAL) # SFCGAL provider
message(STATUS "Found SFCGAL: ${SFCGAL_VERSION} ${SFCGAL_DIR}")
if(SFCGAL_VERSION VERSION_LESS "2.1")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please lower this to 2.0 -- 2.1 is much too recent (it isn't even available on Fedora 42)

Suggested change
if(SFCGAL_VERSION VERSION_LESS "2.1")
if(SFCGAL_VERSION VERSION_LESS "2.0")

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need at least 2.1 and it is available in rawhide. We still need to change the build stage for fedora 42 and add the rawhide repository only for sfcgal dependencies. It should not impact other dependencies.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need at least 2.1 and it is available in rawhide

Can you elaborate why? Rawhide isn't a realistic option.

@nyalldawson
Copy link
Collaborator

Before merging we'll also need the CI updated so that this new code is built and tested

public:

/**
* Creates a SFGAL geometry (inherit QgsAbstractGeometry) from an internal SFCGAL geometry (from SFCGAL library).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant ALL the docs. I can spot others where the content is misleading -- eg "Same as toAbstractGeometry but returned object is casted to QgsSfcgalGeometry."

Comment on lines 80 to 81
//! Returns the underlying QGIS geometry
QgsAbstractGeometry *asQgisGeometry( QString *errorMsg = nullptr ) const;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unique_ptr return still outstanding

/**
* Export the geometry as WKT
*
* \param precision Floating point precision for WKT coordinates. Setting to -1 yields rational number WKT (not decimal).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not actually WKT then, is it? At least the QGIS wkt parser won't accept fraction strings...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants