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

Allow rendering of text to collages #118

Closed
jazmit opened this Issue Jan 16, 2015 · 8 comments

Comments

Projects
None yet
4 participants
@jazmit
Contributor

jazmit commented Jan 16, 2015

Could we have a function in the Collage or Text package that allows rendering of text to the 2D canvas?

eg: (if in the Collage library)

fromText : Text -> Shape
fromTextWithMaxWidth : Text -> Float -> Shape

I know it's possible to convert the text first to an Element, then to a Form, which overlays a span on the canvas. However this doesn't support text stroke styles, and scaling can be wonky. I'd also like this so we can have text in WebGL (once we've added a function to elm-gl to convert a collage to a webgl texture).

Am happy to have a stab at doing a pull request for this if folks think it's a good idea?

@TheunisKotze

This comment has been minimized.

Show comment
Hide comment
@TheunisKotze

TheunisKotze Jan 16, 2015

Canvas text would be great.. There seems to be an issue when scaling toForm'ed text, the size of the resulting div is off, resulting in an incorrect final position.

Performance is also quite bad atm, I'm hoping this will help out with that too

TheunisKotze commented Jan 16, 2015

Canvas text would be great.. There seems to be an issue when scaling toForm'ed text, the size of the resulting div is off, resulting in an incorrect final position.

Performance is also quite bad atm, I'm hoping this will help out with that too

@rehno-lindeque

This comment has been minimized.

Show comment
Hide comment
@rehno-lindeque

rehno-lindeque Jan 19, 2015

Contributor

I chatted a bit to Evan about having pure canvas (Form) version of text - and it sounded like he was positive about it. Aside from all of the other advantages, this would also take a lot awkwardness out of use cases like this one: https://groups.google.com/d/topic/elm-discuss/rLwnzss-hBQ/discussion

Contributor

rehno-lindeque commented Jan 19, 2015

I chatted a bit to Evan about having pure canvas (Form) version of text - and it sounded like he was positive about it. Aside from all of the other advantages, this would also take a lot awkwardness out of use cases like this one: https://groups.google.com/d/topic/elm-discuss/rLwnzss-hBQ/discussion

@jazmit

This comment has been minimized.

Show comment
Hide comment
@jazmit

jazmit Jan 19, 2015

Contributor

Opinion so far seems pretty positive, so I've made a start on this:
jazmit@8a2c7f4
We can now render Text to canvas with outline, gradient, fill, etc, but text css properties (weight, font, etc) are currently ignored. I'd love some feedback on the following before I finish it off and submit a PR:

  • To implement this, I changed the Shape type alias to become its own ADT which could break some existing code if it constructed Shapes using the type alias directly rather than the polygon function (as it probably should do). If we want to use the existing filled and outlined functions with text, text must share a type with Shapes, as Elm isn't polymorphic this way. The other option would be to have special filledText and outlinedText functions.
  • To implement the css attributes, it would be best if I refactored the native Text module. It currently builds up styles as a CSS string, which means we'd have to parse CSS in the Collage module to use the values. It would be possibly better to build up styles as a JS object, and only convert to CSS on rendering. Does anyone see any objections to this?
Contributor

jazmit commented Jan 19, 2015

Opinion so far seems pretty positive, so I've made a start on this:
jazmit@8a2c7f4
We can now render Text to canvas with outline, gradient, fill, etc, but text css properties (weight, font, etc) are currently ignored. I'd love some feedback on the following before I finish it off and submit a PR:

  • To implement this, I changed the Shape type alias to become its own ADT which could break some existing code if it constructed Shapes using the type alias directly rather than the polygon function (as it probably should do). If we want to use the existing filled and outlined functions with text, text must share a type with Shapes, as Elm isn't polymorphic this way. The other option would be to have special filledText and outlinedText functions.
  • To implement the css attributes, it would be best if I refactored the native Text module. It currently builds up styles as a CSS string, which means we'd have to parse CSS in the Collage module to use the values. It would be possibly better to build up styles as a JS object, and only convert to CSS on rendering. Does anyone see any objections to this?
@rehno-lindeque

This comment has been minimized.

Show comment
Hide comment
@rehno-lindeque

rehno-lindeque Jan 19, 2015

Contributor

