-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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(StaticCanvas): fully clean the cache canvas to avoid leaving trailing pixels #9779
Draft
asturur
wants to merge
5
commits into
master
Choose a base branch
from
remove-the-dirty-pixels-cache
base: master
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
Build Stats
|
asturur
changed the title
DRAFT - test run
fix(StaticCanvas): fully clean the cache canvas to avoid leaving trailing pixels
Mar 29, 2024
…abric.js into remove-the-dirty-pixels-cache
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR changes the code that cleans the cache canvas between one frame and another when there is no need to fully wipe the canvas element.
Fabric relies on the knowledge of how large is the drawing and how much is zoomed to clean the canvas correctly.
When we loose understanding of the size of the element ( the case with strange strokes and acute angles ) we don't clean good enough.
This code simplify the cleaning procedure with simply a full canvas wipe in a non transformed state.
It also deletes cacheWidth and cacheHeight that seems like they were around for this exact purpose and nothing else.
This bug wasn't really common before, but with the modifyPoly controls and the nested selections became more common.
It is also very easy to fix and reduces code instead of adding it, so probably a win/win
in Master:
in this branch:
As you can see from this picture the triangle will still bleed out but won't leave the trail when moved.
Does it perform differently?
I'm not sure how to trigger it or measure it properly.
This change triggers when we invalidate the cache but we don't rebuild the canvas.
So i probably have to build a specific use case just to be sure
in this branch:
In Action