-
Notifications
You must be signed in to change notification settings - Fork 17
Added SurfaceText rotation #40
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
Added SurfaceText rotation #40
Conversation
|
Eugene (Sufaev), this one is for you! |
wcmatthysen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Eugene,
I made a couple of suggestions regarding this commit. If you look at SurfaceQuad as an example you'll see that the angle is called "heading" and the type is Angle. Could you make these changes to fit in with the convention already established in WorldWind?
Btw. I'm looking into that chopped-off text issue. I'm also seeing it on my side.
|
I think I found the issue with the text being cut off like that. The bounding-sector calculation should be updated to take the rotation into account. If you rotate the text then the bounding-sector should enlarge if rotated at 45-degrees. If the bounding-sector is too small then the cut-off will occur. To see the bounding-sector for the This will cause a white rectangle to be drawn around it that looks like: You can see from image that the rectangle is not rotated. This causes the clipping issue. To rotate the rectangle, you need to modify the The fix was done by borrowing some logic from However, the bounding-sector is not 100% accurate. If you set the heading to 0-degrees so that the text is not rotated, you get a bounding-sector that looks like: If you remove the rotation fix for the bounding-sector you'll see that the bounding-sector for a 0-degrees heading rotation is perfectly calculated (it fits perfectly around the text). I'm not too sure what the implications of this is. If @emxsys can confirm, I think this is good enough and you can use the fix. |
|
I have changed attribute name and type to |
|
Hi @Sufaev, I'll have a look for you as well. I think the offset needs to be taken into account when calculating the bounding-sector. |
|
@Sufaev, I looked a bit at the offset calculation. The offset-constants specified in the whereas if you look at the default offset of This means that the offset for That is why you were getting strange results when you used |
|
@Sufaev, I found an issue with the rotation logic. You need to modify This ensures that the text is rotated about its center-point before being offset. Then, if I rotate 45-degrees and I offset so that the text is left-aligned and centered vertically: you will get the following bounding-sector: This is very close to what we want. I'm still having issues with the slightly larger bounding-sector. Thus, if I rotate 0-degrees (and left-align as before) I still get the following: I'm currently taking a look at that. |
|
I think the bounding-sector overshoot happens because of the way the path-length is calculated in the If we change this to: You get a bounding-sector that fits more snugly around the text again: But, if you rotate the text, the fit is too snug again and you get this: Correct me if I'm wrong @emxsys, but I think the slightly larger bounding-sector won't cause any noticeable problems? On the other hand, I think a bounding-sector that is too small will cause text to be clipped like we saw initially. |
- Modified the SurfaceText class by first, removing the offset calculation in the drawText() method. We want the text to be drawn normally without any offsets added at this point. Then, we modified the applyDrawTransform() method by moving the text to the center, applying the rotation, moving the text back to the offset position. A new private method called getRotatedTextDimensions() was added. This method calculates the pixel-dimensions of the rotated text. Changed the cumputeSector() method to take the rotated-text dimensions into account when calculating the bounding-sector. - Changed the SurfaceTextUsage class by changing the code to add a lot of SurfaceText objects. We iterate over all of the different position-offsets as well as incrementing the angle by 30-degrees so that we can see the text rotation together with the bounding-box. The position of the surface-text is drawn with a red-dot for debugging purposes (we use a placemark for this).
|
@Sufaev, I managed to fix the If you pull the latest changes I made in your repository you'll see this when you run I iterate over 30-degree angle intervals and over all offsets. The location of each |
|
Thanks. Looks perfect! Can we merge this solution to develop? |
|
@Sufaev, I'm just moving the changes in |
Reverted the SurfaceTextUsage changes back to its original state. The tests are going to be moved out to a new functional test class.
Added a new class called SurfaceTextTest that contains the functional tests for the SurfaceText class. This class can be run to visually inspect whether the SurfaceText rotations and offsets are correctly calculated as well as whether the text is contained within the bounding-sector.
|
OK, I've moved the test class out to the @emxsys, I think we can merge this into the development branch. If you can just cross-check and confirm that everything is in order. |
|
@wcmatthysen @Sufaev I'll perform a sanity check and merge the changes today. |
emxsys
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Sufaev and @wcmatthysen Great collaboration! I set the pull request title/description set to SDK standards.











Description of the Change
Added the capability to rotate
SurfaceTextobjects.headingproperty to theSurfaceTextclassWhy Should This Be In Core?
Enhances the SDK's labeling features.
Benefits
This improvement adds
SurfaceTextrotation capability.Potential Drawbacks
Note: The following truncation issue was resolved through collaboration with @wcmatthysen on this pull request.
Initially, the only issue found - big texts are truncated by tile borders.

Note: See the conversation on this pull requests for more information on the fix.
Applicable Issues
Closes #37