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

Regression with advanced text renderer and line height. #487

Open
itsjamie opened this issue May 25, 2023 · 1 comment
Open

Regression with advanced text renderer and line height. #487

itsjamie opened this issue May 25, 2023 · 1 comment

Comments

@itsjamie
Copy link
Contributor

itsjamie commented May 25, 2023

The original PR #378 added better support for text rendering of multi-line text and respecting the canvas size when calling Canvas2D.drawText to not cause a breaking change for everyone, the PR was focused on the advanced text renderer and was opt-in behavior. As part of that when the line height was larger than the font the size of the canvas that the text was positioned in was sized by the line height.

This commit removed that behavior 4f0b248.

Now, the canvas size of the text is larger than it should be. See PR #378 for the original screenshots showing the screenshots demonstrating the canvas sizing.

Code from PR would render as such:
image

After upgrading same code now renders same result as:
image

@itsjamie itsjamie changed the title Regression with fontBaselineRatio and advanced text renderer for line height. Regression with advanced text renderer and line height. May 25, 2023
@uguraslan
Copy link
Contributor

Hey @itsjamie, thanks for bringing up this issue and your explanations.

From my understanding, PR #378 was initially included in the v2.7.0 release. However, during the development process leading up to the v2.9.0 release, issue #428 was reported and subsequently addressed by PR #432, which was included in v2.9.0. As a result, PR #432 made modifications to the behavior introduced by PR #378, specifically the code below was removed:

else if (renderInfo.lineHeight > fontSize){
   renderInfo.h = renderInfo.lineNum * renderInfo.lineHeight
}

To better understand the recent change (#432) and its impact on the behavior, it would be great if we could get some feedback from @g-zachar and gather more details from his perspective.

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

No branches or pull requests

2 participants