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

Revise template georeferencing setup #1522

Merged
merged 3 commits into from
Feb 13, 2020
Merged

Revise template georeferencing setup #1522

merged 3 commits into from
Feb 13, 2020

Conversation

dg0yt
Copy link
Member

@dg0yt dg0yt commented Feb 12, 2020

Increase flexibility without further complicating the code. Most of all,

  • allows overriding of CRS information when it differs from the map, or when (re-)enabling georeferencing for an existing template,
  • mitigates (Slightly) misplaced templates #1515 (slightly misplaced templates.

This type was used for a parameter which had two usages, both with
the same actual value.
With the parameter removed, we can simplify the constructor.
This change facilitates further modifications.
@dg0yt dg0yt requested a review from lpechacek February 12, 2020 08:21
@dg0yt
Copy link
Member Author

dg0yt commented Feb 12, 2020

@lpechacek I don't ask for a thorough review. I just would like to verify that the changes take a good direction, and we can let them into the pending master snapshot.

Handle CRS and transformation options more independently.
Pass available georeferencing options to the CRS selection dialog,
and initialize the "current" selection accordingly.
Allow overriding of CRS information when it differs from the map,
or when (re-)enabling georeferencing for an existing template.
Allow world files to be used even with local georeferencing
(assuming same CRS).
Mitigates GH-1515 (slightly misplaced templates).
Don't return the resulting value, but indicate success or error.
This is necessary to suppress an error message if the user
intentionally cancelled CRS configuration on enabling geoerefencing.
Copy link
Member

@lpechacek lpechacek left a comment

Choose a reason for hiding this comment

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

For what it's worth, I don't see anything disturbing in the patch set. I've also tested the new changes and the functionality I'm using seems to work as before. I believe this patchset is safe for master. Thanks!

@dg0yt dg0yt merged commit de9b337 into master Feb 13, 2020
@dg0yt dg0yt mentioned this pull request Mar 13, 2020
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.

None yet

2 participants