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

[draw] Finish and release draw API #3411

Merged
merged 73 commits into from
Feb 16, 2022
Merged

[draw] Finish and release draw API #3411

merged 73 commits into from
Feb 16, 2022

Conversation

behdad
Copy link
Member

@behdad behdad commented Feb 2, 2022

Okay I'm determined to finish and release draw API in the next three weeks of hacking. @ebraminio are you available to help? @khaledhosny can I get some help from you?

I know what I like done, and I'll enumerate that in the next comments. Before that, I like to know how @ebraminio was testing this. I guess with JS? UPDATE: Using src/main tool.

@behdad
Copy link
Member Author

behdad commented Feb 2, 2022

So far what I like to change:

  • Use float instead of hb_position_t in the API. If nothing else, saves a roundf() per position.
  • The callbacks should also get the hb_draw_funcs_t pointer themselves. It was a lazy move to not do that to begin with.
  • Expose the hb_draw_helper_t object as a public struct, called hb_draw_state_t or similar; pass them to callbacks as well.
  • Add public API called hb_draw_move_to, hb_draw_line_t, etc, that operate on a hb_draw_funcs_t and a hb_draw_state_t; might need to add a hb_draw_new_path() or similar to initialize the state.
  • (hardest probably) make hb-view use these and plug into cairo user-font API to render without cairo-ft.
  • Remove return value of hb_font_draw_glyph.
  • Make hb_font_draw_glyph a callback of hb_font_funcs_t.
  • Implement above in hb-ft.
  • Figure out interaction with synthetic_slant.
  • Document draw callbacks.
  • Write tests for the hb_draw_move_to/etc.

@ebraminio
Copy link
Collaborator

Okay I'm determined to finish and release draw API in the next three weeks of hacking.

Great to hear!

Guess now needs coordination with the brand new _synthetic_slant also.

@behdad
Copy link
Member Author

behdad commented Feb 2, 2022

Guess now needs coordination with the brand new _synthetic_slant also.

Interesting question!

@behdad

This comment was marked as resolved.

@behdad

This comment has been minimized.

test/api/test-draw.c Outdated Show resolved Hide resolved
@behdad

This comment has been minimized.

@behdad

This comment has been minimized.

meson.build Outdated Show resolved Hide resolved
src/hb-draw.hh Outdated Show resolved Hide resolved
@khaledhosny
Copy link
Collaborator

@khaledhosny can I get some help from you?

Sure, what do you need me to do.

@behdad
Copy link
Member Author

behdad commented Feb 3, 2022

  • Implement above in hb-ft.

@khaledhosny How about this?

@behdad
Copy link
Member Author

behdad commented Feb 3, 2022

  • Write tests for the hb_draw_move_to/etc.

@ebraminio Can I get help from you on this one?

@behdad
Copy link
Member Author

behdad commented Feb 3, 2022

image
First rendering from hb-draw in hb-view. Not perfect, but not nothing. I believe the shortcomings are cairo-user-font shortcomings, not ours...

behdad added a commit that referenced this pull request Feb 3, 2022
Only works with --font-funcs=hb-ot right now.

Uses cairo-user-font backend internally, which seems to extent issues.
Also, with Zapfino, the 'Z' seems to put the scaled-font into an error.

For the extent issue example see:
#3411 (comment)
@behdad
Copy link
Member Author

behdad commented Feb 4, 2022

I believe the shortcomings are cairo-user-font shortcomings, not ours...

