-
-
Notifications
You must be signed in to change notification settings - Fork 311
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
Viewport.Rotation could get to 360 ° or more #2615
Comments
Thanks for the report. This was done on purpose. The reason is that this makes the logic simpler for animations and other gradual changes over time. You can just work with difference calculations and rotation distance. If you decide to normalize the rotation between 0 and 360 all those calculations need to take a possible sudden change into account. We had such logic before, and checks in several places and there were bugs in that code. Of course there should be no error logged. So, at least that is a bug. Are you aware of any other situations where this could cause problems? |
Ok Logging an error was only the fastest method to display the Rotation value. |
The result could be seen here: https://github.com/Mapsui/Mapsui/assets/4381223/8ecf6196-1359-496b-9aa2-deaf58b279e9 |
Thanks for the movie. It looks indeed weird and it made me re-evaluate the current logic, however, I think the current solution is the best one. Considerations:
|
I can't follow your argumentation. It isn't intuitive to say 'Go in direction of 875°'. That's nothing someone expects. I can't find this value on my compass. So why is it important for 99.9 % of the users to have rotations bigger than 360 degrees? Where is the benefit for all them to check all over their app, if the app user rotated the map two or three times (what could happen on a mobile device) and then press the 'Go to north' button? Why isn't it possible for the rest 0.01 % to divide their rotation in 9 rotations by 90 degrees and one by 65 degrees (example from above with 875 degrees)? They get the same result as expected. What most of the users expect when calling an animation from 355 to 5 is, that it takes the shortest way. If you want to have a different route, there are three possible ways:
And if this is the best solution, why don't use any of the other maps libraries this? They call it often 'heading' and a heading shouldn't be over 360 degrees. So it will be possible to divide both in Mapsui. Your heading is always between 0 and 360 and gives the actual orientation of the map, but a rotation could be greater than 360 or small than 0. With this, it is clear what the user wants. |
I assume folks have been using maps api, say Google Maps, heading (rotation) seems to be capped to 0-360 range. CW/CCW are derived outcome of using heading, can be extension methods for better devex. |
@peterblazejewicz The google api may still use values outside of the 0-360 range at a lower level. Or perhaps they which they did, because when I was working on the rotation animation bugs things got a lot simpler when I did not try normalize it. Also it allows all options and you can always normalize at a higher level. It is easy to add GetHeading and SetHeading extension methods on the Viewport that work with normalized values. It could use the RotationSnapper.NormalizeRotation method. |
That is much overhead for 99.9 % of the devs to make it easier for the rest 0.1 %. @pauldendulk Did you ever had a use case (in real life or while using a digital map) to have a rotation bigger than 360°? I couldn't find any. Perhaps that the reason, that no other map library does it in this way. |
This is not an effort to make it easy for the 1% percent of developers. This is an effort to keep the Mapsui code simple and straightforward. There were al least two bugs in the pervious code base because it did not deal correctly with jumps over the 0/360 divide. Some were hard to track because they only showed up in some animation. This is core functionality. If we really need this limited to values between 0 and 360 this could be added in a layer on top of that. It can not be done the other way around. |
Mapsui Version
Mapsui 5.x
Mapsui Platform
All platforms
Device
Windows, but the problem is in common code
Describe the bug
If you change the Rotation via Navigator.RotateTo(), then the degrees for rotation could get 360 ° and larger. There is no check for this case.
To Reproduce
Use the "Animated Viewport - Rotate" sample and press the button in the left upper corner for 8 times or more and check the Rotation of Viewport.
Expected behavior
Rotation is in the range of 0 <= Rotation < 360.
Screenshots
The text was updated successfully, but these errors were encountered: