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

Rotate geometry before calculating bounding box in atlas #4730

Merged
merged 3 commits into from Jul 4, 2017

Conversation

Projects
None yet
3 participants
@Zverik
Contributor

Zverik commented Jun 14, 2017

When maps in the composer were rotated based on current atlas object, the bounding box for the object was calculated without taking the rotation into account. This should fix bug 11954.

Tested locally, it worked.

@nyalldawson

This comment has been minimized.

Show comment
Hide comment
@nyalldawson

nyalldawson Jun 14, 2017

Contributor

Nice fix for an annoying issue - thanks! Could you add a unit test for this in https://github.com/qgis/QGIS/blob/master/tests/src/python/test_qgsatlascomposition.py ?

(by the way, #8466 and #8125 are totally different issues (not related to map rotation). They both relate to rotating any composer item, selecting multiple items, and then attempting to resize using the mouse drag handles)

Contributor

nyalldawson commented Jun 14, 2017

Nice fix for an annoying issue - thanks! Could you add a unit test for this in https://github.com/qgis/QGIS/blob/master/tests/src/python/test_qgsatlascomposition.py ?

(by the way, #8466 and #8125 are totally different issues (not related to map rotation). They both relate to rotating any composer item, selecting multiple items, and then attempting to resize using the mouse drag handles)

@nyalldawson

needs clarification regarding use of centroid

@Zverik

This comment has been minimized.

Show comment
Hide comment
@Zverik

Zverik Jun 15, 2017

Contributor

Thanks for the review, Nyall (sorry I've misspelled the first time :). You are probably right regarding the centroid: in my tests the center was slightly off. I'll fix that.

I also would have to learn the python testing framework you use, so this would take some time.

Contributor

Zverik commented Jun 15, 2017

Thanks for the review, Nyall (sorry I've misspelled the first time :). You are probably right regarding the centroid: in my tests the center was slightly off. I'll fix that.

I also would have to learn the python testing framework you use, so this would take some time.

This was referenced Jun 15, 2017

@Zverik

This comment has been minimized.

Show comment
Hide comment
@Zverik

Zverik Jun 18, 2017

Contributor

I could not solve the issue of shifted map extend, so for now I grow the extent a little, so the feature is completely inside bounds. Experiments show that the margin is still very small, although for complex features it is noticeable. The math turned out too complex for me, sorry.

The python test is also done. Please take another look at this, @nyalldawson.

Contributor

Zverik commented Jun 18, 2017

I could not solve the issue of shifted map extend, so for now I grow the extent a little, so the feature is completely inside bounds. Experiments show that the margin is still very small, although for complex features it is noticeable. The math turned out too complex for me, sorry.

The python test is also done. Please take another look at this, @nyalldawson.

@Zverik

This comment has been minimized.

Show comment
Hide comment
@Zverik

Zverik Jun 27, 2017

Contributor

@3nids could you please look at this and merge if it is okay?

Contributor

Zverik commented Jun 27, 2017

@3nids could you please look at this and merge if it is okay?

@3nids

This comment has been minimized.

Show comment
Hide comment
@3nids

3nids Jun 27, 2017

Member

I might not be the right guy for this one.
@nyalldawson ok for you to merge? or someone else?

Member

3nids commented Jun 27, 2017

I might not be the right guy for this one.
@nyalldawson ok for you to merge? or someone else?

@nyalldawson

This comment has been minimized.

Show comment
Hide comment
@nyalldawson

nyalldawson Jun 27, 2017

Contributor

Sorry for the delay - I'll take a look again tomorrow

Contributor

nyalldawson commented Jun 27, 2017

Sorry for the delay - I'll take a look again tomorrow

@Zverik

This comment has been minimized.

Show comment
Hide comment
@Zverik

Zverik Jul 3, 2017

Contributor

@nyalldawson ping :)

Contributor

Zverik commented Jul 3, 2017

@nyalldawson ping :)

@nyalldawson nyalldawson merged commit 70d2ae2 into qgis:master Jul 4, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@nyalldawson

This comment has been minimized.

Show comment
Hide comment
@nyalldawson

nyalldawson Jul 4, 2017

Contributor

Nice work - thank you!

Contributor

nyalldawson commented Jul 4, 2017

Nice work - thank you!

@Zverik Zverik deleted the Zverik:atlas_rotate branch Jul 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment