[p5.js 2.0 Beta Bug Report]: textFont() throws error when a p5.Font is applied

[davepagurek]

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 beta 1

Web browser and version

Firefox

Operating system

MacOS

Steps to reproduce this

Steps:

1. Load a font
2. Apply it with textFont()
3. Measure width with textWidth()

When no font is applied this works, but with a p5.Font applied, I get TypeError: this.states.textFont._textWidth is not a function

Snippet:

async function setup() {
  createCanvas(400, 400);
  const font = await loadFont('https://fonts.gstatic.com/s/sniglet/v17/cIf4MaFLtkE3UjaJ_ImHRGEsnIJkWL4.ttf')
  textSize(100)
  textFont(font)
  console.log(textWidth('test'))
}

Live: https://editor.p5js.org/davepagurek/sketches/TuNtJ4gJt

[seyko1]

Hi,

I'm student coder at Paris 8 University and I'd like to try to contribute on p5.js.

Does this issue still need someone to work on @davepagurek ?

[davepagurek]

Thanks @seyko1, I'll assign this to you!

This issue is for 2.0, which means branching off of the dev-2.0 branch. We don't have extensive contributor docs for this branch just yet, but npm run dev runs a sketch out of preview/index.html with live reloading when you make changes, and as usual npm run test runs the whole test suite. Don't hesitate to reach out here or in discord if we can help explain anything else about the 2.0 build!

[dhowe]

I looked into this very briefly and it seems that some of the old functions in the subclass p5.Renderer2D are being called (textWidth) in this case, rather than those in parent p5.Renderer itself. The solution is, I believe, to just remove those old functions from p5.Renderer2D (for example, anything mentioning opentype)

[seyko1]

I also got the same error from src/webgl/text.js because its prototype textWidth also contains a call to the old function _textWidth.

Except these two things about _textWidth, there are other functions that seem to be removed :

• p5.Renderer2D._renderText doesn't seem to be used anymore
• Some functions declared into p5.Renderer.js, like textAscent(), textDescent(), textAlign(), textWrap().. are called instead from their prototype declaration in src/type/text2d.js, should they be deleted ?

I will wait for your answers but I suppose It's better to firstly create a PR to fix this little issue with _textWidth(),
And doing other cleaning separatly ?

[dhowe]

I'm not sure what changed in the last couple of weeks to cause these issues, but the updated/correct functions should be in src/type/text2d.js.

There should be no 2d public text functions in the p5.Renderer2D.js file - some private functions are attached to the prototype in src/type/text2d.js

@davepagurek should be able to answer questions regarding these and src/webgl/text.js

First step is probably to add a test that fails on this case as current tests aren't catching it

[davepagurek]

In general, anything defined in src/type/text2d is the correct version, so if a duplicate implementation is still in p5.Renderer.js can be removed in favour of those. So the textWidth in src/webgl/text.js should be removed in favor of the src/type/text2d.js one too, good catch!

There are some methods that do need to be different per renderer, and _renderText is one of those. Right now the Renderer2D one in src/core/Renderer2D.js is the old one and should be removed in favour of the one in src/type/text2d.js, and the one in src/webgl/text.js is still correct.

It would be good, after fixing the bug, to clean up this structure a bit -- src/type/text2d.js is the common core implementation, and then maybe anything render-specific should go in their respective Renderer2D.js/RendererGL.js files? (Maybe also adding a base version in Renderer.js that just throws an error so we're sure to catch it if a new future renderer forgets to implement a required text method?) And also possibly changing the name of the text2d file to reflect this, maybe renaming it to textBase. (If you run git mv oldName.js newName.js and then commit the results, git will understand that the file has just moved and nothing else and will keep the history clean.)

[seyko1]

Sorry for the delay, in addition to @dhowe changes from #7523 PR,
I removed textWidth function in src/webgl/text.js to fix the same error with WEBGL loaded.

[davepagurek]

Thanks everyone, looks like we've got our cases covered so I'll close this issue 🙂