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

Map rotation support #1716

Closed
wants to merge 46 commits into from
Closed

Map rotation support #1716

wants to merge 46 commits into from

Conversation

strk
Copy link
Contributor

@strk strk commented Dec 2, 2014

A rebased version of #1700 (still to be squashed once tests work)

Sandro Santilli added 30 commits December 2, 2014 11:50
WARNING: UNFINISHED WORK
Adds rotation knowledge in maprenderer (just storage, nothing more).
Includes signals for rotation change and sync between renderer/canvas.

Widget is a simple text input, will need to be changed to a spinbox
Add QgsRectangle.rotate function. Pass map center to QgsMapToPixel
WARNING: still far away from a working setup
I've verified that angles are all retained correctly out of
QgsMapToPixel::transformInPlace, so the stretching of them must
be due to external code, maybe in renderer ?
Rotation makes the rectangle aspect ratio change
Clips rendered image in QgsMapCanvasMap::paint, set rendered image
extent to the rotated visible extent. WARNING: needs further cleanup
Shows the bounding rect is not correctly centered when the map
is not centered on 0,0
It seems to fix all cases for now
NOTE: I'd also like to try a QDial instead before going final
This fixes the clipping problems but makes all navigation code based
on the concept of "extent" forceably reducing scale (as rotated extent
is now larger than the non-rotated one). The solution would be NOT
to rely on "extent" but rather on scale and center.
This is to reduce the problems with using extent w/out considering
rotation. Fixes setting map center via the Coordinate edit widget
Fixes display of visible extent in app control and overview
The simpler transformation matrix uses xMin/yMin rather than centers,
hopefully replicating the original code. The aim is to get a successful
run of the testsuite here.
want to take a closer look at the travis run.
current run failed just one test:
https://travis-ci.org/qgis/QGIS/builds/42632320
@strk
Copy link
Contributor Author

strk commented Dec 3, 2014

I've pushed some support for raster rotation. It fails at setting correct scale and offset but I still hadn't sorted out how to correctly deal with that.

Meanwhile Travis is still failing 2 tests, any help with figuring that out ?
The LocalServer for example, has a test expecting a solid red rendering and getting no rendering at all:
http://dash.orfeo-toolbox.org/testDetails.php?test=27227166&build=165511

@nirvn
Copy link
Contributor

nirvn commented Dec 4, 2014

@strk forgot to mention that the north arrow decoration (visible in the above uploaded screenshots) isn't rotated to match the map rotation value. the decoration should rotate along with the map.

@strk
Copy link
Contributor Author

strk commented Dec 4, 2014

@Nirv I know about that but I'm not planning to work on that. I could mention more things that would need to get support for rotation, only I can't work on all of them with the time I have available. For example, grid decoration.

@blazek
Copy link
Member

blazek commented Dec 4, 2014

For performance reasons the rotation should happen at the beginning of the pipe, similar as reprojection, I think, but I have no detailed ideas.

@Nirv
Copy link

Nirv commented Dec 4, 2014

Hi,
(Sorry, i'm french).
Please, be careful to write @nirvn , and not @Nirv. Because @Nirv it's me and i received emails about comments while i'm not concerned.

Thanks you

Sandro Santilli added 3 commits December 4, 2014 23:45
Basically the QgsMapToPixel is "unrotated" for the calculations
in the renderer. Rotation is later inducted during draw.
@strk
Copy link
Contributor Author

strk commented Dec 5, 2014

With latest commit I think raster support for rotation is acceptable.
Basically I delegate all rotation work to QPainter and leave computation of visible extent and such as if no rotation took place.

So we're back to the need to pass the testsuite as the only blocking factor for getting this in master (IMHO).

Sandro Santilli added 3 commits December 5, 2014 14:25
Hopefully gives expected output for existing raster rendering tests
It seems to give more successes for me locally
... it needs much more love than this ...
@strk
Copy link
Contributor Author

strk commented Dec 5, 2014

So right now PyQgsPalLabelingComposer is the only failing test. It also fails on my local system but I can't understand how to run the single failing one to understand better what's going on.

@dakcarto any help with that ?

@strk
Copy link
Contributor Author

strk commented Dec 5, 2014

NOTE: the PyQgsPalLabelingComposer test on my machine also fails with master, as of 7400106

@strk
Copy link
Contributor Author

strk commented Dec 5, 2014

Sandro Santilli added 6 commits December 7, 2014 11:45
This reverts commit 1ecb0a2.

... just trying if this change was really needed for travis to pass

Conflicts:
	src/core/qgsmaptopixel.cpp
This reverts commit fa31bfa.

Travis confirmed that the updated expected images were needed.
NOTE: all this noise will need to be squashed before merge to master
Leave only the qreal based function to transform in place
I can't tell with my eye what the difference is, but the tests complain,
see http://dash.orfeo-toolbox.org/testDetails.php?test=27287021&build=165868
@strk
Copy link
Contributor Author

strk commented Dec 7, 2014

finally GREEN!
Touching base, I'll rebase again and squash some to get to step 2.

@strk
Copy link
Contributor Author

strk commented Dec 7, 2014

Squash-rebased version in #1722

@strk strk closed this Dec 7, 2014
@strk
Copy link
Contributor Author

strk commented Dec 7, 2014

Bah, for some reason the squashed version failed on travis, so reopening this one

@strk strk reopened this Dec 7, 2014
@strk
Copy link
Contributor Author

strk commented Dec 7, 2014

Found the issue, I had missed a commit, back to #1722

@strk strk closed this Dec 7, 2014
@gioman
Copy link
Contributor

gioman commented Dec 9, 2014

The zoom and the selection tools (by dragging the mouse) feel a bit odd because they also are rotated (but their size seems to follow the horizontal/vertical movements), is this to stay as it is now?
263

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

5 participants