[p5.js 2.0 Bug Report]: That one font test is flaky
Most appropriate sub-area of p5.js?
- [ ] Accessibility
- [ ] Color
- [ ] Core/Environment/Rendering
- [ ] Data
- [ ] DOM
- [ ] Events
- [ ] Image
- [ ] IO
- [ ] Math
- [x] Typography
- [ ] Utilities
- [ ] WebGL
- [ ] Build process
- [ ] Unit testing
- [ ] Internationalization
- [ ] Friendly errors
- [ ] Other (specify if possible)
p5.js version
2.x
Web browser and version
Chrome
Operating system
Github action runner
Steps to reproduce this
If you've submitted a PR before, you may have seen this visual test fail:
https://github.com/processing/p5.js/blob/60ef55ed941b131b7d81ce060c7d46185bf1d78c/test/unit/visual/cases/typography.js#L79-L94
When it fails, we see it just using the default serif font rather than the Google Font.
Often just rerunning the CI jobs causes it to work. It'd be great to figure out exactly why this is flaky and see what we can do about it.
My current theory is this: when we load a font via a CSS file (as we are when we load a Google Font), there are potentially a lot of different font files referenced in it, and we pick what we think the most relevant one is to load. However, we also attach the whole CSS file to the page, so the other font options are still accessible to the sketch. But potentially the browser isn't adding those other fonts to document.fonts reliably until they are actually requested for use by something in the DOM or on the canvas, so when we immediately try to render using it, it's not present, but might be present if we waited for it to load and then rerendered. The main problem with this theory: why does rerunning the tests work, then, when it should be running in a fresh container with no history of the last run? Does it just happen to load fast enough sometimes?
There are two potential things we can do about this:
- Identify and fix the underlying cause of the flakiness and make it
awaitable - Figure out another way to test font weight usage in CSS fonts without this flakiness
Hi, I'd like to take this on. Please assign it to me.
Thanks @Harsh16gupta! Before assigning this one to someone, we need to do a bit of research to figure out specifics on how to proceed -- do feel free to start looking into it though! Keep us updated in the comments here about what you find and what you would do to address the issue, and once we figure out a path forward we can then move to implementation.
HI @davepagurek , I looked into the issue and found that:
When loading a Google Font via a CSS file, multiple font-face rules are included. P5.loadFont() loads only the first matching font-face block, but the browser may not fetch all the weights (400, 800) immediately. Even with await, the browser may still load the CSS before the actual font files are ready.
How we could approach fixing it:-
Wait for document.fonts.ready or document.fonts.load() to confirm speciic weights are available before running test. Replace google fonts with a local test font so CI doesn't depend on external network requests. Or maybe render twice - first load the font, then force a rerender after a delay to ensure weight availability.
If this direction seems right, I'd like to work on this issue and propose a PR. Let me know if you would like me to take ownership of the issue.
Hi @davepagurek ,
I’ve looked into the flaky font test and found it’s likely caused by the font not being fully loaded before rendering in CI, making tests unreliable. To fix this, we should wait for document.fonts.ready before taking screenshots, and consider preloading the font or hosting it locally to avoid network issues. If you’re open to it, I’d love to work on a PR to address this!
We actually already await document.fonts.ready here: https://github.com/processing/p5.js/blob/60ef55ed941b131b7d81ce060c7d46185bf1d78c/src/type/p5.Font.js#L1065
I also tried waiting for document.fonts.load in my WIP WebGPU mode PR while working on font support there, but that doesn't help this test's case -- again possibly because it isn't loading all the weights. We could try an experiment and see if loading every weight here works? That may not be what we want long term though because that's a lot of upfront loading for fonts that may not get used. Lazy loading is likely the best default behaviour here. But possibly we could add a method to p5.Font like font.waitFor(style) so we could do await that manually in the test and pass in a CSS string with the weight we want? (I don't actually know 100% that this is the problem though, it's worth testing this out to see if it makes a difference.)
Hii @davepagurek I tested the hypothesis by adding explicit document.fonts.load() calls but this caused the test to timeout (1000ms), which suggests the fonts aren't available in document.fonts at all after the CSS is loaded.This seems to confirm the theory the browser doesn't add all font weights to document.fonts just from loading the CSS file. They only become available
when actually used by the DOM/canvas.So the issue is: we can't await fonts that the browser hasn't registered yet.
Thanks @Homaid, what did you pass in to load()? My initial test didn't specify a given weight so it made sense that that wouldn't necessarily work. Did you try requesting the weights that the test was about to use?
Yes, I specified the exact weights and size that the test uses
await document.fonts.load('400 35px Sniglet');
await document.fonts.load('800 35px Sniglet');
Both calls timed out at 1000ms that means even though the the CSS is loaded, the specific font weights (400 and 800) aren't being registered in document.fonts,so there's nothing for fonts.load() to load.It seems the browser only registers these fonts when they're actually rendered what do you think @davepagurek
If you manually add something to the DOM first, like a <span style="font-family: Sniglet; font-weight: 400">Test</span>, does that work? If that works, going on the idea that we can make a p5.font.waitFor(...) function, that could first add something like that to the DOM (maybe with display: none or something to hide it), then await, then remove it
So adding DOM elements with the font styles first worked! The fonts.load() calls completed successfully after the spans were added to the DOM. so i think we can implement your idea!!! @davepagurek
Thanks for testing that out @Homaid! Are you interested in making that function and updating the test?
nice find - have we tested this on other browsers?
I would love to work on this, currently my midterm is going on I will make a pr after that!!! Thank you @davepagurek
oh also one other question, for @dhowe too: since in the CSS file case we'd need users to specify exactly what font usage they want in order to get a special promise to wait for, any thoughts on that API? my initial idea of font.waitFor(cssString) is a good catch-all of usage but might not be intuitive for people who just want to load a font and then use a font weight. Another option could be a global loadFontSettings() (or a better name), so usage would be like:
async function setup() {
let font = await loadFont('some/google/fonts/url')
textFont(font)
textWeight(800)
await loadFontSettings()
// now you can draw with the set font options and guarantee it's there
}
I think the second approach would be better (loadFontSettings()) since it will have better user experience, They would just need to add one await loadFontSettings() call after setting properties. And about the name I think it's fine we can also use something like ( waitForFonts() or ensureFontsLoaded())
Are there any cases where calling this would be necessary other than if you've loaded a CSS font? If not, maybe we could include CSS in the name somewhere to make the intended usage clearer
First-time load of a sketch can use the wrong font in the wild
(Hi all! Just adding some hopefully useful data points.) This loading of the wrong font also occurs in a normal sketch in the wild for me on Chrome. Specifically I've been using this simplified sketch on OP: https://openprocessing.org/sketch/2797183
For me, it fails about 1 in 4 or 1 in 5 times, provided I load using empty cache and hard reload each time.
By "fail", I mean the wrong font is used in the render.
empty cache and hard reload
- open devtools
- right-click the browser's reload button
- click "empty cache and hard reload"
more detail
Almost all failures have presented something that looks similar to times new roman. I have exactly once in about 100 attempts seen the sketch instead show the weight 800 font in place of the weight 400.
Almost all tests were done whilst logged into open processing as the owner of the sketch, with the sketch presented in side-by-side view (code + output). I've just now tried a few incognito loads which present the canvas-only view: yielded 2 fails in 22 attempts.
I haven't been able to get it to fail once in 30+ "normal" reloads of the tab, only when using empty cache and hard reload.
results with various browsers
-
Chrome 141.0.7390.123 on macOS 14.6.1: test sketch fails ~20% of the time, on average. (Reported above)
-
Firefox (145.0.1 (aarch64) on macOS 14.6.1): zero test sketch failures in 20 attempts (with cache disabled)
-
Chrome 142.0.7444.158 on android: no test sketch failure in 12 attempts. However, I'm not confident i'm really emptying the cache correctly on mobile.
previous discussion on discord
(Note that I'm using a different simplified test sketch than the one used in earlier discussion on discord. (See parent sketch). All findings reported above were made with the new test sketch.)
I don't think so there are really other cases @davepagurek we can use something like this ( loadCssFontVariant() or ensureCssFontsLoaded()) and the data point from the @nbogie it confirms this isn't just a testing issue but affects real users on first load.
To clarify, the case here is only for fonts defined in a css file with multiple font-face declarations? Are there other properties/axes besides weight where this issue might occur?
From an API perspective, I'm a bit hesitant about the global function as it may be confusing for average users as to when it should be used.
Would it be useful to allow the user to query the font as to which styles it contains and which are loaded, something like font.getStyles() or font.cssStyles() ?
@dhowe more than just weight I think -- looking at a google font file as an example, e.g. https://fonts.googleapis.com/css2?family=Sniglet:wght@400;800&display=swap, there are different fonts that could be loaded based on both weight and unicode range. These could also be based on font-style or possibly some other variants too. I think that also means that technically it's not just the font settings in p5 that could trigger a font load, but also the text content itself, if it includes characters from a different unicode range.
We do at one point have a list of all the variants we think are in a CSS file we could consider storing on the p5.Font if you have ideas on how that should work.
That makes sense about Unicode ranges, regarding storing variants on p5.Font, since you mentioned p5.js already extracts the variants list from CSS, what if we used that to make a helpful API? Not maintaining separate state, but providing a convenient wrapper around document.fonts like this
await font.loadVariant({ weight: 800, style: 'italic' });
Right, so I was thinking we might allow the user to query the font styles or variants (where each is an object) and then to select one or more to be loaded:
let styles = font.cssVariants(); // array of style descriptor objects
await font.loadCSSVariant(styles[2]);
we might also support partial matching, as below, but would add some complexity and could inadvertently result in quite a large number of variants being loaded:
await font.loadCSSVariants({ weight: 800 }); // all variants matching the weight