And as I suspected, fixed in cairo main... Just not in a release yet :(.

@behdad
Copy link
Member Author

behdad commented Feb 4, 2022

image
In its full glory with cairo main branch.

@behdad
Copy link
Member Author

behdad commented Feb 4, 2022

To summarize, what's left:

  • hb-ft implementation
  • chaining up to parent font in hb-font's default implementation
  • Figure out interaction with synthetic_slant.
  • Document draw callbacks and other new API.
  • Write tests for the hb_draw_move_to/etc.

I think this can go in then, with the hb-view implementation as well, but will depend on very recent cairo. So initially I suggest we push in everything but that last bit.

@behdad
Copy link
Member Author

behdad commented Feb 4, 2022

I think this can go in then, with the hb-view implementation as well, but will depend on very recent cairo. So initially I suggest we push in everything but that last bit.

Okay, rewrote the logic to keep the cairo-ft implementation as well, but enable the hb-draw logic if cairo is recent-enough (1.17.5 version-check) AND font-funcs are NOT ft. The latter check can be removed when we implement the callbacks for hb-ft.

@behdad behdad force-pushed the draw branch 3 times, most recently from ade9833 to 0d2d8f6 Compare February 5, 2022 01:18
@behdad

This comment was marked as resolved.

@khaledhosny

This comment was marked as resolved.

@behdad
Copy link
Member Author

behdad commented Feb 5, 2022

Question: Should I add back the return value of the hb_font_get_glyph_shape() call? Such that callers can know whether implementation exists?

@behdad
Copy link
Member Author

behdad commented Feb 5, 2022

Question: Should I add back the return value of the hb_font_get_glyph_shape() call? Such that callers can know whether implementation exists?

I personally don't see the point of that. You are writing the code, you know what's on the other side...

This makes it actually match freetype behaviour even though rasterizer
should filter such contours specially for stroking.

See #3411 (comment) for the context.
test/api/test-draw.c Outdated Show resolved Hide resolved
Let's have that part of the code also covered.
@ebraminio
Copy link
Collaborator

Also added a test for ft callback 0429921 feel free to just remove the commit from history if you don't see fit.

Also any interest on having binding to coretext/gdi/directwrite #3411 (comment) somewhere somehow? Just though it could be nice for the sake of debugging and so later.

@behdad
Copy link
Member Author

behdad commented Feb 13, 2022

Also any interest on having binding to coretext/gdi/directwrite #3411 (comment) somewhere somehow? Just though it could be nice for the sake of debugging and so later.

If you are willing to implement the rest of the font-funcs callbacks for them... In 10+ years no one bothered / cared / requested.

@behdad
Copy link
Member Author

behdad commented Feb 13, 2022

@ebraminio why did the linux-ci bot fail?

behdad and others added 2 commits February 13, 2022 15:39
As CI failure, apparently the my local freetype and CI one have different
result so let's switch the case with a simpler one just to test quadratic command
is emitted correctly.
@ebraminio
Copy link
Collaborator

ebraminio commented Feb 13, 2022

why did the linux-ci bot fail?

My local freetype version is 24 and the CI one is 21 and that apparently means different results (or maybe because the debug bit leftover I just spotted now)

I swapped the test case with a simpler one case just to test line_to/quadratic_to commands of ft callbacks are emitted correctly.

@behdad
Copy link
Member Author

behdad commented Feb 13, 2022

Would be nice to add a hb-draw commandline tool also, that spews SVG-like draw commands given glyphnames...

Fixes another color fonts issue.
@ebraminio
Copy link
Collaborator

Would be nice to add a hb-draw commandline tool also, that spews SVG-like draw commands given glyphnames...

I wish it could create a SVG from input text without Cairo, say like https://harfbuzz.github.io/harfbuzzjs/ demo which creates SVG from input text, I needed accurate rendering of some text in SVG to upload to Wikipedia and shaped the tool for the purpose which creates relative SVG path also to have more optimized yet accurate result.

Just to note main.cc also creates SVG for individual glyphs of a font when hb-draw is enabled and supports COLRv0/SVG/png dumping also.

@behdad
Copy link
Member Author

behdad commented Feb 16, 2022

Would be nice to add a hb-draw commandline tool also, that spews SVG-like draw commands given glyphnames...

I wish it could create a SVG from input text without Cairo, say like https://harfbuzz.github.io/harfbuzzjs/ demo which creates SVG from input text, I needed accurate rendering of some text in SVG to upload to Wikipedia and shaped the tool for the purpose which creates relative SVG path also to have more optimized yet accurate result.

Maybe we can spec and develop that separately then. I like to merge this soon.

@behdad behdad merged commit a396543 into main Feb 16, 2022
@behdad behdad deleted the draw branch February 16, 2022 00:47
@behdad
Copy link
Member Author

behdad commented Mar 2, 2022

behdad/glyphy#39

I ported GLyphy to use hb-draw:
image

@yisibl
Copy link
Contributor

yisibl commented Apr 28, 2023

I wish it could create a SVG from input text without Cairo, say like https://harfbuzz.github.io/harfbuzzjs/ demo which creates SVG from input text, I needed accurate rendering of some text in SVG to upload to Wikipedia and shaped the tool for the purpose which creates relative SVG path also to have more optimized yet accurate result.

Maybe we can spec and develop that separately then. I like to merge this soon.

@behdad Any progress since then?

@behdad
Copy link
Member Author

behdad commented Apr 28, 2023

@behdad Any progress since then?

No. Can you use the hb-view SVG output and massage it through svgopt or other tools?

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

6 participants