Skip to content

Update text layout cache checking #trivial#930

Open
Adlai-Holler wants to merge 1 commit intomasterfrom
AHTextNodeProblem
Open

Update text layout cache checking #trivial#930
Adlai-Holler wants to merge 1 commit intomasterfrom
AHTextNodeProblem

Conversation

@Adlai-Holler
Copy link
Copy Markdown
Member

Fixes the bug in #553 .

The solution in that diff was actually landed incidentally in #918 which fixes the correctness of the check, but degrades layout performance and memory by recomputing layouts more often.

This diff changes the check to have both the correctness fix and improved performance. I tested the diff against the sample code that @wsdwsd0829 gave and it fixes that case.

It also modifies ASTextLayout to use pixel-ceiling rather than point-ceiling.

@Adlai-Holler Adlai-Holler requested a review from appleguy May 21, 2018 20:06
@ghost
Copy link
Copy Markdown

ghost commented May 21, 2018

1 Warning
⚠️ Any source code changes should have an entry in CHANGELOG.md or have #trivial in their title.

Generated by 🚫 Danger

@Adlai-Holler Adlai-Holler changed the title Update text layout cache checking Update text layout cache checking #trivial May 21, 2018
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jul 9, 2019

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Adlai Holler seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@jparise
Copy link
Copy Markdown
Member

jparise commented Jan 2, 2020

@Adlai-Holler, looks like this could use a rebase.

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.

3 participants