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

Add "text as image" node #2125

Merged
merged 16 commits into from
Aug 28, 2023
Merged

Add "text as image" node #2125

merged 16 commits into from
Aug 28, 2023

Conversation

adegerard
Copy link
Contributor

Motivation
This node transforms a text into an image.
It creates an image and draw the text with the specified alignement
It calculates the font size to fit the canva: width and height input values.

text_as_image_example

Choices and limitations

  • Font style (regular, blod, italic): via a dropdown.
  • Text alignement via a dropdown
  • Position in the canva via a dropdown

As discussed on Discord, would be great to change the dropdown by grids of 'checkable buttons' (checkboxes)

For better user experience, PR #2087 should be merged in main

@joeyballentine
Copy link
Member

This seems fine to me, but I'll let @RunDevelopment confirm

@joeyballentine
Copy link
Member

I think this node probably fits better in the "Create Images" group in the "Images" category, since it involves image creation. Thoughts?

@adegerard
Copy link
Contributor Author

adegerard commented Aug 21, 2023 •
edited
Loading

I don't know because, to me, it would be more a converter, however, in chaiNNer, I would agree with you to put it in "create images" group. It makes more sens.

What about node title? "Create from text" ? it might be confused with IA...

@RunDevelopment
Copy link
Member

Is there no better way than to vendor fonts? Can't we use fonts installed on the current system?

@joeyballentine
Copy link
Member

If there's no built in way, we could probably just manually scan the system fonts folder and build a list of all the available valid fonts

@adegerard
Copy link
Contributor Author

adegerard commented Aug 21, 2023 •
edited
Loading

Because I think it would be a nightmare.
My thoughts:

  • (Please have a look to) C:\Windows\Fonts (more than 200 fonts on my computer) or /usr/share/fonts/ and ~/.local/share/fonts on Linux
  • Only ttf fonts are loadable by PIL. (freetype also but let's forget it)
  • Some are static fonts, other dynamic fonts: it is not always possible to differentiate style: bold/italic/light/black by filename.
  • A dropdown of 200 elements is not understandable, especially if there is no preview (like in ms word)
  • I tried to use dynamic fonts but had a problem with the index no. (which has to be used to indicate to PIL which style you'd like to load)
  • If you share your project across different OS, or if you have some sw which installs specific fonts (like adobe), you could have trouble with font resource/name/path

@joeyballentine
Copy link
Member

Ok yeah, you've convinced me

@adegerard @joeyballentine
Update backend/src/packages/chaiNNer_standard/image/create_images/tex…
77cd085 
…t_as_image.py

Co-authored-by: Joey Ballentine <34788790+joeyballentine@users.noreply.github.com>
@RunDevelopment
Copy link
Member

Hmmm, yeah, that certainly will be difficult. But having 3 font families to pick from and having those fonts be vendored just doesn't sit right with me.

I mean, what's the point of having a font selector if there is no real choice? So how about we just remove the font selector for now and just use one. Font style (bold, italic, etc.) could be its own input.

@adegerard
Copy link
Contributor Author

Hmmm, yeah, that certainly will be difficult. But having 3 font families to pick from and having those fonts be vendored just doesn't sit right with me.

I mean, what's the point of having a font selector if there is no real choice? So how about we just remove the font selector for now and just use one. Font style (bold, italic, etc.) could be its own input.

Thanks for your feedback.
It's ok with me. KISS principle.

adegerard and others added 2 commits August 23, 2023 19:00
@adegerard
Merge branch 'chaiNNer-org:main' into text_as_image
a2245fd
@adegerard
Simplify node:
158a78f 
- removed font selection
- added bold/italic checkbox
- improved test project
@adegerard
Copy link
Contributor Author

I made a version without font selection.
There are now 2 checkboxes to select font style.

image

Like other parameters, it would be great to convert them to "buttons".

adegerard and others added 2 commits August 26, 2023 18:47
@adegerard
Merge branch 'chaiNNer-org:main' into text_as_image
a9be371
@adegerard
Modifications:
5ca8b2c 
- Changed ouptut image as RGBA only
- Change default width to 500: better UX when using default node values
RunDevelopment
RunDevelopment requested changes Aug 27, 2023
View reviewed changes
Copy link
Member

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

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

2 other minor things.

backend/src/packages/chaiNNer_standard/image/create_images/text_as_image.py Outdated Show resolved Hide resolved
backend/src/packages/chaiNNer_standard/image/create_images/text_as_image.py Outdated Show resolved Hide resolved
adegerard and others added 3 commits August 27, 2023 14:47
@adegerard @RunDevelopment
Update backend/src/packages/chaiNNer_standard/image/create_images/tex…
00c7da1 
…t_as_image.py

Co-authored-by: Michael Schmidt <msrd0000@gmail.com>
@adegerard @RunDevelopment
Update backend/src/packages/chaiNNer_standard/image/create_images/tex…
709a33d 
…t_as_image.py

Co-authored-by: Michael Schmidt <msrd0000@gmail.com>
@adegerard
Modifications as suggested, ran validation tests.
95354b3
@adegerard
Copy link
Contributor Author

2 other minor things.
Done with some corrections.

@adegerard adegerard closed this Aug 27, 2023
@adegerard
Copy link
Contributor Author

wrong manipulation, sorry.

@adegerard adegerard reopened this Aug 27, 2023
@joeyballentine
Copy link
Member

I'll play around with this again today and then code review it, but I'm guessing it's probably good now 👍

RunDevelopment
RunDevelopment approved these changes Aug 28, 2023
View reviewed changes
Copy link
Member

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

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

Thanks @adegerard!

@joeyballentine joeyballentine merged commit a45a2d0 into chaiNNer-org:main Aug 28, 2023
@adegerard adegerard deleted the text_as_image branch September 13, 2023 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joeyballentine joeyballentine joeyballentine left review comments

@RunDevelopment RunDevelopment RunDevelopment approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@adegerard @joeyballentine @RunDevelopment