-
-
Notifications
You must be signed in to change notification settings - Fork 212
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
Support ligature using harfbuzz #1659
Draft
Mandarancio
wants to merge
15
commits into
lite-xl:master
Choose a base branch
from
Mandarancio:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
faeaae5
feat: added dependency to `harfbuzz` library
efafbbb
feat: added initial support to ligatures
2bfafc7
fix: free hb objects
b5e2203
fix: better constructor and distructor fo hb objects
85a7513
feat: re-enabled glyset logic with 256 size buffer
ff81ee4
fix: added harfbuzz.wrap
5c90651
fix: using harfbuzz wrap
b5bcf19
fix: removed not used dependencies
89108f8
Update meson.build
Mandarancio dd4f27e
feat: better loading performances
f599543
feat: added support for fallback fonts
40d9a3a
fix: restored missing character
509efd8
fix: tab spacing could modify metrics of other fonts
878579d
fix: put back support for space advance
02c1959
fix: fixed issue with tab and colliding codepoints
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
[wrap-file] | ||
directory = harfbuzz-8.2.2 | ||
source_url = https://github.com/harfbuzz/harfbuzz/releases/download/8.2.2/harfbuzz-8.2.2.tar.xz | ||
source_filename = harfbuzz-8.2.2.tar.xz | ||
source_hash = e433ad85fbdf57f680be29479b3f964577379aaf319f557eb76569f0ecbc90f3 | ||
source_fallback_url = https://github.com/mesonbuild/wrapdb/releases/download/harfbuzz_8.2.2-1/harfbuzz-8.2.2.tar.xz | ||
wrapdb_version = 8.2.2-1 | ||
|
||
[provide] | ||
dependency_names = harfbuzz |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question, in case that a fallback font is used for some codepoints not available on main font, shouldn't that portion get shaped also with the fallback font instead of using the main font?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, as it is implemented now, ligatures are supported only for the main font and so the shaping is only applied to the main one, for the fallaback case I am using the
fb_codepoint
that is computed insrc/renderer.c:413
that is then converted if needed to the correct font index usingFT_Get_Char_Index
atsrc/renderer.c:181
.I hope the explanation is clear.
It would be possible to shape for all the available fonts but I suspect will be more computational intensive and you could end up with half a ligature of a fallback font and half a normal character form another.
What do you think?
edit: also I am not sure that the mapping between text and glyph would be the same and could be possible that some kind of mapping would be needed between different fonts and the string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function shared on pragtical/pragtical@ed28c3a iterates a string giving you the range of a string (thru the
char** next
out param) and returns the font that has all of its codepoints for rendering:This way you can shape string segments using the assigned font (which is the strategy used on that patch). When tested I didn't notice a performance drop but I didn't benchmark the thing :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I start to have a look at your commit but I did not finish yet as I am quite busy now. However I am still not 100% sure that is a good idea to support ligatures in fallback fonts, maybe can @Guldoman gives us his opinion?