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

New "Emoji" object in editor #358

Open
wants to merge 64 commits into
base: release/1.3
Choose a base branch
from

Conversation

jairbubbles
Copy link

@jairbubbles jairbubbles commented Jan 9, 2022

This allows to quickly insert emojis in the editor.

  • Emoji editor is based on Emoji.WPF so a bit of interop is needed to get it working in the Winforms editor. Kudos to @samhocevar who made all the hard work!
  • SixLabors.ImageSharp.Drawing is used to do the rendering. Thx @JimBobSquarePants!
  • It's using Twemoji font packaged by Mozilla
  • The combination of all possible emojis is taken from https://unicode.org/Public/emoji/14.0/emoji-test.txt
    • A custom MsBuild task is used to convert it to a simplified .xml
  • Installer size is growing from 4,10MB to 5,29 MB

Screenshots

image

image

TODO:

  • Disable feature when "Segoe UI Font" is not available (for Windows 7 / 8)
  • Fix double-click for picking emoji (modifiction needed on Emoji.WPF)
  • Handle rotation image perfectly
  • Check performance (we create several bitmaps during the drawing)
    • Maybe add an image cache?
  • Polish button to add an emoji (icon / translation)
  • Reduce installer size

@mischlrebl
Copy link
Contributor

looks very interesting and promising!
could you provide an executable for testing? that would be great :-)

@jairbubbles
Copy link
Author

looks very interesting and promising! could you provide an executable for testing? that would be great :-)

Sure! 👉🏼Greenshot_emoji.zip 👈🏼

@Lakritzator
Copy link
Member

This is a great PR, I really like the result! I mean REALLY nice work!!!!! ❤️
The code is very good too, follows all the current styles and ways to implement (which I do not even like myself).

I currently have only two things which are a challenge:

  1. Translations, for Greenshot to have a new text we unfortunately have to go through a lot of hoops. We can do a few ourselves, but we have to hope we can find people supporting us with the other ones.
  2. I was a bit confused about how I can define the emoji to use, this might cause issues in our issue tracker. It wasn't completely intuitive.

I'm not sure about what to do about the second item...

@Lakritzator
Copy link
Member

If I double click, the emoji has a black area on the right side, and the picker is short first when you click again.
Did you create an issue with Emoji.Wpf? Maybe you can link it here, if so.
It might also be an issue with the WPF / Forms interop.

@jairbubbles
Copy link
Author

If I double click, the emoji has a black area on the right side, and the picker is short first when you click again. Did you create an issue with Emoji.Wpf? Maybe you can link it here, if so. It might also be an issue with the WPF / Forms interop.

Yes the edition is buggy, I'll dig deeper as it seems you have interest in the feature ;-)

(The picker is a WPF button that displays a popup on click. So for now you need to click 3 times. As for the black background it's probably easy to fix)

@Lakritzator
Copy link
Member

I'm definitively interested in this, I've been thinking about providing something like it via a collection of SVGs (it has to be vector graphics due to the scaling). This is especially needed for things like checkboxes etc., but also for emoji.

The other containers have settings, like color, like thickness etc., in the toolbar under the destinations. I think for it to be consistent with the others, it would make sense to have it the same way. The double click is not directly bad, but it's a deviation to how others work. So if possible, we should provide a way to configure it in the toolbar. This would also solve the location issue with the selection.

About the translations, if we just call it "Emoji", instead of "add emoji" we might not need any. This would solve a lot of troubles.

@jairbubbles
Copy link
Author

The double click is not directly bad, but it's a deviation to how others work.

Well it's similar to what is done with text edition. Also, it was easier for me to implement 😅

About the translations, if we just call it "Emoji", instead of "add emoji" we might not need any. This would solve a lot of troubles.

Consider it done.

@Lakritzator
Copy link
Member

I agree for the text, as this is pretty much "inline" but providing a drop down or other window (like the color picker) is always done via the toolbar.

I had a quick look at the license of the other project, this is always a very important thing... they need to fit.
There is no issue there, so from that perspective we have a go.

Some considerations I do have:

  • It's build for .NET Framework 4 and .NET Core 3.1, so it could need some .NET 6 loving to be able to use it in the near future. I might have a look at providing a PR for that project.
  • I am looking at what technologies I might use for Greenshot 2, and WPF is not yet completely off the table, but it's close. I was considering Avalonia, and if this is the case, I might not be able to provide a similar experience, which will upset our users.

@jairbubbles
Copy link
Author

jairbubbles commented Jan 14, 2022

I updated the PR with a few changes.

I improved a lot the edition. Now it will display the popup by default when you create an Emoji and then you need to use the double-click. Let me know what you think.

I had to make a very simple change in Emoji.WPF so it won't compile out of the box, you can test on those binaries: Greenshot_emoji_v2.zip

I was considering Avalonia, and if this is the case, I might not be able to provide a similar experience, which will upset our users.

While testing Lunacy it looks like the font system supports colored emoji by default so it should be easy. For the edition the best is probably to use the Windows Emoji picker but you need the app to target a recent windows SDK. And anyway the custom control in Emoji.WPF is not very complex.

@Lakritzator
Copy link
Member

I just noticed that I also need to check the other dependencies, there seem to be more and I am not sure where they come from...
e.g. Stfu.dll

@jairbubbles
Copy link
Author

jairbubbles commented Jan 14, 2022

I just noticed that I also need to check the other dependencies, there seem to be more and I am not sure where they come from...
e.g. Stfu.dll

that's weird, I didn't include it in the installer and it's working. It's used in a few places in the .xaml for Picker.

https://github.com/samhocevar/emoji.wpf/blob/main/Emoji.Wpf/Picker.xaml#L7

Anyway it's from the same author and the licence is quite permissive: https://github.com/samhocevar/stfu/blob/main/COPYING

@jairbubbles
Copy link
Author

New update, I fixed most of the issues I have in mind:
Greenshot_emoji_v3.zip

@jairbubbles
Copy link
Author

jairbubbles commented Jan 14, 2022

Final tweaks on my side, I'm undrafting the PR as I'm happy with the result and the code.

  • No tweak is needed anymore on Emoji.WPF (I look at the children to find the toogle button and check it).
  • @Lakritzator Are you ok with the icon I used in the toolbar?
  • It works with the installer too
  • The button is displayed if the font is installed

Greenshot_emoji_v4.zip

@Lakritzator
Copy link
Member

I just noticed that I also need to check the other dependencies, there seem to be more and I am not sure where they come from...

e.g. Stfu.dll

that's weird, I didn't include it in the installer and it's working. It's used in a few places in the .xaml for Picker.

Anyway it's from the same author and the licence is quite permissive: https://github.com/samhocevar/stfu/blob/main/COPYING

Unfortunately it's not just about licenses, every dependency brings more work maintaining it and restricts my future work. If possible I prefer it without, if every PR would do so Greenshot would be a DLL hell on its own. I also still need to check the others, those with the glyphs.

@wskellenger-intrepid
Copy link

A lot of work sitting here -- would be nice to see it merged in. :-)

