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 on multiline text breaks if global textLeading does not match the provided font size #7147

Closed
1 of 17 tasks
davepagurek opened this issue Jul 26, 2024 · 5 comments
Closed
1 of 17 tasks

Comments

@davepagurek
Copy link
Contributor

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

1.9.4

Web browser and version

Firefox, Chrome

Operating system

MacOS

Steps to reproduce this

Steps:

  1. Load a font
  2. Measure the textBounds() of multiline text, passing in a larger font size than the default (e.g. 50)
  3. The resulting box does not cover the text

Snippet:

In this example, I'm setting the font size on a graphic, so the main canvas still has the default text size:

let message
let fnt
let g


function tightBound() {
  const bbox = fnt.textBounds(message.value(), 0, 0,g.textSize());
  g.noFill()
  g.rectMode(CENTER)
  
  g.rect(0,0,bbox.w,bbox.h)
  console.log(message.value())
}

function preload() {
fnt = loadFont("https://fonts.gstatic.com/s/opensans/v40/memSYaGs126MiZpBA-UvWbX2vVnXBbObj2OVZyOOSr4dVJWUgsg-1y4nY1M2xLER.ttf")
}

function setup() {
  
  createCanvas(windowWidth,windowHeight);
  g = createGraphics(width, height)
message = createElement('textarea');
  message.value('test\nanother')
  message.position(20, 20);
  message.size(300, 200);
  g.textAlign(CENTER,CENTER)
}

function draw() {
  g.background(220);
  g.push()
  g.translate(width/2,height/2)
      g.textFont(fnt);
  g.textSize(50)
  g.text(message.value(),0,0)
  tightBound()
  g.pop()
  image(g, 0, 0)
}

Result:
image

Expected:
image

Cause

In the implementation of textBounds, it gets the text leading from the main instance's renderer:

const lineHeight = p._renderer.textLeading();

I'm not sure the best way to fix this; if you're drawing text onto a different renderer like a graphic, there's not a deterministic way to figure out which renderer to use ahead of time. The best option might be to let you pass that value in, which might require a change to the arguments to this function to not have an overwhelmingly long parameter list.

@dhowe
Copy link
Contributor

dhowe commented Jul 28, 2024

Thanks @davepagurek -- this is a known-issue discussed in #6967
The fix, which will hopefully be included in v2.0, is to accept textLeading (and all other relevant params) as part of options

@davepagurek
Copy link
Contributor Author

Hey @dhowe! Do you know if this is currently in 2.0, or if we should open this up to contributors?

@dhowe
Copy link
Contributor

dhowe commented Dec 21, 2024

The API is a bit different now, as textBounds() is implemented on the renderer rather than p5.Font, and fontSize is not an argument. p5.Font.textBounds() remains for backwards compatibility but delegates to the renderer function:

Renderer.prototype.textBounds = function (str, x, y, width, height);

So if one wanted to use textBounds() for text on a graphics object g, one could simply call it that way: g.textBounds(...).

But this does suggest that p5.Font.textBounds() and p5.Font.fontBounds() should probably also take an optional graphics object) in its options, as textToPoints() and textToPaths() currently do (as in the commented line below).

     let f, g, pts;
      p.setup = async function () {
        p.createCanvas(400, 300);
        f = await p.loadFont("https://fonts.gstatic.com/s/opensans/v40/memSYaGs126MiZpBA-UvWbX2vVnXBbObj2OVZyOOSr4dVJWUgsg-1y4nY1M2xLER.ttf");
        g = p.createGraphics(p.width, p.height)
        g.background(220);
        g.textAlign(p.CENTER, p.CENTER)
        g.textFont(f, 50);
        g.text('Serendipity', g.width / 2, g.height / 2);
        let bb = g.textBounds('Serendipity', g.width / 2, g.height / 2);
        // let bb = f.textBounds('Serendipity', g.width / 2, g.height / 2, { graphics: g });
        g.noFill();
        g.stroke(0);
        g.rect(bb.x, bb.y, bb.w, bb.h);
        pts = f.textToPoints('Serendipity', g.width / 2, g.height / 2, { graphics: g });
        pts.forEach((pt, i) => {
          g.fill(0);
          g.circle(pt.x, pt.y, 2);
        });
        p.image(g, 0, 0);

This is now implemented in 2.0 so that p5.Font.textBounds() and p5.font.fontBounds() both accept p5.Graphics as an optional argument, similar to p5.Font.textToPoints() and p5.Font.textToPaths()

davepagurek added a commit that referenced this issue Dec 21, 2024
Type: implement #7147 and minor refactors
@davepagurek
Copy link
Contributor Author

@dhowe is this one good to close now?

@dhowe
Copy link
Contributor

dhowe commented Dec 23, 2024

all good

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

2 participants