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

[ Feature Request ] Option to load paint on new sub-path instead of distance-based #118

Closed
gwygonik opened this issue Dec 1, 2013 · 13 comments

Comments

@gwygonik
Copy link
Contributor

gwygonik commented Dec 1, 2013

For certain artistic uses and to keep strokes as continuous as possible, it would be useful to be able to choose to reload paint on new sub-paths instead of the distance-based default which increments across sub-paths. While I believe this can be done in manual paint mode by first loading paint then stroking individual sub-paths, this can be quite time consuming with pictures that have many strokes, or are unable to be easily broken apart into separate paths.

However, as this may create a larger-than-normal number of paint reloads, it can wear down the paint quickly. This can be solved by adding the ability to select between a full reload or a dip reload (dip water, dip paint) after the initial full paint load.

This process may increase the time it takes to paint a picture, but as an option it would be up to the user to decide which they prefer -- speed or stroke continuity.

I've successfully implemented the code to do the first bit -- reload on new sub-path -- but am unable to get preferences, UI elements, or dip option working correctly. I'm adding this item as a feature request so the folks who know what they're doing can weigh in. I'm more than happy to provide my code, but it's a hack. I'm also happy to work on it properly if I can get some knowledge assistance. :-)

The pic below was painted with my reload-on-new-sub-path code -- note all the sub-paths 1) start with fresh paint on the left and are continuous across the image, and 2) get slightly thicker as each sub-path does a full paint reload (a dip would suffice):

wcb_perlin_new_1

@oskay
Copy link
Contributor

oskay commented Dec 1, 2013

We have separately had this request in the Inkscape context, and have an open issue there to add this capability:
evil-mad/wcb-ink#24

@techninja
Copy link
Contributor

Sweet! I like it. I'd imagine this could just be a checkbox in the main settings for "prefer paths/subpaths over distance". And yes, I do apologize for the state of the settings code, It's grown into quite a monster and no longer belongs in the main.js at all. Bit of a total mess.

@gwygonik if you've ever used GitHub before, the standard practice is to fork the project, and with this copy, you can make your edits directly, push them back up, and then make a pull request. I can then look over the changes, make comments and additions, and can even put it directly into the project from there. It's awesome!

Lemme know if any of that freaks you out, I have no problem walking you through it if need be. All hacks welcome and appreciated, we're all learning here 👍

@gwygonik
Copy link
Contributor Author

gwygonik commented Dec 1, 2013

Added a pull request with my changes. There is now UI, settings, logic, and some placeholders. I couldn't get the dip methods working, but the whole reload method choice thing should be refactored into the getMorePaint method instead of three different places. Thanks much! :-)

@techninja
Copy link
Contributor

Feel free and re-submit your PR whenever you have a minute, I'd love to see if we can still get this in. My time available to work on new features like these is unfortunately fairly limited as I have to concentrate on bugfixes and features in with advancing the direction of the app to allow for easier integration. Aiming to get 0.7.5 out before the end of the month.

@gwygonik
Copy link
Contributor Author

I revoked my original PR due to it being too aggressive in what it considered a new path... I haven't been able to get back to explore fixing that, so I've not wanted to re-submit. That, and now I'm 2 versions behind. :-) I can re-submit if you feel like merging/looking, but I don't want to disrupt your dev time.

@techninja techninja added this to the 0.9.7 (Pie in the sky) milestone Dec 3, 2014
@techninja
Copy link
Contributor

@gwygonik I know this is nearly a year old now, was wondering if we could try to work this back in? It's a great feature that seems like It'd be awesome to have as a default (the exacting & uncaring method currently used technically works, but produces unnatural strokes). I'm sure with some effort we could get it going again, the reinking logic is now a bit easier to figure out and all living right here.

@gwygonik
Copy link
Contributor Author

gwygonik commented Dec 6, 2014

Let's do it! I haven't looked at recent code to see what's changed (assuming a lot over the last year!), nor have I updated my app, but I'm sure we can get it going. I'll pull latest and see what's what. :-)

@techninja
Copy link
Contributor

Sweet! Can't wait.

@gwygonik
Copy link
Contributor Author

gwygonik commented Feb 8, 2015

Hey folks. I have this working in 0.9.2, though I haven't run that many SVGs through it. I'm going to get a fork of 0.9.2 proper and send a pull request. I'd ask that the changes be looked over and more sanity tests run. 😃

@techninja
Copy link
Contributor

Wonderful! And good luck. Big changes are afoot, but this should continue to be useful and relevant as we move forward for more bot types support. Feel free and ask any questions about how things work or if you have any issues or need some testing here :) 👍

@gwygonik
Copy link
Contributor Author

gwygonik commented Mar 6, 2015

Ok! Looks like I got all the options working -- stroke each path and full brush reload vs single dip vs double dip. I did NOT get translations (I'd have to Google translate). Will do a pull probably tomorrow night.

@techninja
Copy link
Contributor

I'll take a look and see if I can get this in :)

@techninja techninja modified the milestones: 0.9.6 Bugfix release, 0.9.7 Mar 12, 2015
@techninja
Copy link
Contributor

Looking great! I'm going to do some minor cleanup to this and It'll be in the 0.9.6 bugfix release coming up. Will close this ticket when I'm done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants