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

Tracking bugs and features #8

Closed
8 tasks done
teunbrand opened this issue Nov 18, 2021 · 12 comments
Closed
8 tasks done

Tracking bugs and features #8

teunbrand opened this issue Nov 18, 2021 · 12 comments

Comments

@teunbrand
Copy link
Collaborator

teunbrand commented Nov 18, 2021

It's good that we have identified some issues and ideas that we want to keep track of, but these are currently mentioned all over the place. I thought it could be useful to have a checklist issue to keep track of bugs we're aware of. Plus, it is very satisfying to check off things from a checklist. Feel free to comment additional ones as you encounter them.

Bugs

Features

@AllanCameron
Copy link
Owner

The latest commit should fix the curvature / spacing problem. It turns out that the spiral is a really useful test - you should notice that this looks nicer at all aspect ratios now if you look at the updated readme.

I have ticked this off the above list, though I wonder if once we are done with the above problems we should just make a separate issue for each.

@AllanCameron
Copy link
Owner

We should now have line breaks implemented:

library(geomtextpath)
#> Loading required package: ggplot2

spiral <- data.frame(x = rev(sin(seq(0, 5*pi, length.out = 1000)) * 1000:1),
                     y = rev(cos(seq(0, 5*pi, length.out = 1000)) * 1000:1),
                     s = seq(1, 10, length.out = 1000),
                     z = paste("Like a circle in a spiral, like a",
                               "wheel within a wheel, never ending",
                               "or beginning on an ever spinning reel"))

spiral$z <- paste(spiral$z, spiral$z, sep = "\n")

ggplot(spiral, aes(x, y, label = z)) +
  geom_textpath(size = 5, linewidth = 0, vjust = 2) +
  coord_equal(xlim = c(-1500, 1500), ylim = c(-1500, 1500))

library(geomtextpath)
#> Loading required package: ggplot2

 ggplot(within(iris, Species <- paste("This is", Species, sep = "\n")), 
        aes(x = Sepal.Length, color = Species)) +
  geom_textpath(aes(label = Species),
                size = 5, stat = "density",
                fontface = 2, hjust = 0.1, vjust = 0.9)

I've had to split the lines into different groups, each with their own vjust to get this to work. This means that each path length is calculated separately, and that can result in the upper and lower lines not centering well with each other the whole way along a path. I'll work on fixing this, though to be honest it's not very aesthetically pleasing to have two or more lines bent along a path.

@AllanCameron
Copy link
Owner

Tracking is now exposed to end users as a "spacing" parameter in geom_textpath:

library(geomtextpath)
#> Loading required package: ggplot2

ggplot(iris, aes(x = Sepal.Length, color = Species)) +
  geom_textpath(aes(label = Species),
                size = 6, stat = "density",
                fontface = 2, hjust = 0.2)

 
 
ggplot(iris, aes(x = Sepal.Length, color = Species)) +
  geom_textpath(aes(label = Species),
                size = 6, stat = "density",
                fontface = 2, hjust = 0.2, spacing = 100)

ggplot(iris, aes(x = Sepal.Length, color = Species)) +
  geom_textpath(aes(label = Species),
                size = 6, stat = "density",
                fontface = 2, hjust = 0.2, spacing = 1000)

  
ggplot(iris, aes(x = Sepal.Length, color = Species)) +
  geom_textpath(aes(label = Species),
                size = 6, stat = "density",
                fontface = 2, hjust = 0.2, spacing = -100)

Created on 2021-11-18 by the reprex package (v2.0.0)

@teunbrand
Copy link
Collaborator Author

Through a stroke of luck in #9, flipping labels are fixed:

library(geomtextpath)
#> Loading required package: ggplot2
df <- data.frame(x = 1:1000, y = 1, text = "This is a perfectly flat label")

ggplot(df, aes(x, y, label = text)) +
  geom_textpath(size = 6) +
  coord_polar(start = pi)

Created on 2021-11-18 by the reprex package (v1.0.0)

@AllanCameron
Copy link
Owner

Sorry about all the automation fails Teun. I now have it so that the readme updates automatically - it's actually quite useful as a visual test following a commit, and I was finding it tiresome to manually knit the rmd every time I committed.

@teunbrand
Copy link
Collaborator Author

Nice work with all the tracking, line breaks and automation Allan! Everything seems to be coming along very smoothly!

@AllanCameron

This comment has been minimized.

@teunbrand teunbrand mentioned this issue Nov 19, 2021
@teunbrand
Copy link
Collaborator Author

teunbrand commented Nov 22, 2021

It would be nice if we can have a halign parameter a la {ggtext} for how to handle horizontal justification of multi-line strings.

Notice that in the example below, the textbox placement relative to the reference points (red) is controlled by the hjust parameter, whereas the text justification within the box is controlled by the halign parameter.

library(ggtext)
library(ggplot2)

df <- expand.grid(
  x = c(0, 0.5, 1),
  y = c(0, 0.5, 1),
  text = "This is a<br>multi-line<br>string for<br>demonstration."
)

ggplot(df, aes(x = x, y = y, label = text)) +
  geom_textbox(
    aes(hjust  = x,
        halign = y),
    width = unit(1.5, "inch")
  ) +
  geom_point(colour = "red") +
  labs(x = "hjust", y = "halign") +
  scale_y_continuous(expand = c(0.2,0))

Created on 2021-11-22 by the reprex package (v2.0.1)

@AllanCameron
Copy link
Owner

Yes, I'd had the same thought. The multi-line text support seems disproportionately complex to get right. I'm currently working on a method for allowing flipping of upside-down text, which might make it even harder...

@teunbrand
Copy link
Collaborator Author

teunbrand commented Nov 22, 2021

I think the multi-line support could be relatively straightforward by taking the y_offset parameter from systemfonts::shape_string(vjust = original_vjust), but it'd take me some time to refactor stuff exactly right. For example, the anchorpoint of the text should be different for each line, and the distance between subsequent glyphs should be preserved, so we can't simply project/offset to the adjusted path. The halign could then just be implemented as the shape_string(hjust = halign). I might need to make a separate branch to experiment with this so I don't get a screwed up repo when I commit stuff.

@AllanCameron
Copy link
Owner

It could maybe be easier this way, but it's not the calculation of the correct y position that's the difficult part (I think I have this correctly calculated from the number of lines of text, the vjust and the lineheight).

The difficult part is that the curvature is different for different lines of text. To preserve the correct letter spacing in different lines we need to work out the curvature for each line of text and correct for it.

Imagine the text is on the inside of a circle. If you decrease the vjust from 0 to -1, the text moves towards the centre of the circle, and the string squashes in, so you need to account for that by calculating an "adjusted length" - this is the length of the offset path at vjust = -1. That's what we currently do with the curvature calculation. For multi-line text, the natural way to do this is by breaking up the text line by line and calculating the path for each line separately, which is what happens currently.

You may find a creative way round this, but my instinct is that it's inherently complicated. I certainly had a few false starts trying to get it right in the first place.

@AllanCameron
Copy link
Owner

The bugs in this issue are fixed, and the feature requests are now implemented.

teunbrand added a commit that referenced this issue Dec 6, 2021
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

No branches or pull requests

2 participants