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

Expose SVG Export Options and Add Some Useful Extras #596

Merged
merged 10 commits into from
Jan 28, 2021
Merged

Expose SVG Export Options and Add Some Useful Extras #596

merged 10 commits into from
Jan 28, 2021

Conversation

jmwright
Copy link
Member

@jmwright jmwright commented Jan 20, 2021

I'm still working on this, but wanted to get it out in the open to get feedback.

Exposed the getSVG() 'opt' parameter so that things like height, width and margins could be controlled. Also added a projection direction opt to control the projection view of the shape. The following code currently works with this fork/branch:

import cadquery as cq
from cadquery import exporters

result = cq.Workplane("XY").circle(1.0)

exporters.export(result,
                 "/home/jwright/Downloads/out.svg",
                 opt={"width": 100,
                      "height": 100,
                      "marginLeft": 10,
                      "marginTop": 10,
                      "projectionDir": (0, 0, 1)})

There's been some discussion around SVGs and HLR lately, so feedback is welcome. I'll share on Discord to get feedback as well.

Tagging @adam-urbanczyk and @marcus7070

@marcus7070
Copy link
Member

It looks like you're planning to add an option to display the axis indicators?

@jmwright
Copy link
Member Author

@marcus7070

It looks like you're planning to add an option to display the axis indicators?

I'm actually thinking of taking the axis indicator out because it's too hard to draw it dynamically for each possible projection direction. I'm not sure how much value it adds either.

@adam-urbanczyk
Copy link
Member

adam-urbanczyk commented Jan 21, 2021

I'm actually thinking of taking the axis indicator out because it's too hard to draw it dynamically for each possible projection direction. I'm not sure how much value it adds either.

We could use CQ to render the indicator, say a cube with embossed +-XYZ text on appropriate faces. But maybe that is something for a separate PR and definitely should be optional.

Another thing I realized recently - maybe it'd be good to make hidden dashed line optional. For complex models they add quite some visual noise:

image

@MarcSallent
Copy link

MarcSallent commented Jan 21, 2021

Looks good!
I've been not able to test it yet, but I consider the following options interesting for many folks:

  • Hidden lines:
    • visible / hidden (not transparent, but entirely not drawn)
    • stroke-width
    • stroke (color)
  • Visible lines:
    • stroke-width
    • stroke (color)

These are actually the ones that I found myself editing to preview the SVGs in the browser.
I just found out that you can add both stroke-width and color to the style attribute of the SVG tag, so an option could be to send the CSS attributes instead of many options in the function call

@jmwright
Copy link
Member Author

Thanks for the feedback everyone.

We could use CQ to render the indicator, say a cube with embossed +-XYZ text on appropriate faces. But maybe that is something for a separate PR and definitely should be optional.

I'll add a deprecation warning. I definitely think we can do something better with that indicator in the future. I'm wondering if for now I just go ahead and add an option to show/hide the axes indicator and make it so that it only works with the default direction, and then note that in the docstring. It will be a little confusing for now, but everything will fall into place when a new indicator is added later.

I think I also prefer flat options.

@adam-urbanczyk By "flat options" you mean keyword arguments, correct?

@adam-urbanczyk @marcus7070 Am I understanding correctly that the dictionary opt will still be passed to exporters.export(), but that the dict will be destructured to be passed to the exportSVG/getSVG methods, which will accept keyword arguments instead of a dict?

@adam-urbanczyk
Copy link
Member

I think I also prefer flat options.

@adam-urbanczyk By "flat options" you mean keyword arguments, correct?

@adam-urbanczyk @marcus7070 Am I understanding correctly that the dictionary opt will still be passed to exporters.export(), but that the dict will be destructured to be passed to the exportSVG/getSVG methods, which will accept keyword arguments instead of a dict?

Indeed, that is what I meant. Though after looking at the code I'm not so sure if I still have that opinion. Maybe let's park it for now.

@jmwright
Copy link
Member Author

There's still work to do, but I think I've got all the options added unless there's a desire to have hidden line stroke-width set independently of the visible line stroke-width.