@jairbubbles
Copy link
Author

You're right @wskellenger-intrepid, it's been a while now but I think we were not that far away from merging.

I think we were trying to reduce the impact on the installer size. Embedding the font + the list of all emojis was increasing a lot the installer size.

@jairbubbles
Copy link
Author

@Lakritzator I updated to emoji 14.0!

@jairbubbles
Copy link
Author

jairbubbles commented Aug 12, 2023

I also checked the latest installer size, it's adding a little more than 1 MB :

image

The new files are:

image

EDIT: I removed the .gzip compression and it's actually improving the installer size:

image

@jairbubbles
Copy link
Author

I was playing with the emoji before, and noticed that I can drag and increase the size of the emoji without losing the aspect ratio, but when I reduce the size I seem to be able to drag it however I like and destroy the aspect ratio. Did you ever see this? I was just wondering if this is new or it was already like that...

I just fixed it, it was a typo in your code.

@jairbubbles jairbubbles marked this pull request as ready for review August 12, 2023 15:14
@mischlrebl
Copy link
Contributor

I was playing with the emoji before, and noticed that I can drag and increase the size of the emoji without losing the aspect ratio, but when I reduce the size I seem to be able to drag it however I like and destroy the aspect ratio. Did you ever see this? I was just wondering if this is new or it was already like that...

I just fixed it, it was a typo in your code.

The same problem exists with rectangle - and other objects I guess.
Hope, your fix adresses these too, thanks for that!

@jairbubbles
Copy link
Author

The same problem exists with rectangle - and other objects I guess.
Hope, your fix adresses these too, thanks for that!

You mean when pressing Shift while resizing? By default, rectangles can be resized without any constraint.

@mischlrebl
Copy link
Contributor

The same problem exists with rectangle - and other objects I guess.
Hope, your fix adresses these too, thanks for that!

You mean when pressing Shift while resizing? By default, rectangles can be resized without any constraint.

Correct, when pressing Shift.
Same happens with ellipses.

@jairbubbles
Copy link
Author

It's fixed too yeah, should I create another PR with just that fix? It's a one liner (see f6ca02b)

@JimBobSquarePants
Copy link

This looks like it's approved but don't merge it yet. There's a very strong chance I'm launching a stable version of ImageSharp.Drawing today.

@jairbubbles
Copy link
Author

This looks like it's approved but don't merge it yet. There's a very strong chance I'm launching a stable version of ImageSharp.Drawing today.

Oh that's cool, I saw that you just released SixLabor.Fonts as 1.0.0. Will you update to ImageSharp 3.0.x? If so, Greenshot is still on net472.

@JimBobSquarePants
Copy link

You're safe. v1.x of ImageSharp.Drawing will be compatible with ImageSharp v2.x only so will support net472.

@mischlrebl
Copy link
Contributor

It's fixed too yeah, should I create another PR with just that fix? It's a one liner (see f6ca02b)

I do not know, sorry.
Thanks for all anyway!

@jairbubbles
Copy link
Author

jairbubbles commented Aug 26, 2023

It's fixed too yeah, should I create another PR with just that fix? It's a one liner (see f6ca02b)

I do not know, sorry. Thanks for all anyway!

I created #514 anyway, that might get merged quickly.

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

Successfully merging this pull request may close these issues.

None yet

8 participants