[p5.js 2.0 Bug Report]: textSize and textWeight order shouldn't matter

[ksen0]

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

2.0.4

Web browser and version

139.0.7258.139 (Official Build) (arm64)

Operating system

MaxOS 13.0 (22A380)

Steps to reproduce this

Steps:

1. Use textSize and textWeight to apply a text style
2. If textWeight comes after textSize, then the behavior is unexpected: the weight won't be applied to the next text correctly.

Snippet:

This has unexpected behavior, where the top word uses changing weight value, and not the bottom word:

async function setup() {
  createCanvas(400, 400);
  background(255);
  
  font = await loadFont("https://fonts.googleapis.com/css2?family=Google+Sans+Code:ital,wght@0,300..800;1,300..800&display=swap");
  textFont(font);
}

function draw(){
  background(255);
  
  textSize(10);
  textWeight(300);
  text("Hello", 10, 50); //this should be a constant weight
  
  textSize(30);
  textWeight(map(sin(frameCount/20), -1, 1, 300, 800));
  text("World", 10, 100); //this should be an animated weight
  
}

This shows the expected behavior:

async function setup() {
  createCanvas(400, 400);
  background(255);
  
  font = await loadFont("https://fonts.googleapis.com/css2?family=Google+Sans+Code:ital,wght@0,300..800;1,300..800&display=swap");
  textFont(font);
}

function draw(){
  background(255);
  
  textWeight(300);
  textSize(10);
  text("Hello", 10, 50);
  
  textWeight(map(sin(frameCount/20), -1, 1, 300, 800));
  textSize(30);
  text("World", 10, 100);
  
}

Intuitively, these snippets should act the same.
Thanks @pattvira for finding this behavior!

[perminder-17]

Hi @ksen0 , thanks for opening this issue and thanks @pattvira for catching this. Since I am not a mac user and this issue seems to be a browser specific issue, it would be great to check if this happens to safari as well?

I tried to debug this problem and found some weirdness and want to be more sure before going ahead. Actully, I tried running it in two different browsers and it was working expected in firefox but in chrome we can see the issue.

So, after some debugging I tried to get some findings,

we are actually setting context.font

p5.js/src/type/textCore.js

Line 2391 in e9eaf99

drawingContext.font = fontString;

and also setting canvas.style.fontVariationSettings

p5.js/src/type/textCore.js

Line 1673 in e9eaf99

this._setCanvasStyleProperty('font-variation-settings', `"wght" ${weight}`);

and this second setting is the element.style declaration, which has very high priority in the CSS cascade so, In Chrome, when font-variation-settings is present, it can override the weight coming from context.font.

So, when we call textWeight(300) to draw “Hello” and later call textWeight(some animation) and draw “World” Because canvas.style.fontVariationSettings still has the last wght you set, Chrome applies that axis value to all text.

Maybe firefox is doing things differently here, (not sure tbh).

A possible fix I was thinking was:

Clearing fontVariationSettings which will remove the extra CSS override that was being set on the element. As in Chrome, this caused the two sources of truth to clash (context.font and fontVariationSettings). By resetting fontVariationSettings to an empty string, we ensure that only context.font controls the text weight.

Tagging other maintainers for their review with the approach or maybe they had some other suggestion with the fix : @limzykenneth @davepagurek

Or if everything in the approach sounds good, I could go ahead with the PR?

[ksen0]

Re: browser-specific issue, some more observations on MacOSX.

Vivaldi: behavior is the same as reported in Chrme (order matters; both sketches run)

Firefox: as Perminder found, this works as expected (both sketches run and order doesn't matter)

Safari: Both sketches do not work, I get the below error for both snippets, which is triggered by loadFont (also without any of the other code). True for all 2.x versions so far:

TypeError: Illegal constructor
    at CSSStyleSheet ([native code])
    at (anonymous function) (https://cdn.jsdelivr.net/npm/p5@2.0.4/lib/p5.js:119942:45)
    at asyncFunctionResume ([native code])
    at (anonymous function) ([native code])
    at promiseReactionJobWithoutPromise ([native code])
    at promiseReactionJob ([native code])

[dhowe]

Clearing fontVariationSettings which will remove the extra CSS override that was being set on the element. As in Chrome, this caused the two sources of truth to clash (context.font and fontVariationSettings). By resetting fontVariationSettings to an empty string, we ensure that only context.font controls the text weight.

Will the proposed fix not cause problems when fontVariationSettings is used to manage other attributes/axes ?

Also seems like we need a separate issue for the Safari loadFont problem ?

[perminder-17]

Hi @dhowe, really sorry if anything sounds silly or wrong. Also, sorry for the delay in getting back to this.

Tbh, I tried changing attributes/axes of fontVariationSettings, but it was not having any issues. Maybe the reason is :

Actually, the fontVariationSettings I’m clearing is on the canvas, not on a DOM text node. It’s like:

this.textCanvas().style.fontVariationSettings = ' ' 

DOM text gets its weight/axes from CSS (e.g., font-variation-settings: 'wght' 700), so clearing that does change how the DOM text looks. Canvas text does not read CSS from the <canvas> element. Clearing on a DOM element will show visible changes, but clearing it on the <canvas> element (e.g., canvas.elt.style.fontVariationSettings = ' ') produces no visible change to canvas-drawn text, because the canvas draw calls don’t use the element’s CSS for variable axes.

I’ve also been testing with a dummy sketch after clearing fontVariationSettings in code. Here’s a sketch that demonstrates font-variation-settings controlling other axes/attributes for DOM text: https://editor.p5js.org/aman12345/sketches/gmxEXZDKh. @ksen0, could you check the behavior in Vivaldi? It works in Chrome and Firefox now. I’ll open an issue for Safari as well.

Please let me know wherever I’m mistaken, @dhowe. If anything sounds off or imprecise, I’d really appreciate your corrections and guidance. Thank you!

[ksen0]

@perminder-17 Looks in Vivaldi (7.5.3735.66 (Stable channel) (arm64)) same as Chrome / behaves as it seems it should! Thank you for investigating.

[davepagurek]

If we test setting custom font variable axes with textProperty() does it still work or does that get cleared?

Just want to double check, because when we were figuring out how to get variable font axes to work cross browser, Chrome actually did read the don't variation settings from the canvas element when drawing text onto the canvas. Firefox ignores it I think, and Safari breaks if it's present, leading to our inclusion of this short circuiting: https://github.com/processing/p5.js/blob/dev-2.0/src%2Ftype%2FtextCore.js#L1714-L1719

At first we thought that meant variable fonts would only work in chrome, but we found that there were other ways to get specific variables, namely weight, to work in other browsers. So font weight probably still does work correctly with this cleared, but other variables will work differently, but also don't have cross browser support yet.

So I think we should double check (and maybe add some test cases since we probably have none currently) for non-weight variables in chrome to make sure that doesn't break.

[ksen0]

I'm not sure I really understand the test case @davepagurek but I tried to include a non-weight var in this revision of @perminder-17 's example: https://editor.p5js.org/ksen0/sketches/H00diuECj Does that test what you mean?

+1 on adding tests, including to the manual suite on this one