search

diagrams / SVGFonts

Fonts from the SVG-Font format
http://hackage.haskell.org/package/SVGFonts
Other
20 stars 12 forks source link

Refactor and fix the Text module, text wrapping #32

Closed p4l1ly closed 4 years ago

p4l1ly commented 4 years ago

Hi,

I have refactored the API and the code a bit and added a text wrapping functionality. Could someone please look at the following ideas and my code contributions before I continue to work on it (document it, etc)?

Text.hs

I have refactored the code of Graphics.SVGFonts.Text module for several reasons:

  1. There was a lot of repeated code.
  2. It was not modular: instead of composing small reusable functions there were big functions with case switches that were solving many elementary problems at once.
  3. TextOpts confusingly contained both width and height but often only one of them was used (according to the Mode).
  4. INSIDE_WH had a hardcoded stretching behaviour, which was even wrong (the advance of the last character was stretched too). I've made the stretching much more configurable (the configuration is a monad, so it may even e.g. fail if the stretching exceeds the acceptable bounds) and implemented two stretching mechanisms:
    • a simple one render_fitRect, which is similar to the previous hardcoded behaviour, but fixes the aforementioned bug
    • space-stretching one render_fitRect', which enables configuring of how much stretching should be applied to spaces in comparison to the other characters.

As the code is now modular, it is much more flexible than before: one may use the predefined text rendering pipelines or compose custom pipelines from the predefined building blocks.

Wrap.hs

With use of the refactored Text module, I have implemented configurable text wrapping. One may specify mechanisms like: Try to split the text at spaces but if there are too long words and it cannot be split nicely, try to split it at syllables; if this splitting is also unsuccessful, then fail. The configuration is specified again by a monad, so we may e.g. ask user to help us splitting, or we may access dictionaries with syllabically split words, etc.

Docs

The code is still documented only by examples. I will document it properly if you like the idea.

Terminology

I'm not sure about the consistency of the terminology (e.g. "render") with the rest of the diagrams lib. If you come up with more convenient function names, I'm open to change them. What I'm sure of is that I didn't like the old terminology textSVG + a random unmeaningful character.

Backward Compatibility

This pull request is not backward compatible. A backward compatibility layer can be implemented if you think it's a good idea.

tkvogt commented 4 years ago

This is really good. I feel a little bit embarassed that I didn't do it like this from the beginning. For backward compatibility I would keep the short textSVG, like stroke $ textSVG "Hello World" 1 What don't you like about the name? Maybe it should be textNative to differentiate it from text The longer versions textSVG_ and textSVG' should be marked deprecated or just deleted in my opinion. What about renderText instead of render. render is so generic that it may always be imported qualified.

p4l1ly commented 4 years ago

I'm really glad you like it, you made my day!

I'm OK with the renderText name. Me personally, I like qualified imports (I don't see much advantage of renderText over Text.render) but I understand that it's not convenient for every library. So, let's stick with something less general: renderText or maybe textDiagram or textPath or svgText, because the term "render" in the diagrams library is used for the final export.

How about the bounds and BoundedPath? Is the approach diagram-ish enough? Maybe bounds could be attached to the path as an envelope already in the beginning (render would return a QDiagram). But I didn't find a way how to drop the envelope, if one would want to convert the QDiagram back to path (to get the effect of the old textSVG' function). I'm not yet very familiar with the diagrams library but maybe you could come up with a way or explain why it is not possible.

You asked me what I don't like about the names. I was OK with the textSVG name, I just didn't like the textSVG' and textSVG_ names because I lacked some convention in naming. They don't say anything about what the functions do, only that it is something like textSVG but somehow different. But now when I think about it, I can see that maybe there was some convention: ' could mean "center origin" and _ could mean "add envelope". Their combination would be maybe '_? OK, too many talking about something which is not useful nor applicable anymore, after we finish and merge this.

p4l1ly commented 4 years ago

Ok, I think I can answer the second paragraph of my last comment (correct me if I'm wrong). QDiagram is too abstract, it holds the underlying path only somewhere in closure, therefore stroke is not revertible (which is good). Anyway, BoundedPath should be a data or at least a newtype and it should be called TextSVG, or maybe, what I like more, SvgText and various class instances from diagrams should be set and set_envelope should be renamed to strokeSvgText and drop_bounds to something like pathOfSvgText (this mixedCase look sometimes so ugly, I think it was a wrong choice of convention for such a beautiful language...).

byorgey commented 4 years ago

Yes, that's right, it is not possible to recover a Path after converting it to a QDiagram.

p4l1ly commented 4 years ago

I've adjusted the terminology and the docs. From my point of view and from what we've agreed upon here, it's ready to be merged. Could you please check it?

tkvogt commented 4 years ago

Thank you for this contribution. Can you adjust the diagrams manual in https://github.com/diagrams/diagrams-doc ?

byorgey commented 2 years ago

By the way, I know it has been over a year, but I just released SVGFonts-1.8 to Hackage which includes these changes.

expipiplus1 commented 2 years ago

Is there any kind of migration guide for this change?

(Specifically for the problems encountered here: https://github.com/timbod7/haskell-chart/issues/232). I'm not familiar with any of the libraries involved, and it's not really obvious to me what changes to make.

byorgey commented 2 years ago

Good question! There doesn't currently exist a migration guide, but I will also soon be updating some code in https://github.com/diagrams/diagrams-doc and I can write up what I learn and post it here.

byorgey commented 2 years ago

@expipiplus1 , I've written up a migration guide which for now you can find in the SVGFonts README on GitHub: https://github.com/diagrams/SVGFonts/#porting-to-version-18