jschobben
commented
3 years ago Thanks for this PR! Let me comment on each "topic".
parallel used for doing renders in parallel
Indeed the old backgrounding code is wrong, it made the false assumption that if x jobs finish, then wait -n won't block for x calls. That alone causes less jobs running in parallel than expected, and maybe also the issue you saw. It's fixable with pure bash, but will add even more complication to the script.
Using something like GNU parallel, with fallback if it's missing, does seem the better option. However its "citation" demand is a bit overbearing IMO (we're not doing academic research on parallel algorithms in here, are we 😛), and the output would probably confuse a colorscad user who doesn't usually use parallel and didn't run parallel --citation to silence it. Maybe we should just add --will-cite with a nice comment above it? Hopefully that option is portable enough.
Options replace the argument positions
Great! Maybe the "$@" syntax can also be used to forward args to openscad with less quoting trouble, have to try it myself.
Fix for openscad file not in current working directory
This nasty trick was done to make it work with Windows openscad, I suspect the fix might break Windows. It wasn't a perfect hack either. There are also still some issues such as trying to include a file (stl, font) in your .scad file. If the fix indeed breaks Windows, maybe we should just make a separate issue for this.
Option to overwrite output
Fair enough!
More output
Always good to have extra output, might indeed be best to hide it behind a flag though.
Whitespace changes...
This will always remain a religious thing, and to make it worse for some reason I tend to use tabs in shellscript and spaces in c++ code 🤣. Anyway I don't mind if it's tabs or spaces, as long as it's consistent. Maybe best to not change spacing together with "real" changes, though.
This is a large change to the bash script. Feel free to do with it what you will, use it as an alternative version, replace the current version, use parts of it, whatever you want. This works way better for my needs but YMMV. Tested on a Debian based system, unknown if it works on Mac/Win/etc.
Main changes
This replaces the backgrounding with
parallel. (The script was failing for me due to the current back-grounding method). The code is a lot less complex. However it adds in a new requirement that you installparallelif you wish to do parallel rendering. It does fall back to single threaded if you don't have parallel installed. So if rendering inparallelis not needed you can use the script normally.I also added in getopts to take in options as I needed to pass through var's into openscad while generating objects. It was going to be too complex to deal with positional args and options so I just made everything an option with input and output options required.
The original bash also fails if the openscad file for input is outside your CWD. This is because openscad puts its output in the same place as its input when given a relative path. Openscad output was changed to put the file into the temp directory instead of your CWD which avoids the issue. Cleanup was reduced to deleting the tmp directory.
I was annoyed that I had to always delete the existing output to re-generate, so I added a
-foption to do that for me.This is a bit more verbose as I added messages while debugging. I decided I liked the output so I just left it. It could potentually be hidden behind a verbose flag I suppose.
Sorry, I tend to like spaces so I wrote my bits with spaces and updates some of the tabs while I was around. You might want to sed out " " for "\t" to bring back your tabs if desired.