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

Add a small emoji picker/selector example #420

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

DJMcNab
Copy link
Member

@DJMcNab DJMcNab commented Jul 1, 2024

This depends on:

These are all present on Masonry main.

Xilem Emoji Picker.

Some notes:

  1. The accessibility of this example probably isn't great. Not sure what to do about this.
  2. Our layout primitives aren't great; I've hacked around a grid layout, but we really need to make a Grid widget/View
  3. The pagination is slightly hacked together - it should really try and give you page numbers. I'm not planning to address this, unless someone provides the algorithm
  4. Emoji only render on systems with a system COLR Emoji font. I know my Android device does, but I don't know if any other systems do? Maybe Windows and some Linux systems? This blocks this PR

This is created for linebender/linebender.github.io#56

@jaredoconnell
Copy link
Contributor

I tested this, and the fonts are not visible on Mac OS. Maybe the font isn't COLR format.

@DJMcNab
Copy link
Member Author

DJMcNab commented Jul 3, 2024

Indeed. This is called out in linebender/linebender.github.io#56, for example.

@DJMcNab DJMcNab marked this pull request as draft July 18, 2024 10:16
@DJMcNab
Copy link
Member Author

DJMcNab commented Jul 18, 2024

Moving this into draft due to the lack of bitmap emoji support.

@DJMcNab
Copy link
Member Author

DJMcNab commented Oct 29, 2024

This should start working properly once we have Vello 0.3 and Parley 0.2. See #616

github-merge-queue bot pushed a commit that referenced this pull request Nov 18, 2024
The biggest remaining issue in this PR is that IME support is not
present.

However, I think landing this is *better* than not landing it, because:
1) If we don't land it, it's going to languish again
2) Getting IME support back can be parallelised (cc @tomcur)
3) Getting Vello 0.3.0 and Parley 0.2.0 unlocks real advantages,
including full emoji support (#420).

To be clear, my first follow-up priority will be connecting the IME back
up. I do not however think this should block on Parley 0.3.0.

Discussion in
https://xi.zulipchat.com/#narrow/channel/317477-masonry/topic/Updating.20Parley.20dependency
Add license heading

Add accessibility comments

Fix clippy and formatting

Fix android compilation
@DJMcNab DJMcNab marked this pull request as ready for review November 21, 2024 16:01
@jaredoconnell
Copy link
Contributor

Are emojis on non-COLR fonts out of scope for this PR? It still does not show emojis on MacOS.

@DJMcNab
Copy link
Member Author

DJMcNab commented Nov 21, 2024

Hmm... It supports bitmap and COLR emoji - it works on my desktop Linux machine.

@dfrg do we expect this to work with the Emoji support in Parley/Vello as-is?

@dfrg
Copy link
Contributor

dfrg commented Nov 21, 2024

I think we make an attempt to do proper emoji font selection. It should work fairly well on Mac/Windows. Linux will depend on installed fonts and fontconfig setup.

@DJMcNab
Copy link
Member Author

DJMcNab commented Nov 21, 2024

@jaredoconnell then I'm going to have to ask you to debug this. I don't have a Mac to test with

@jaredoconnell
Copy link
Contributor

Are there any resources or documentation I should use to debug this?

@dfrg
Copy link
Contributor

dfrg commented Nov 21, 2024

Actually, @LaurenzV discovered a bug in the sbix bitmap code here: https://github.com/linebender/vello/blob/98192612d9f4b7aed9b3223680527473ec449ee9/vello/src/scene/bitmap.rs#L165

The tag should be “png “. This will need to be fixed to properly handle Apple Color Emoji.

@DJMcNab
Copy link
Member Author

DJMcNab commented Nov 22, 2024

@LaurenzV are you willing to make that PR to Vello?

Edit: I now see the context that this was googlefonts/fontations#1167 (comment); I'll leave it open today since it is your change, but otherwise I'll open it myself in Vello.

@LaurenzV
Copy link

If you could do it, that would be great! If not I'll try to do it in the next few days.

github-merge-queue bot pushed a commit to linebender/vello that referenced this pull request Nov 22, 2024
See
googlefonts/fontations#1167 (comment)

Should make linebender/xilem#420 work.

Co-authored-by: Laurenz Stampfl <47084093+LaurenzV@users.noreply.github.com>
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.

4 participants