timjs / elm-collage

Create interactive vector graphics and position them relative to each other
BSD 3-Clause "New" or "Revised" License
58 stars 19 forks source link

Add sketchy transformation #44

Open chipjacks opened 3 years ago

chipjacks commented 3 years ago

Based on excalidraw, rough.js, and this research paper, this allows transforming a collage into a randomized sketchy, hand-drawn version.

The only change I've made to the core modules is to add an API for drawing a curve through a set of points. Admittedly, I don't know much about all the different ways to accomplish this, so I just opted for the simplest implementation I could find.

I've added a way to toggle examples between normal and sketchy rendering and I think the flowchart example best captures the look I was shooting for:

Screen Shot 2021-05-21 at 11 10 57 AM

This is still pretty rough around the edges (hehe), and I'm not entirely sure it belongs inside this package. Maybe would make more sense as a fork or some kind of extension - open to ideas! @timjs any thoughts?

timjs commented 3 years ago

Hi @chipjacks! Looks really nice! I hope to take a look into this the coming week.

chipjacks commented 3 years ago

Okay, no rush. I've been working on it a bit more and just finished adding hachures for fills.

PR is getting pretty big - if you'd like I can separate out all the Sketchy logic and then either leave it on a fork, or look into opening a new PR to expose the API I'd need to build this on top of elm-collage but as a separate package.

Right now this is all just a fun experiment, but I am kinda hoping to get it to a good enough place that people could use it if they want.

timjs commented 3 years ago

I'm really sorry for the delay, a week turned into a month. A clear sign I should transfer ownership to a new maintainer :-P

It's indeed a quite big PR at the moment. Best would be to split it in a sketchy and in a curves part, but I can imaging this could be quite some work to split it in two separate PRs, and I'm quite happy to merge it this way. It helps that it's just a minor update in the end.

I'll review your changes today and come back to you!

timjs commented 3 years ago

BTW, what I forgot to write down yesterday before a colleague walked in, the curve function is more a "smooth line" than a general implementation of Bezier curves isn't it? If I understand correctly, it automatically smooths a line, but you can't add control points to have full control over the line. I like this feature, but we should come up with an other name for it!

chipjacks commented 3 years ago

the curve function is more a "smooth line" than a general implementation of Bezier curves isn't it?

Yeah, that's correct. Maybe rename it to smoothPath?

On a related note, if we added a real bezier curve API as discussed in #8, then I could improve the browser performance significantly by rendering the hachure fills as one svg path using a bunch of M x y commands, instead of a list of separate path elements as I'm currently doing. Probably just leave that for a separate PR though.

chipjacks commented 3 years ago

Thanks for the review @timjs! I've made the changes you requested.

chipjacks commented 3 years ago

As elm-format is the community standard, I could run it on all source files and make a commit on master, you can do the same and rebase on that? Or would that give us a lot of whitespace errors?

Yep, I tried rebasing but it results in too many whitespace errors. An alternative could be to run elm-format on both branches separately - it looks like that leads to a clean diff without all the unrelated whitespace changes.

timjs commented 2 years ago

Hi @chipjacks,

Sorry for the delay, was on a holiday ⛰🥾

Merged your other two pull requests. Next steps will be to rebase this one on current master, and I'll merg it!

Thanks for all the great work and the constructive discussions 😄