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

textBounds outputs nan if number is input instead of string #6298

Open
1 of 17 tasks
micuat opened this issue Jul 26, 2023 · 8 comments
Open
1 of 17 tasks

textBounds outputs nan if number is input instead of string #6298

micuat opened this issue Jul 26, 2023 · 8 comments

Comments

@micuat
Copy link
Member

micuat commented Jul 26, 2023

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
  • Internalization
  • Friendly Errors
  • Other (specify if possible)

p5.js version

1.7.0

Web browser and version

FF 115.0.2

Operating System

mac 12.6

Steps to reproduce this

Steps:

  1. input number to textBounds

Snippet:

https://editor.p5js.org/micuat/sketches/sYWYkUTJv

let f;
function preload() {
  f = loadFont("RobotoMono-VariableFont_wght.ttf")
}
function setup() {
  createCanvas(400, 400);
  console.log(f.textBounds("io", 0, 0, 10))
  console.log(f.textBounds(10, 0, 0, 10))
}

function draw() {
  background(220);
}

console:

{x: 0.9912109375, y: -7.2021484375, h: 7.2998046875, w: 10.41015625, advance: 0.9912109375}

{x: Infinity, y: Infinity, h: -Infinity, w: 0, advance: Infinity}

I'm not sure what is the expected result (number + "" would solve the issue). If number is not accepted, there should be an error message, I suppose?

@Qianqianye
Copy link
Contributor

Thanks @micuat for reporting the bug. I'm tagging this year's GSoC contributor @munusshih who's working on typography related issues, and the mentors @kyeah @aahdee @limzykenneth.

@adityagarg06
Copy link

The initial PR that I made had some issues, so I closed it and made a new one. You can delete the previous one.

@limzykenneth
Copy link
Member

limzykenneth commented Jul 27, 2023

@adityagarg06 I would suggest discussing potential fixes here first before making any implementation as the approach filed in a PR may not be the one we preferred.

@adityagarg06
Copy link

@limzykenneth Thank you for your response will surely take care of that.

@kyeah
Copy link
Contributor

kyeah commented Jul 29, 2023

It seems like there's a couple options here:

  • FES Validation Error
  • Type coercion to string
  • Return a default value

Different typography methods do different things —

  • text() will do type coercion to string (even if it's an object)
  • methods like textLeading will do type coercion to number if possible, otherwise return a FES validation error
  • methods like font.textToPoints will return a default value (empty array)

so I could see probably forcing type coercion to string (to match the text function). It also seems like textToPoints should be coercing parameters into a string to match text()?

I did notice that even with an empty string, textBounds() will return Infinity.

Screen Shot 2023-07-29 at 11 35 13 AM

@limzykenneth
Copy link
Member

@micuat Raised a good point in the PR that string coersion for basic types like numbers will work fine but how are we to handle object types? And as @kyeah also identified the same above, my opinion is also the same that when it comes to type coersion for textBounds(), its behaviour should match text() with a FES validation error.

@dhowe dhowe mentioned this issue Nov 10, 2024
@dhowe
Copy link
Contributor

dhowe commented Nov 14, 2024

what is the consensus here? do we want to support non-string types in text() for v2.0 ? or just primitives (string, number, boolean) ? one easy option is to simply call toString() on anything not a string

@limzykenneth
Copy link
Member

I believe in some parts of the library where string or number is expected there is an attempt to try to convert the type such as with toString(). FES parameter validation will catch the mismatched type and print a warning message but we can try and do a simple coersion in the function itself for basic types.

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.

6 participants