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.

Sign up for GitHub

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

Jump to bottom

Rotate labels #54

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open

Rotate labels #54

wants to merge 16 commits into from

Conversation

jswift-stfc
Copy link
Contributor

@jswift-stfc jswift-stfc commented May 20, 2022 •
edited by trisyoungs

Relatively small PR that enables a TextEntity to be rotated. This is aimed in particular at Axis Titles.

  • Added TextAlignment enum for defining rotation of text (currently horizontal / vertical).
  • Added setter for text alignment in TextEntity class.
  • When updating the translation, the rotation is applied to the positional transform.
  • Added setter for axis label and tick label alignment in AxisEntity class.
  • Updated basic example to showcase rotation of axes labels.

Current issues:

  • Bounding boxes are not quite correctly calculated, so extra space appears, for example, below the x axis.

Closes #13.

@jswift-stfc jswift-stfc added the enhancement New feature or request label May 20, 2022
@jswift-stfc jswift-stfc self-assigned this May 20, 2022
trisyoungs
trisyoungs requested changes May 20, 2022
View reviewed changes
Copy link
Member

@trisyoungs trisyoungs left a comment

Choose a reason for hiding this comment

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

This is definitely heading down the right lines, but one crucial thing is missing - the rotation of the label isn't accounted for in the bounding cuboids, which means that the layout is generated incorrectly. To illustrate this, run the basic example, set the Y-axis title to "Vertical", and put in a long title. It's clear that the graph area is reacting to an increase in only the horizontal (un-rotated) size of the label.

src/entities/text.cpp Outdated Show resolved Hide resolved
src/entities/text.h Outdated Show resolved Hide resolved
src/entities/axis.cpp Outdated Show resolved Hide resolved
@jswift-stfc
Copy link
Contributor Author

This is definitely heading down the right lines, but one crucial thing is missing - the rotation of the label isn't accounted for in the bounding cuboids, which means that the layout is generated incorrectly. To illustrate this, run the basic example, set the Y-axis title to "Vertical", and put in a long title. It's clear that the graph area is reacting to an increase in only the horizontal (un-rotated) size of the label.

Oops! Yes you are right - I hadn't thought about that.

@jswift-stfc
Copy link
Contributor Author

Regarding this, at which point do you think the rotation should be performed? In reference to the boundingCuboid function i.e. during the construction of the basic cuboid, or when returning the final cuboid etc.

@trisyoungs
Copy link
Member

I think I'd be tempted to add another argument to the second boundingCuboid() function (the one which currently takes the anchor point) and do the rotation there. The rotation centre should be the anchor point, incidentally.

jswift-stfc and others added 13 commits August 28, 2022 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trisyoungs trisyoungs trisyoungs requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rotation of axis labels
2 participants
@jswift-stfc @trisyoungs