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

Reinstate sfcgal functions #760

Closed

Conversation

lbartoletti
Copy link
Contributor

In version 2.x of PostGIS, there was a mechanism for choosing the backend between
GEOS and SFCGAL for certain operations. When adding the SFCGAL extension, the
backend was removed, and some SFCGAL functions that duplicated GEOS functionality
were also removed. This commit reintroduces these functions but prefixes them
with CG_ to distinguish them from the "classic" GEOS functions using the prefix
ST_.
Often, the 2D functions will produce the same result, sometimes with small
differences due to the algorithms or calculation methods used.

Fixes https://trac.osgeo.org/postgis/ticket/5405

In version 2.x of PostGIS, there was a mechanism for choosing the backend between
GEOS and SFCGAL for certain operations. When adding the SFCGAL extension, the
backend was removed, and some SFCGAL functions that duplicated GEOS functionality
were also removed. This commit reintroduces these functions but prefixes them
with CG_ to distinguish them from the "classic" GEOS functions using the prefix
ST_.
Often, the 2D functions will produce the same result, sometimes with small
differences due to the algorithms or calculation methods used.

Fixes https://trac.osgeo.org/postgis/ticket/5405
@lbartoletti lbartoletti self-assigned this Feb 15, 2024
@lbartoletti lbartoletti marked this pull request as draft February 15, 2024 13:49
@lbartoletti lbartoletti marked this pull request as ready for review February 19, 2024 16:28
@lbartoletti
Copy link
Contributor Author

For the documentation, I added each function in a new sub-folder to make the file reference_sfcgal.xml more digestible. It seems to me that we have to add the ENTRY in postgis.xml; I couldn't do it in reference_sfcgal.xml.
In another PR, I will put the functions prefixed with CG_ instead of ST_ in the same sub-folder, and I will update and organize the functions in reference_sfcgal.xml.

@robe2
Copy link
Member

robe2 commented Feb 21, 2024

For the documentation, I added each function in a new sub-folder to make the file reference_sfcgal.xml more digestible. It seems to me that we have to add the ENTRY in postgis.xml; I couldn't do it in reference_sfcgal.xml. In another PR, I will put the functions prefixed with CG_ instead of ST_ in the same sub-folder, and I will update and organize the functions in reference_sfcgal.xml.

I'd rather we not break each function into a separate file. It's not like there are that many that reference_sfcgal.xml is that bloated. It would also break convention, as other extensions and even postgis proper, do not have a separate file for each function.

We could have more than one sfcgal file, if you feel there are distinct themes deserving their own dedicated file.

@lbartoletti
Copy link
Contributor Author

For the documentation, I added each function in a new sub-folder to make the file reference_sfcgal.xml more digestible. It seems to me that we have to add the ENTRY in postgis.xml; I couldn't do it in reference_sfcgal.xml. In another PR, I will put the functions prefixed with CG_ instead of ST_ in the same sub-folder, and I will update and organize the functions in reference_sfcgal.xml.

I'd rather we not break each function into a separate file. It's not like there are that many that reference_sfcgal.xml is that bloated. It would also break convention, as other extensions and even postgis proper, do not have a separate file for each function.

We could have more than one sfcgal file, if you feel there are distinct themes deserving their own dedicated file.

OK, done in a6dac83

@robe2
Copy link
Member

robe2 commented Feb 26, 2024

@lbartoletti Now that you have commit rights, feel free to commit this yourself. Let me know if you have any questions.

@robe2 robe2 self-requested a review February 26, 2024 01:42
Copy link
Member

@robe2 robe2 left a comment

Choose a reason for hiding this comment

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

All looks good to me

@lbartoletti
Copy link
Contributor Author

@lbartoletti Now that you have commit rights, feel free to commit this yourself. Let me know if you have any questions.

Yes, I didn't see anything in the documentation. But, is it safe to merge on github, or should I push my commits to gitea?

I opened here, to check the Cirrus CI at the same time. I've seen other PRs integrated from Github, but I'm intrigued by this sentence in the documentation:
Changes committed to mirror sites will be overwritten the next time a change is synchronized from OSGeo.

@lbartoletti
Copy link
Contributor Author

@lbartoletti lbartoletti closed this Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants