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

emoji background doesn't change with faces #7

Closed
ryanprior opened this issue Nov 15, 2015 · 14 comments
Closed

emoji background doesn't change with faces #7

ryanprior opened this issue Nov 15, 2015 · 14 comments
Labels

Comments

@ryanprior
Copy link

Perhaps we can use transparent images or regenerate with backgrounds for different faces.

emojify
^shows the problem: the blue portion is highlighted because the text is selected, the light gray portion because it's the current line.

@iqbalansari
Copy link
Owner

Hi, thanks for the bug report. I think I have a solution for this. Just to properly debug this, what is the theme you are using, the effect is not noticeable in the themes I use (solarized).

@ryanprior
Copy link
Author

I'm using monokai in the screen shot above. I just tried switching to the 'solarized-dark' and 'tango' themes, both of which seem to have the same issue.

@iqbalansari
Copy link
Owner

Actually I meant the effect is not as pronounced as the screenshot above, I agree that it is present. Just pushed a possible fix, could you please try (when MELPA updates)?

@ryanprior
Copy link
Author

The display is much improved and mostly fixed, but it seems like maybe only the fully transparent portions of the emojis are affected.

emojify
^notice the pixels closest to the emoji which appear to be anti-aliased against the darker background

@iqbalansari
Copy link
Owner

I think that is a problem in the images themselves, I will try to fix this tonight

@iqbalansari
Copy link
Owner

This seems to be an issue with images. The images bundled with emojify are resized (24px) versions of the ones from emoji-one this was done to mitigate the inability to resize images on Emacsen compiled with no imagemagick support. I tried using original images from emoji-one and they look better than the resized ones (though not perfect), have a look

screenshot from 2015-11-22 22 02 48

The first emoji is rendered using the resized version bundled with emojify the second is rendered using the original image from emoji-one.

I hope to solve this indirectly by implementing #4 (which I will get to soon) and then (using your idea of different image sets) allowing users to download unresized images if they prefer.

@iqbalansari
Copy link
Owner

On an unrelated note, thanks a lot for all your inputs on different issues, much appreciated 😄. Thanks!

@ryanprior
Copy link
Author

Perhaps we can test for imagemagick support and create pixel-perfect renditions when it's available.

Thank you for putting your effort into creating a great emoji experience in Emacs. You're doing a great job with this package and it's my pleasure to support you.

@iqbalansari iqbalansari removed the done label Nov 27, 2015
iqbalansari referenced this issue Dec 1, 2015
This allows easily executing form for emojis in given region
@iqbalansari
Copy link
Owner

Hi,

Just wanted to post an update on this. I initially came up with a specific solution that solved the problem only for emojis in regions that code is in feature/better-backgrounds branch, a more general solution for the problem is in exp/aggressive-bg-face branch which works for any background face. My only concern is efficiency of updating emoji backgrounds (which I am doing in a post-command-hook), on my old laptop Intel Core i3 processor and 4 GB RAM it around 0.000660s - 0.001609s seconds to update backgrounds of visible emoji in the data/emoji.json file, which I think might be efficient enough. What do you think?

I will be testing it over the week for any bugs or performance issues, if all goes well I will merge this to master.

@iqbalansari
Copy link
Owner

Hi @ryanprior,

Did you get a chance to test the branch? I personally haven't come across any regressions and plan to merge exp/aggressive-bg-face tomorrow.

@ryanprior
Copy link
Author

I haven't had the chance to test yet. Go ahead and merge, I'll report any bugs I find!

@iqbalansari
Copy link
Owner

I have merged the branch to master, hopefully the issues you have mentioned should have been fixed. Could you please try it when MELPA builds and report back?

Thanks

@ryanprior
Copy link
Author

Looks flawless with my brief testing just now. With this fix and hidpi icons coming with #4, Emacs will have the best-looking emoji rendering of any app on any platform. What an accomplishment!

@iqbalansari
Copy link
Owner

Thanks for confirming! A lot for credit for this goes to you, I would have not got around to implementing this had you not pushed for it 😉.

I will leave this issue open for sometime before closing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants