Issue #421 : Added support for clipping the job to the material boundaries#422
Issue #421 : Added support for clipping the job to the material boundaries#422jarney wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
The ClipFilter here works great if there is only one copy and no transformation is applied, but the svg can be manipulated by rotating/scaling which causes the clip boundary to be changed (make the clip filter return the clip path, and apply a rotation to the job to see what I mean).
To correct this I think there should be another filter subclass that applies to the whole document after individual copies are created (eg in Job.create around https://github.com/inkcut/inkcut/blob/master/inkcut/job/models.py#L434) and the clip filter should use that so it clips everything. Edit: Or if easier it could be builtin to the job instead of a separate filter.
I think the split of svg and path filters is a good idea.
| self.path_filters = self._default_filters(filters.PATH_FILTERS) | ||
|
|
||
| def _default_filters(self): | ||
| def _default_filters(self, filter_list): |
There was a problem hiding this comment.
The _default_<member> methods are special atom methods that are invoked to populate the initial value of the instance value (https://atom.readthedocs.io/en/latest/basis/mangled_methods.html#default-values). This changes causes that to not work an show a warning.
Can this be split into _default_svg_filters and _default_path_filters?
There was a problem hiding this comment.
I ended up calling these "job_filters", but I have now modified to reflect _default_svg_filters and _default_job_filters as requested. I think the warning is gone now too, so that should be fixed now.
| return doc | ||
|
|
||
| @observe('path', 'order', 'filters') | ||
| @observe('path', 'order', 'filters', 'clip_to_plot_area') |
There was a problem hiding this comment.
The observer 'filters' is no longer valid, it should be 'svg_filters', 'path_filters'.
There was a problem hiding this comment.
Changed to svg_filters and job_filters as requested, this should be fixed now.
| @@ -0,0 +1,77 @@ | |||
| """ | |||
| Copyright (c) 2018, Jairus Martin. | |||
There was a problem hiding this comment.
Please update the copyright as noted in the other PR
|
Hmm, yes, I see the problem. Let me take a little bit to think about how to rework the structure. I think perhaps a "JobFilter" which would apply to the job after all transformations and copies have been made would be appropriate. Later, if needed, a "PathFilter" applying to only the SVG shape before transformation can then be introduced to do things like removing redundant path segments or collapsing path segments that are within some tolerance of each other. |
…t_svg_filters and _default_path_filters to match the member-variable names. Changed @observe to listen for 'svg_filters' and 'job_filters' instead of 'filters'. Updated copyright notice in unit-test header. Changed the idea of 'path' filters to 'job' filters and applied them to the entire job, not just the primitive paths that are potentially rotated, copied, or otherwise modified by the job. The 'job_filters' now apply to the finished job as a whole instead of the raw input document.
|
I believe I have now resolved the problem of clipping the whole job instead of just the input document before copies/scaling/rotation. This should now clip the final result to the material size regardless of transformations of the shapes as specified in the job. |
This relates to issue #421. This PR adds support for clipping the drawing to the material boundaries as described in the issue. This helps with ensuring that the plotter head always remains inside a "reasonable" place in the plotter's working area.