The opts dictionary has to stay in some capacity because of how the exporters interface works, but I may take a shot at getting us set up to benefit from type annotations via keyword arguments on getSVG(). With the way the interface is the opts dict makes sense, but I'd like to be able to have a standard docstring with explanations of each option and type annotations too.

The axes indicator only works if you set showAxes to True and don't change the projectionDir (let it stay at the default). If you don't pass any opts in, the exported SVG will look exactly like it did before I added these changes. The following code shows what options are available, and how it looks.

import cadquery as cq
from cadquery import exporters

result = cq.Workplane("XY").box(10, 10, 10)#.circle(1.0)

exporters.export(result,
                 "/home/jwright/Downloads/out.svg",
                 opt={"width": 100,
                      "height": 100,
                      "marginLeft": 10,
                      "marginTop": 10,
                      "showAxes": False,
                      "projectionDir": (-1.75, 1.1, 5),
                      "strokeWidth": 0.25,
                      "strokeColor": (255, 0, 0),
                      "hiddenColor": (0, 0, 255),
                      "showHidden": True})

exporters.export(result,
                 "/home/jwright/Downloads/out2.svg")

out.svg:
Screenshot from 2021-01-21 17-50-55

out2.svg:
Screenshot from 2021-01-21 17-49-31

@jmwright jmwright changed the title [WIP] Expose SVG Export Options and Add Some Useful Extras Expose SVG Export Options and Add Some Useful Extras Jan 22, 2021
jmwright and others added 2 commits January 22, 2021 15:55
Co-authored-by: Marcus Boyd <50230945+marcus7070@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Jan 23, 2021

Codecov Report

Merging #596 (c8ca3a2) into master (53a76d4) will decrease coverage by 0.01%.
The diff coverage is 95.65%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #596      +/-   ##
==========================================
- Coverage   94.26%   94.25%   -0.02%     
==========================================
  Files          29       29              
  Lines        6348     6366      +18     
  Branches      675      679       +4     
==========================================
+ Hits         5984     6000      +16     
- Misses        226      227       +1     
- Partials      138      139       +1     
Impacted Files Coverage Δ
cadquery/occ_impl/exporters/svg.py 96.55% <94.73%> (+1.35%) ⬆️
cadquery/occ_impl/exporters/__init__.py 93.06% <100.00%> (ø)
tests/test_exporters.py 100.00% <100.00%> (ø)
cadquery/cq.py 89.28% <0.00%> (-0.38%) ⬇️
tests/test_assembly.py 100.00% <0.00%> (ø)
cadquery/occ_impl/shapes.py 91.92% <0.00%> (ø)
tests/test_cadquery.py 99.04% <0.00%> (+<0.01%) ⬆️
cadquery/assembly.py 90.54% <0.00%> (+0.09%) ⬆️

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 53a76d4...c8ca3a2. Read the comment docs.

@jmwright
Copy link
Member Author

@adam-urbanczyk codecov refuses to see that line 260 is covered, even though I can test it manually and it is hit. Please review at your convenience. If you don't find anything else, this should be ready to merge unless you want to wait until after the 2.1 release.

@jmwright
Copy link
Member Author

I really wasn't paying enough attention. I copied the STL test and added the SVG options to it. I've fixed that now and hopefully everything will go green, including the lint error for line 260.

@adam-urbanczyk
Copy link
Member

Great, let's merge this before the final 2.1 . Just give me some time for the review.

@adam-urbanczyk
Copy link
Member

Looks good @jmwright , thanks! Shall I merge it?

@jmwright
Copy link
Member Author

Looks good @jmwright , thanks! Shall I merge it?

Yes, please do.

@adam-urbanczyk adam-urbanczyk merged commit 36d4178 into CadQuery:master Jan 28, 2021
@jmwright
Copy link
Member Author

jmwright commented Feb 5, 2021

I'm adding this here for future reference, and to let the community know about it.

The Paramak team is already making good use of these new SVG export options here.

They have created an animation script that looks really useful here as well.

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

Successfully merging this pull request may close these issues.

4 participants