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(FabricObject): Render clipPath as sharp as the object #9774

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

asturur
Copy link
Member

@asturur asturur commented Mar 28, 2024

Description

replace #7186

This PR prescale the canvas of the clipPath in order to be as scaled up as the object BEFORE drawing the actual Clippath on top of it.

In Action

Copy link

codesandbox bot commented Mar 28, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link
Contributor

github-actions bot commented Mar 28, 2024

Build Stats

file / KB (diff) bundled minified
fabric 915.658 (+0.803) 305.835 (+0.293)

@asturur asturur marked this pull request as draft March 28, 2024 23:42
@asturur
Copy link
Member Author

asturur commented Mar 28, 2024

The difference is great:
image

but need to write the code in a way it doesn't fail for non cached object

@asturur
Copy link
Member Author

asturur commented Mar 29, 2024

Also a bunch of strange results that needs investigations

@asturur
Copy link
Member Author

asturur commented Mar 29, 2024

Note for myself:
i need an svg with overlapping clipPaths so that i can measure regression clearly and measure nested scaling

@asturur
Copy link
Member Author

asturur commented Apr 4, 2024

Did some test, scaling by zoomX and zoomY while works for a single nested clippath is not the way to go because it is too hard carry over the nested scaling to inner clippaths.

In theory the main cache canvas of the object is always big enough ( and of course too big, but never too small )
The first clipPath start drawing at cacheTranslationX/cacheTranslationY.

The nested one needs to start drawing in a canvas that is the same as that one, but needs to carry over the transformation of the first clippath and then the second.

@asturur
Copy link
Member Author

asturur commented Apr 4, 2024

image image

Svg vs rendering.
While it is approaching, i don't think this code is correct.
As it is it can only work for when there are no nested clippaths.
That would still better than nothing

@asturur
Copy link
Member Author

asturur commented Apr 4, 2024

oh well now it matches

image

but yet is not ifinitely recursive, so isn't a great idea.

But is a step forward

@ShaMan123
Copy link
Contributor

ShaMan123 commented Apr 8, 2024

I know this is WIP but I wanted to suggest using parent instead of clipPathParent

@ShaMan123
Copy link
Contributor

It will still respect the fact that a parent is unique

@asturur
Copy link
Member Author

asturur commented Apr 8, 2024

the clipPathParent is a temporary notation to see if things could work.
Won't go anywhere.
I wanted to experiment if i could at leat get the drawing correct and understand which data i need to render correctly

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.

None yet

2 participants