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

[2.0] Make variable Typr p5.Fonts use the same variables as the 2D canvas #7447

Open
2 of 17 tasks
davepagurek opened this issue Dec 24, 2024 · 2 comments · May be fixed by #7459
Open
2 of 17 tasks

[2.0] Make variable Typr p5.Fonts use the same variables as the 2D canvas #7447

davepagurek opened this issue Dec 24, 2024 · 2 comments · May be fixed by #7459

Comments

@davepagurek
Copy link
Contributor

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Build process
  • Unit testing
  • Internationalization
  • Friendly errors
  • Other (specify if possible)

p5.js version

dev-2.0

Web browser and version

Firefox

Operating system

MacOS

Steps to reproduce this

Currently, if you load a variable font (e.g. test/manual-test-examples/type/font/BricolageGrotesque-Variable.ttf) in WebGL mode, word wrapping and measurement is currently broken because the parameters that Typr seems to read are different than the variables picked by default by the 2D canvas, which is what is used for measurement.

Typr now supports reading variable font data (photopea/Typr.js#56) so it seems like after updating Typr, we should be able to make these match!

One other complication: We currently associate one FontInfo (containing font bezier cache data for WebGL) with one font:

p5.js/src/webgl/text.js

Lines 687 to 690 in 05e35cb

let fontInfo = this.states.textFont._fontInfo;
if (!fontInfo) {
fontInfo = this.states.textFont._fontInfo = new FontInfo(font);
}

However, we will need a new FontInfo whenever any variable changes for the font, as that will change all the bezier data. So we likely will also need to have a cache by variable value in the font (possibly using a lot of memory if these change a lot? maybe needing an LRU cache to evict old ones?), or just invalidate the existing font info whenever variables change (possibly slow if this is animating every frame.)

@davepagurek davepagurek changed the title [2.0] Make Typr p5.Fonts use the same variables as the 2D canvas [2.0] Make variable Typr p5.Fonts use the same variables as the 2D canvas Dec 24, 2024
@dhowe
Copy link
Contributor

dhowe commented Dec 24, 2024

I'm added an updated Typr version into our repo in #7449 so this is ready to work on
(note that Typr will hopefully be made into an npm package at some point, but it is now a manual process to upgrade)

AritraDey-Dev added a commit to AritraDey-Dev/p5.js that referenced this issue Jan 6, 2025
Fixes processing#7447

Update `FontInfo` class in `src/webgl/text.js` to account for variable font changes and cache by variable value.

* Add `variableFontCache` to `FontInfo` class to store variable font data.
* Modify `getGlyphInfo` method to use cache key based on glyph index and variable values.
* Adjust `getGlyphInfo` method to return cached glyph info based on cache key.
* Update initialization of glyph info to use cache key.

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/processing/p5.js/issues/7447?shareId=XXXX-XXXX-XXXX-XXXX).
@AritraDey-Dev AritraDey-Dev linked a pull request Jan 6, 2025 that will close this issue
@AritraDey-Dev
Copy link

@davepagurek
Thank you for bringing up this issue. I’ve addressed it and submitted a pull request. The problem with variable font measurement and word wrapping in WebGL mode has been fixed by:

Updating Typr.js to the latest version to ensure proper reading of variable font data.
Modifying the FontInfo implementation to handle variable font changes dynamically. I’ve implemented an LRU cache to efficiently manage memory usage when variable parameters are updated frequently.

Please verify my PR and let me know if you need any additional changes or tests!

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

Successfully merging a pull request may close this issue.

3 participants