-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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): _getFontDeclaration
#9079
Conversation
fix fix Revert "fix" This reverts commit 656e25f. Update text.js
Build Stats
|
this is a time sink!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
horrible fix
Took so long to find the problem that was unearthed by the fix and made me cleanup the code step by step to find it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During triage I made props passing stricter - meaning the methods pass only what they need and not the entire style decl
stylesAreEqual = | ||
prevChar && | ||
fontDeclaration === this._getFontDeclaration(prevCharStyle!), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
after fixing the trivial bug it took me too long to find and fix this line
fontCache[previousChar] = previousWidth; | ||
if (previousWidth === undefined && stylesAreEqual && prevChar) { | ||
previousWidth = ctx.measureText(prevChar).width; | ||
fontCache[prevChar] = previousWidth; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed vars
Motivation
closes #9010
closes #9041
Description
see #9010
Changes
Gist
In Action