Closed LeLocTai closed 2 years ago
Merging #16 (4850a12) into master (4215e47) will decrease coverage by
2.24%
. The diff coverage is27.58%
.
@@ Coverage Diff @@
## master #16 +/- ##
==========================================
- Coverage 58.72% 56.47% -2.25%
==========================================
Files 5 5
Lines 940 988 +48
==========================================
+ Hits 552 558 +6
- Misses 388 430 +42
Impacted Files | Coverage Δ | |
---|---|---|
src/main.rs | 32.06% <0.00%> (-1.46%) |
:arrow_down: |
src/converter.rs | 66.10% <32.00%> (-7.93%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 4215e47...4850a12. Read the comment docs.
Could you clarify what problem this is solving? If there is no width and height on the root, is it not being scaled properly?
Yes. The path coordinates are normalized to 0-1 based on the viewport. Without anything to scale them back up, the gcode will range from 0-1mm
Ok, I'd call that a bug then. I didn't really look into viewBox vs. width + height at the time of writing this, but my current understanding is as follows (correct me if I am wrong):
So then, there should be no scaling with the viewBox, only with width & height.
No the current implementation is correct. viewBox should be used to normalized the coord inside it. However, when width and height are not defined on the outer most svg element, the specs is unclear. I assume the initial value for width and heigth is auto (or 100%), at least that what browsers are doing. As we don't know the size of the machine's work area, allow user specified size seem useful.
Cool that makes sense, thanks for clarifying
I want to be able to specify the final size, not a scale. I found that to be much more convenience to use. If width and height attributes are present in the root svg element, we would need access to those to implement resizing. This is is not available outside of converter.rs
, unless we re-parse the svg.
I usually specify only one of width or height, taking advantage of the aspect ratio keeping feature. Aspect ratio come from viewBox which is also not available outside of converter.rs
, unless we re-parse the svg.
Importantly, I find working directly with gcode not very clean. If converter.rs
is too big, we can probably split the transform handling path to another file.
Sidenote: the offset function can easily be done with a G92
Importantly, I find working directly with gcode not very clean. If converter.rs is too big, we can probably split the transform handling path to another file.
Yeah I agree with you. Now that I'm thinking about this again, the scaling really needs to be done before so that the tolerance from lyon_geom is applied correctly.
I want to be able to specify the final size, not a scale.
Would you be fine calling it --size 100,100
then?
Sidenote: the offset function can easily be done with a G92
True, hadn't thought about doing it that way before. I think shifting them manually is still valuable, as it avoids any confusion when inspecting the output.
--size 100,100
is fine. What would be the best way to specify auto size?
100,
100,-1 (ffmpeg)
100,auto (css)
I'm leaning toward the first one.
Agreed, first one looks good. If you wanted auto-width would it be ,100
?
Yeah that sound good.
Nice, i was about create same issue :) i had similar problem when my SVG app is not exporting correct size and/or viewport in svg. I will happy if this will be implemented
@LeLocTai how is this going? Things have changed a lot since this PR was opened, so wanted to check if you are planning to rebase.
You can make the changes to lib alone if you prefer and I can update the web+cli
Sorry I don't have any plans to work on this at the moment.
Ok, no worries!
I have pushed changes for this feature on the CLI -- web interface support is a WIP.
Let me know if you have any questions and/or suggestions!
@sameer thanks for merging, i will try, but can you please add how to use parameters to readme? I see in code that i can use --dimensions
parameter and mm
as unit, so --dimensions=210mm,297mm
? I am correct?
@sameer thanks for merging, i will try, but can you please add how to use parameters to readme?
Sure, I can create a wiki page explaining how to use the various CLI features and link to that from the readme. How does that sound?
I see in code that i can use
--dimensions
parameter andmm
as unit, so--dimensions=210mm,297mm
? I am correct?
That's correct, any absolute length unit measurement is accepted.
Certain tool I'm using to generate svg does not output width and height attribute in the root
For svgs with these attributes, being able to quickly resize drawing would still be useful I think.
This implementation will allow specifying either width or height, keeping aspect ratio; or both to stretch the result. The aspect ratio come from the first viewBox, not sure if that is a good assumption.