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

fix: text() swallows characters in CHAR wrap mode #7437 #7438

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

top-mind
Copy link

Resolves #7437

Changes:

diff --git a/src/core/p5.Renderer.js b/src/core/p5.Renderer.js
index 21ce3bd64..a08966639 100644
--- a/src/core/p5.Renderer.js
+++ b/src/core/p5.Renderer.js
@@ -390,9 +390,9 @@ class Renderer extends p5.Element {
               line = `${chars[charIndex]}`;
             }
           }
+          nlines.push(line);
         }
 
-        nlines.push(line);
         let offset = 0;
         if (this._textBaseline === constants.CENTER) {
           offset = (nlines.length - 1) * p.textLeading() / 2;
@@ -423,16 +423,16 @@ class Renderer extends p5.Element {
               line = `${chars[charIndex]}`;
             }
           }
+          this._renderText(
+            p,
+            line.trim(),
+            x,
+            y - offset,
+            finalMaxHeight,
+            finalMinHeight
+          );
+          y += p.textLeading();
         }
-        this._renderText(
-          p,
-          line.trim(),
-          x,
-          y - offset,
-          finalMaxHeight,
-          finalMinHeight
-        );
-        y += p.textLeading();
       }
     } else {
     // Offset to account for vertically centering multiple lines of text - no

Screenshots of the change:

Screenshot from 2024-12-21 08-06-30

PR Check

list

Copy link

welcome bot commented Dec 21, 2024

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page!

@davepagurek
Copy link
Contributor

Thanks @top-mind! As we move towards a 2.0 release in the new year, a lot of refactoring is being done in the dev-2.0 branch. @dhowe do you think it would make sense to try to do this change in dev-2.0 instead, since I believe this section of code has moved in that branch?

@dhowe
Copy link
Contributor

dhowe commented Dec 21, 2024

I don't see this issue in v2.0:

function setup() {
    createCanvas(100, 100);
    textWrap(CHAR);
    text("1\n222\n3", 0, 0, 20);
}
image
function setup() {
    createCanvas(100, 100);
    textWrap(CHAR);
    textSize(15);
    text("1\n222\n3", 0, 0, 20);
}
image

@davepagurek
Copy link
Contributor

ok nice! @ksen0 @limzykenneth do you think we'll be doing another 1.x release before 2.0? if so, it could make sense to merge this to get the fix in for users ahead of 2.0.

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

Successfully merging this pull request may close these issues.

text() swallows characters in CHAR wrap mode
3 participants