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

feat: support background fill for freedraw shapes #4610

Merged
merged 10 commits into from
Feb 9, 2022
Merged

Conversation

h7y
Copy link
Member

@h7y h7y commented Jan 16, 2022

fix #4592

chrome_LR9SV7RT36.mp4

@vercel
Copy link

vercel bot commented Jan 16, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

excalidraw-package-example – ./src/packages/excalidraw

🔍 Inspect: https://vercel.com/excalidraw/excalidraw-package-example/J7pSJ5GhRHgqt1BjoDPW1NGNpj4e
✅ Preview: https://excalidraw-package-example-git-support-fill-freedraw-excalidraw.vercel.app

excalidraw – ./

🔍 Inspect: https://vercel.com/excalidraw/excalidraw/6t1jYjuUB1wFdoDBGnaZA31LmC5j
✅ Preview: https://excalidraw-git-support-fill-freedraw-excalidraw.vercel.app

@h7y h7y marked this pull request as ready for review January 16, 2022 14:48
@h7y
Copy link
Member Author

h7y commented Jan 16, 2022

Just noticed that the fill style is not working. Looking into it.

@dwelle
Copy link
Member

dwelle commented Jan 16, 2022

Very cool @h7y!

Just noticed that the fill style is not working. Looking into it.

You'd need to use rough-js renderer for this.


Also, let's make sure that filled freedraw shapes are selectable from inside. Don't recall where in the checks you need to enable this.

@dai-shi
Copy link
Member

dai-shi commented Jan 16, 2022

This looks cool.
(This may require excalidraw-animate to have some new logic.)

@h7y
Copy link
Member Author

h7y commented Jan 18, 2022

Fixed fill style and made the freedraw shapes with background selectable from inside.

firefox_97IltXUnDp.mp4

One thing I noticed is sometimes the fill goes slightly out of the shape. Trying to figure out why that happens.
Capture

@h7y
Copy link
Member Author

h7y commented Jan 20, 2022

One thing I noticed is sometimes the fill goes slightly out of the shape. Trying to figure out why that happens.

Okay this is already happening on all existing shapes.

From Excalidraw.com

From Excalidraw.com

@dwelle
Copy link
Member

dwelle commented Jan 20, 2022

Strangely, the hitbox doesn't work when the fill is solid --- unclear what's happening

@h7y
Copy link
Member Author

h7y commented Jan 23, 2022

Found out why this is happening. It's because when the fill style is solid most of the curve path ops of the filled shape have op value as "lineTo" which is not yet handled in the hitTestRoughShape function.

} else if (op === "lineTo") {
// TODO: Implement this

I came up with something that somewhat works which I'll push shortly. Trying to understand how this all works to come up with a proper solution.
firefox_ERnEsYuAUr

@dwelle
Copy link
Member

dwelle commented Feb 9, 2022

Ad the collision check — I've reused hitTestCurveInside() which gives much more accurate results (even if it's not meant to be used with freedraw shapes). For example, on the one below, the check correctly reports false when you move inside the horseshow:

image

https://excalidraw-fmuyhaoqx-excalidraw.vercel.app/#json=FKUqS6rIx6IU2zyUixVbZ,joS7GdzobemjGPstuPwOtA

@dwelle dwelle merged commit 0cdd0ee into master Feb 9, 2022
@dwelle dwelle deleted the support-fill-freedraw branch February 9, 2022 16:43
@dwelle
Copy link
Member

dwelle commented Feb 9, 2022

Thanks for the PR (and sorry for the delay!) ❤️

@h7y
Copy link
Member Author

h7y commented Feb 9, 2022

Thanks for taking it up and making the changes! :) ♥

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.

support background fill for freedraw shapes
3 participants