It took me a moment or two to grasp that this commit actually takes a styled Text type instead of a String type - this does seem very useful. I wonder if it's ok to merge this in as is, ignoring the styles applied to the Text. We'd love to use it asap! :)

I was going to say that I wonder if Shape constructors should be hidden from the export list for Collage. I suppose one reason for exporting them might be to allow users to pattern match on their shapes, but using them to construct shapes instead of polygon and text does seem slightly dodgy (I personally don't mind so much provided that polygon and text remain just aliases for the constructors).

Contributor

rehno-lindeque commented Jan 19, 2015

It took me a moment or two to grasp that this commit actually takes a styled Text type instead of a String type - this does seem very useful. I wonder if it's ok to merge this in as is, ignoring the styles applied to the Text. We'd love to use it asap! :)

I was going to say that I wonder if Shape constructors should be hidden from the export list for Collage. I suppose one reason for exporting them might be to allow users to pattern match on their shapes, but using them to construct shapes instead of polygon and text does seem slightly dodgy (I personally don't mind so much provided that polygon and text remain just aliases for the constructors).

@jazmit

This comment has been minimized.

Show comment
Hide comment
@jazmit

jazmit Jan 19, 2015

Contributor

I agree vis. the constructors, but I see that jvoigtlaender is dealing with
the exports and want to keep this PR focused.

James Smith
james@schoolshape.com

On 19 January 2015 at 17:01, Rehno Lindeque notifications@github.com
wrote:

It took me a moment or two to grasp that this commit actually takes a
styled Text type instead of a String type - this does seem very useful. I
wonder if it's ok to merge this in as is, ignoring the styles applied to
the Text. We'd love to use it asap! :)

I was going to say that I wonder if Shape constructors should be hidden
from the export list for Collage. I suppose one reason for exporting them
might be to allow users to pattern match on their shapes, but using them to
construct shapes instead of polygon and text does seem dodgy.


Reply to this email directly or view it on GitHub
https://github.com/elm-lang/core/issues/118#issuecomment-70526681.

Contributor

jazmit commented Jan 19, 2015

I agree vis. the constructors, but I see that jvoigtlaender is dealing with
the exports and want to keep this PR focused.

James Smith
james@schoolshape.com

On 19 January 2015 at 17:01, Rehno Lindeque notifications@github.com
wrote:

It took me a moment or two to grasp that this commit actually takes a
styled Text type instead of a String type - this does seem very useful. I
wonder if it's ok to merge this in as is, ignoring the styles applied to
the Text. We'd love to use it asap! :)

I was going to say that I wonder if Shape constructors should be hidden
from the export list for Collage. I suppose one reason for exporting them
might be to allow users to pattern match on their shapes, but using them to
construct shapes instead of polygon and text does seem dodgy.


Reply to this email directly or view it on GitHub
https://github.com/elm-lang/core/issues/118#issuecomment-70526681.

@jazmit jazmit referenced this issue Jan 19, 2015

Merged

Canvas text #120

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Jan 19, 2015

Contributor

Whether or not to export the Shape constructors here is really Evan's call to make. In all the export-list PRs related to elm/elm-lang.org#173, I was mainly following what intentions were already expressed in the elm-doc style documentations.

Contributor

jvoigtlaender commented Jan 19, 2015

Whether or not to export the Shape constructors here is really Evan's call to make. In all the export-list PRs related to elm/elm-lang.org#173, I was mainly following what intentions were already expressed in the elm-doc style documentations.

@TheunisKotze

This comment has been minimized.

Show comment
Hide comment
@TheunisKotze

TheunisKotze Jan 19, 2015

Nice! Looks good to me.. from a quick look only obvious thing missing is text-alignment (non-issue for me personally)

TheunisKotze commented Jan 19, 2015

Nice! Looks good to me.. from a quick look only obvious thing missing is text-alignment (non-issue for me personally)

@jazmit

This comment has been minimized.

Show comment
Hide comment
@jazmit

jazmit Jan 19, 2015

Contributor

Yeah, I skipped text alignment for now, because the alignment functions in Text all return Element - incorporating could be controversial. Any ideas?

Contributor

jazmit commented Jan 19, 2015

Yeah, I skipped text alignment for now, because the alignment functions in Text all return Element - incorporating could be controversial. Any ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment