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

textToPoints not working with letter "B" from Google Font #5462

Closed
1 of 17 tasks
pifragile opened this issue Oct 27, 2021 · 7 comments
Closed
1 of 17 tasks

textToPoints not working with letter "B" from Google Font #5462

pifragile opened this issue Oct 27, 2021 · 7 comments

Comments

@pifragile
Copy link

Most appropriate sub-area of p5.js?

  • Accessibility (Web Accessibility)
  • Build tools and processes
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Friendly error system
  • Image
  • IO (Input/Output)
  • Localization
  • Math
  • Unit Testing
  • Typography
  • Utilities
  • WebGL
  • Other (specify if possible)

Details about the bug:

  • p5.js version: 1.4.0
  • Web browser and version:Chrome 94.0.4606.81
  • Operating System: MacOSX
  • Steps to reproduce this:

I loaded the Google Font "Dancing Script" and used textToPoints on it. It works well for all letters, but with letter "B" no points are returned. When simply using "text" with the same font, the letter "B" prints as expected. So i suppose it is not a problem with the font.

Please refer to https://editor.p5js.org/pifragile/sketches/l0JS6TCks. It is all setup there. You can take a look at the console output and you can see that for letter "B" an empty array of points is returned.

@pifragile pifragile added the Bug label Oct 27, 2021
@welcome
Copy link

welcome bot commented Oct 27, 2021

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, be sure to follow the issue template if you haven't already.

@dhowe
Copy link
Contributor

dhowe commented Oct 29, 2021

The should be resolved with #5466

Explanation: the issue here is the final line of the isSpace() function which checks that the glyph's index is not the literal number 3. I'm not sure why this was originally included in the code (there is no comment), but I have run all the tests without it and see no adverse affects. It is likely however that there was a valid reason for including it originally, so we should keep an eye out for problems its removal may cause. I've also fixed some tests which were broken when fonts were moved around.

function isSpace(i) {
  return (
    (glyphs[i].name && glyphs[i].name === 'space') ||
    (txt.length === glyphs.length && txt[i] === ' ') ||
    (glyphs[i].index && glyphs[i].index === 3) // PR removes this line
  );
}

@limzykenneth
Copy link
Member

@dhowe I found that the function came from #2108, do you recall why the checks are written that way?

@dhowe
Copy link
Contributor

dhowe commented Oct 29, 2021

yes, I also looked up that commit -- but no idea where that index check came from

@dhowe
Copy link
Contributor

dhowe commented Oct 31, 2021

it occurs to me that we may need to handle other types of whitespace in that function, let me do a few more tests before we merge

@dhowe
Copy link
Contributor

dhowe commented Nov 12, 2021

So this is fine with single or multiple spaces, including tabs, and can be merged as is
I do notice a possible issue with line breaks, but have created a new ticket for this (#5474)

limzykenneth added a commit that referenced this issue Nov 14, 2021
@limzykenneth
Copy link
Member

Fixed by #5466

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

No branches or pull requests

3 participants