-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
SFCGAL: Use WKB instead of WKT #11006
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see alpine:edge has sfcgal 1.5.2, so we should get some CI testing for the new code path
#if SFCGAL_VERSION_MAJOR >= 1 && SFCGAL_VERSION_MINOR >= 5 && \ | ||
SFCGAL_VERSION_PATCH >= 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That will break for 1.6.0 or 2.0.0
So more something like:
#if SFCGAL_VERSION_MAJOR >= 1 && SFCGAL_VERSION_MINOR >= 5 && \ | |
SFCGAL_VERSION_PATCH >= 2 | |
#if SFCGAL_VERSION_MAJOR * 10000 + SFCGAL_VERSION_MINOR * 100 + SFCGAL_VERSION_PATCH >= 1 * 10000 + 5 * 100 + 2 |
(would be better to hav a SFCGAL_MAKE_VERSION(major, minor, path) macro that does that math :-)
Same below
size_t nSize = poLS->WkbSize(); | ||
unsigned char *pabyWkb = static_cast<unsigned char *>(CPLMalloc(nSize)); | ||
|
||
// Create export options and set byte order | ||
OGRwkbExportOptions oOptions; | ||
oOptions.eByteOrder = wkbNDR; | ||
|
||
if (poLS->exportToWkb(pabyWkb, &oOptions) == OGRERR_NONE) | ||
{ | ||
sfcgal_geometry_t *_geometry = sfcgal_io_read_wkb( | ||
reinterpret_cast<const char *>(pabyWkb), nSize); | ||
CPLFree(pabyWkb); | ||
return _geometry; | ||
} | ||
else | ||
{ | ||
CPLFree(pabyWkb); | ||
return nullptr; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we have a lambda or a static method that does this job of exporting to Wkb and calling sfcgal_io_read_wkb(), that can be used by this case, and the 3 below ones?
What does this PR do?
This PR adds support for WKB (Well-Known Binary) instead of WKT (Well-Known Text) for SFCGAL versions greater than or equal to 1.5.2.
It is noted that SFCGAL introduced support for WKB in version 1.5.1, but full binary support is available starting from version 1.5.2. Therefore, this update ensures that WKB is utilized for SFCGAL versions 1.5.2 and above.
Additionally, the PR includes a check for the SFCGAL version using a configure_file to generate the ogr_sfcgal.h file, and modifications are made to the CMakeLists.txt to handle this versioning logic properly.
This PR is currently opened as a draft because I still need to verify some parts of the GDAL code to ensure everything is properly implemented and handled as expected.
What are related issues/pull requests?
Tasklist
Environment
Provide environment details, if relevant: