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(fabric.text) fix rtl/ltr performance issues #7610

Merged
merged 2 commits into from
Jan 15, 2022
Merged

Conversation

asturur
Copy link
Member

@asturur asturur commented Jan 15, 2022

closes #7435

As pointed out by @SLKnutson we are spamming calls of canvas.setAttribute('dir') and this causes the browser to do extra work.
We put the calls in the render cycle because we want to support mixed bidirectional test, but for now no one is working on them.
Brought the attribute at top level in the render cycle, inside the stack save, and it gets changed only if needed.
I could it bring it higher up but this restores the performances enough, and anyway the final destination is to handle this on a per style chunk basis.

Before code changes

image

after code changes

image

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2022

Code Coverage Summary

> fabric@4.6.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.38 |    77.09 |   86.16 |    83.1 |                                               
 fabric.js |   83.38 |    77.09 |   86.16 |    83.1 | ...,30174,30299,30379-30444,30567,30666,30883 
-----------|---------|----------|---------|---------|-----------------------------------------------

@asturur asturur changed the title justpush fix(fabric.text) fix rtl/ltr performance issues Jan 15, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2022

Code Coverage Summary

> fabric@4.6.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.38 |    77.11 |   86.16 |    83.1 |                                               
 fabric.js |   83.38 |    77.11 |   86.16 |    83.1 | ...,30172,30297,30377-30442,30565,30664,30881 
-----------|---------|----------|---------|---------|-----------------------------------------------

@asturur asturur merged commit a816eed into master Jan 15, 2022
@asturur asturur mentioned this pull request Jan 26, 2022
rockerBOO pushed a commit to rockerBOO/fabric.js that referenced this pull request Jan 28, 2022
@asturur asturur deleted the rtl-performance-issue branch February 4, 2022 17:53
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.

right to left support reduced text performance
1 participant