Skip to content

Fix layout bug caused by floating point rounding error#1860

Open
matthieugicquel wants to merge 2 commits intofacebook:mainfrom
matthieugicquel:text-overflow-failing-test
Open

Fix layout bug caused by floating point rounding error#1860
matthieugicquel wants to merge 2 commits intofacebook:mainfrom
matthieugicquel:text-overflow-failing-test

Conversation

@matthieugicquel
Copy link
Copy Markdown

Closes #749 and possibly #1574

The current layout algorithm sometimes rounds down by a small value the size of text nodes, which makes the renderer (at least iOS) skip a line of text, resulting in buggy layout like this:

482035993-9f93908b-7068-49eb-aae2-759b55294ced

I don't have a deep enough understanding of the algorithm to know if this fix is "correct", but what I can say is that:

  • it didn't break existing tests
  • we didn't get any bug reports while using it in a production app

I only applied the change to the height calculation as I only encountered the issue on height, but I suppose width could benefit from a similar change.

@vercel
Copy link
Copy Markdown

vercel Bot commented Oct 14, 2025

@matthieugicquel is attempting to deploy a commit to the Meta Open Source Team on Vercel.

A member of the Team first needs to authorize it.

@meta-cla
Copy link
Copy Markdown

meta-cla Bot commented Oct 14, 2025

Hi @matthieugicquel!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@IgorTodorovskiIBM
Copy link
Copy Markdown
Contributor

Can we create a new release of yoga? see #1859

@matthieugicquel
Copy link
Copy Markdown
Author

This PR in react-native seems to fix the same issue (it's been merged then reverted) facebook/react-native#54260

YGNodeCalculateLayout(node, YGUndefined, YGUndefined, YGDirectionLTR);

// If this value is anything less than 96, iOS will not wrap the text to a 4th line
ASSERT_FLOAT_EQ(YGNodeLayoutGetHeight(node), 96.0f);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the value without the change? I think the intent was that we don't round down nodes with measure function during pixel grid rounding.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An issue I've encountered on the Android side, is that we could round up, and create a TextView, with a layout pixel larger than the one used for layout, which could cause mismatch.

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.

Layout bug caused by floating point rounding error

3 participants