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

Add new option to proj_create_crs_to_crs_from_pj method to force +over on transformation operations #2914

Merged
merged 9 commits into from
Nov 12, 2021

Conversation

mlptownsend
Copy link
Contributor

The stale bot closed my original PR 2528. I didn't have the free time to follow up on this until now.

My original strategy used a method to set a flag on the context to set the PJ->over flag. The problem is getting the value down to the pipelines. As-is, there isn't really a clear path down to send options, and I didn't really feel comfortable making that big of a breaking change to the internal APIs to hurl down additional parameters.

image
For instance in this situation, the PJ created in the pipeline method is the PJ that needs to actually receive the 1 in its over flag. But it actually gets generated through calling proj_list_get. Thus, I still had to keep the flag on the context. I just clear it when proj_create_crs_to_crs_from_pj returns. I'm not the biggest fan of the approach if anyone has a better idea there.

@mlptownsend mlptownsend marked this pull request as ready for review October 26, 2021 21:50
@@ -201,6 +201,9 @@ paragraph for more details.
- ALLOW_BALLPARK=YES/NO: can be set to NO to disallow the use of
:term:`Ballpark transformation` in the candidate coordinate operations.

- FORCEOVER=YES/NO: can be set to YES to force the +over flag on the transformation
Copy link
Member

Choose a reason for hiding this comment

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

maybe with an underscore (and change accordingly in the rest of the code) ?

Suggested change
- FORCEOVER=YES/NO: can be set to YES to force the +over flag on the transformation
- FORCE_OVER=YES/NO: can be set to YES to force the +over flag on the transformation

src/4D_api.cpp Outdated
@@ -1843,6 +1843,7 @@ PJ *proj_create_crs_to_crs_from_pj (PJ_CONTEXT *ctx, const PJ *source_crs, cons
const char* authority = nullptr;
double accuracy = -1;
bool allowBallparkTransformations = true;
int forceOver = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
int forceOver = 0;
bool forceOver = false;

and use true/false in all other places

@@ -683,6 +683,7 @@ struct pj_ctx{
void *logger_app_data = nullptr;
struct projCppContext* cpp_context = nullptr; /* internal context for C++ code */
int use_proj4_init_rules = -1; /* -1 = unknown, 0 = no, 1 = yes */
int forceOver = 0; /* 0 = no, 1 = yes */
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
int forceOver = 0; /* 0 = no, 1 = yes */
bool forceOver = false;

src/4D_api.cpp Outdated
@@ -1930,6 +1940,7 @@ PJ *proj_create_crs_to_crs_from_pj (PJ_CONTEXT *ctx, const PJ *source_crs, cons
if( preparedOpList.empty() )
{
proj_destroy(P);
ctx->forceOver = 0;
Copy link
Member

Choose a reason for hiding this comment

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

instead of doing that here and in the below cases, you could likely do that just after pj_create_prepared_operations()

@mlptownsend
Copy link
Contributor Author

Adjusted. No rush on trying to meet any sort of release deadline on it since you're trying to get 8.2 out the door.

I created another flag that does something a lil' bit similar. When I have another bout of free time I'll try and whip up another PR for it. It might end up working itself in the create function in the same kind of way.

@rouault rouault added this to the 9.0.0 milestone Nov 12, 2021
@rouault rouault merged commit ac88226 into OSGeo:master Nov 12, 2021
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.

Using +over with EPSG codes and WKT CRS Input/Output?
2 participants