Skip to content

Conversation

techninja
Copy link
Contributor

@techninja techninja commented Jun 21, 2016

Note on new PR style: Modern software projects use a structure pull request where the developer in charge of initially creating the work, upon submission in a PR summarizes all notable changes made by the work, and includes verification steps for testing, for another developer to run through and test/verify by hand, along with code review. This will become the standard for this project and others immediately associated moving forward.

This PR does the following:

  • Refactors all fill algorithms out of paper.auto.fill.js and into their own files paper.auto.fill.line.js for dynamic line fills, and paper.auto.fill.overlay.js for spiral fill.
  • Adds the brand new fill algorithm controller/handler in paper.auto.fill.cam.js from the JSCUT library addition taken from PancakePainter work.
  • Upgrade the Clipper Library for polygonal path operations. Likely doesn't change much except fixing a few bugs with outlier paths.

robopaint _042

To verify:

  • Checkout this branch into your RP dev env.
  • Run RP via npm start
  • Head over to Settings -> Style -> Fill and select the new option "Cam (shape offset)".
  • See that the preview updates and displays the appropriate "cam" style fills.
  • Change the various fill options and see that the preview responds appropriately.
  • Exit settings and go to print mode, opening an example SVG file.
  • Click print and see that it generates the fill/strokes appropriately.
  • Actually do a few prints and see how well the fill works by default with default settings.

The conversion between the width settings and the output might not be right, so if the correct width setting for line fill isn't working correctly for this new fill type, we'll need to finesse it a bit.

@oskay
Copy link
Contributor

oskay commented Jun 22, 2016

Where did the name "cam" come from in this context? CAM is the general process of converting a design into a toolpath-- I would not expect it to refer to a specific type of toolpath (and most of my CAM software can't even do this kind of offset-based toolpath). I would recommend the name "Offset" instead, unless there is a compelling reason to keep it this way.

@techninja
Copy link
Contributor Author

I couldn't find much info on the definition for "Cam" as it was being used in the library, so I just kept it. We can move to just "Shape Offset" for the fill type name if you think that fits.

@techninja techninja merged commit 3288dcf into master Jun 24, 2016
@techninja techninja deleted the camfill branch June 24, 2016 21:00
@oskay
Copy link
Contributor

oskay commented Jun 24, 2016

This is fantastic work!

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.

3 participants