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

fullcontrol: visualize: add TubeMesh #18

Merged
merged 1 commit into from
Jun 8, 2023

Conversation

ES-Alexander
Copy link
Contributor

@ES-Alexander ES-Alexander commented May 27, 2023

Initially discussed in #15

Acts like a 3D line plot, but the line has consistent geometry relative to the plot data scale, rather than staying the same pixel width when zooming in and out. A line plot is better for seeing the entirety of the path, whereas a tubular mesh is better for getting a sense of the design (e.g. how the filament is distributed in space, and where there are likely to be intersections / overlaps between filament paths).

Changing (back) to a line plot instead is possible by setting PlotControls(use_tubes=False).

Tube geometry can be configured via the following plot controls:

  • tube_sides determines the tube outline resolution
    • the default is dynamic, and should generally provide a decent tradeoff between appearance and performance
  • extrusion_width controls the transverse diameter ('width') of the tube
  • layer_height controls the vertical diameter ('height') of the tube
    • if unspecified, falls back to a 'circular' tube with its height specified by extrusion_width

Some of these results look great, and the data-based extrusion widths are really helpful for visualising printed lines at scale:
Screenshot 2023-05-28 at 3 18 03 am

Screenshot 2023-05-28 at 8 07 00 pm Screenshot 2023-05-28 at 3 19 05 am

NOTE: When there are lots of path points (>50k) the generated mesh uses simplified corners (a direct TubeMesh instead of the nicer looking ChamferedTubeMesh), which looks worse at sharp corners and 90 degree turns, but uses half the triangles so is less resource intensive to load and view.

We can change this behaviour or make the threshold configurable if that's relevant.

@ES-Alexander ES-Alexander force-pushed the tube-mesh branch 4 times, most recently from 41447f3 to 61888ee Compare May 28, 2023 08:48
@fullcontrol-xyz
Copy link
Contributor

This is awesome. I'm going to test it for a few of my own designs. I think it's definitely best if this is not the default option initially. Until its been tested for a fair variety of paths. It makes sense, eventually, to have width and height information come directly from the FC design (based on ExtrusionGeometry). But I'll need to tweak the plot data generation stuff to do that (that data is currently only used for GCode generation). I also need to think about numpy being a requirement of the main FC package, and check that doesn't introduce any problems

@ES-Alexander
Copy link
Contributor Author

ES-Alexander commented May 28, 2023

This is awesome.

I think so too! It's been fun to get working :-)

I'm going to test it for a few of my own designs. I think it's definitely best if this is not the default option initially. Until its been tested for a fair variety of paths.

I'm pretty comfortable with the known "failure" modes at the moment (especially now that it's using ChamferedTubeMesh most of the time - I think you might have been looking just before I finished the description update), and I think having the physical view is likely more intuitive and valuable than the line view as a starting point.

Would it be acceptable to use as the default if we add a try-except block that falls back to a 3D line plot if an exception is raised, or are you more concerned with potential silent failures that could run without an error but produce an unexpected visual result?

It makes sense, eventually, to have width and height information come directly from the FC design (based on ExtrusionGeometry). But I'll need to tweak the plot data generation stuff to do that (that data is currently only used for GCode generation).

Agreed. I think the existing manual controls are a good starting point, and feeding that in automatically can come later.

I also need to think about numpy being a requirement of the main FC package, and check that doesn't introduce any problems

Fair enough - I thought I had seen it used elsewhere in the project, but I may be misremembering. It's a staple of numerical computing with Python, so it should be supported on practically any platform you can think of.

If numpy does cause problems then likely the best approach for this kind of plot will be offloading as much as possible to the javascript side - I suspect pure Python would have performance issues with the calculations involved, especially at higher path point counts.

@fullcontrol-xyz
Copy link
Contributor

I'm just about to test it. Quite excited. Haven't got long now, so I'll continue testing in the next couple of days

I'm going to test it for a few of my own designs. I think it's definitely best if this is not the default option initially. Until its been tested for a fair variety of paths.

I'm pretty comfortable with the known "failure" modes at the moment (especially now that it's using ChamferedTubeMesh most of the time - I think you might have been looking just before I finished the description update), and I think having the physical view is likely more intuitive and valuable than the line view as a starting point.

Would it be acceptable to use as the default if we add a try-except block that falls back to a 3D line plot if an exception is raised, or are you more concerned with potential silent failures that could run without an error but produce an unexpected visual result?

It's mostly about people doing paths that we might not expect and getting a poor visual preview. Or if plots take much longer to generate (checking shortly). I don't expect it to be long until this could be default, but because visualization is a critical step in almost all designs, I don't want to put something as default that might not work for some users. It could be things like numerical issues caused by someone plotting lines in directly opposite directions, or if two points are almost coincident. Although these could be caught with a try-except, for sure. I don't expect there to be issues, but just want to do a bit of feeling it out. In the short term, after it becomes default, a 1-line printout saying the default has changed could help make sure any such problems are instantly fixable by designers.

I also need to think about numpy being a requirement of the main FC package, and check that doesn't introduce any problems

Fair enough - I thought I had seen it used elsewhere in the project, but I may be misremembering. It's a staple of numerical computing with Python, so it should be supported on practically any platform you can think of.

If numpy does cause problems then likely the best approach for this kind of plot will be offloading as much as possible to the javascript side - I suspect pure Python would have performance issues with the calculations involved, especially at higher path point counts.

I again think this will be fine, but just want to make sure there aren't any unanticipated issues

@fullcontrol-xyz
Copy link
Contributor

Played around a bit and it seems really good! So useful! I've begun a few notes about it. The itertools import (and maybe others) requires python 3.10+ I believe, so this will need to be reflected in the overall fullcontrol install procedures.

One thing I think would be good is to change the terminology slightly for the end user. So instead of 'use_tubes', the name might be something like be ‘3d-extrusions’ or ‘3d_lines’ or ‘volumetric’ or ‘volumetric_lines’ (numeric start-character may not allowed potentially) - this is based on the assumptions that very few designers, using FullControl, will be thinking about the practical implementation of the plot. Even so, 'tubes' is a reasonable term, but something that is self-explanitary in screaming out the extrudates are going to be plotted in 3D instead of lines would be ideal. It may be that 'tubes' is the best term, but we can keep thinking about it.

I'm also beginning the stuff go get extrusion geometry data coming from the FullControl 'design'. I think it would be really good if there is only one source of geometric data since this plot genuinely represents the actual designed part/geometry.

Interestingly, most of the time, I found 4 tube_segs to look better than 8. So that could be a better default. This is a shame cos I very much like your neat code to dynamically reduce segments with path length! Did you find any issues with 4 segments as opposed to 8? E.g. for twisting issues or anything?

Great job though thanks! I'm hopefully not annoying by not implementing immediately, but I want it to be perfect! 😁

@ES-Alexander
Copy link
Contributor Author

It's mostly about people doing paths that we might not expect and getting a poor visual preview. Or if plots take much longer to generate (checking shortly). I don't expect it to be long until this could be default, but because visualization is a critical step in almost all designs, I don't want to put something as default that might not work for some users. ... In the short term, after it becomes default, a 1-line printout saying the default has changed could help make sure any such problems are instantly fixable by designers.

Fair enough - let's start with off by default with a printout notifying that it exists, and then if+when we switch to on by default we can change the printout to mention how to turn it off.

The itertools import (and maybe others) requires python 3.10+ I believe, so this will need to be reflected in the overall fullcontrol install procedures.

Ahh yep, I'll add some notes as relevant in the next couple of days, then squash.

One thing I think would be good is to change the terminology slightly for the end user. So instead of 'use_tubes', the name might be something like be ‘3d-extrusions’ or ‘3d_lines’ or ‘volumetric’ or ‘volumetric_lines’ (numeric start-character may not allowed potentially) - this is based on the assumptions that very few designers, using FullControl, will be thinking about the practical implementation of the plot. Even so, 'tubes' is a reasonable term, but something that is self-explanitary in screaming out the extrudates are going to be plotted in 3D instead of lines would be ideal. It may be that 'tubes' is the best term, but we can keep thinking about it.

Hmm, agreed that use_tubes is not so intuitive from a design standpoint. I think I like "3D extrusions" the most, but unfortunately starting a variable name with a number is not allowed, and if we used a preceding underscore or something then that would likely be hard to remember. We could maybe do volumetric_extrusions, but that's long enough that it might be hard to remember... Will need to have a think.

I'm also beginning the stuff go get extrusion geometry data coming from the FullControl 'design'. I think it would be really good if there is only one source of geometric data since this plot genuinely represents the actual designed part/geometry.

Alright cool :-)
Fair on the 'one source of truth', although I don't think it's a problem to have the interface for that achieved through PlotControls, since in future if there end up being additional plotting options we'd want them to all derive from consistent controls where possible.

If you're happy with that idea then this is unrelated to when/whether this PR gets merged, but let me know if you'd prefer some other approach.

Interestingly, most of the time, I found 4 tube_segs to look better than 8. So that could be a better default. This is a shame cos I very much like your neat code to dynamically reduce segments with path length! Did you find any issues with 4 segments as opposed to 8? E.g. for twisting issues or anything?

I expect multiples of 6 will tend to look the best, because there's a flat surface on the top. I still think the dynamic reduction is useful to avoid excessive segments at high path point counts, so I'll try just wrapping the existing approach in max(<existing>, 6) for now, and then we can modify that if we decide we'd rather have a different falloff rate at the higher path point counts or something.

On the twisting front, twists are entirely determined by the relative directions of the normal vectors of successive points, which are consistently used as the basis for any number of sides. Accordingly, no number of sides is worse than any other there, although with some shapes at particular angles it might look a bit more noticeable because of shadow rendering and whatnot.

I'm hopefully not annoying by not implementing immediately, but I want it to be perfect! 😁

Not at all. Perfect is the enemy of good, but some degree of verification and iteration are practically always required to get from "it's ready to be looked at" to "it's good enough to merge". As part of that, getting feedback from someone who's not as in the weeds of the implementation tends to be helpful for finding and fixing oversights to the experience of actually using a feature :-)

I'll make some small fix-ups in the next couple of days, and (if nothing else comes up in your additional testing) we can merge once it's ready :-)

@ES-Alexander
Copy link
Contributor Author

ES-Alexander commented May 29, 2023

Some notes/todos:

  • Update pyproject.toml
    • Python >= 3.10
      • I believe this is only for itertools.pairwise at the moment, so if there's a strong preference to keep the minimal version as 3.9 then we could instead catch the ImportError and just implement pairwise manually - it's not currently being used for anything performance critical
        • do the aforementioned catch + manual implementation of pairwise, and confirm 3.9 is still sufficient
      • I've used union type expressions throughout tube_mesh.py and am not keen to change them all to Union[..., etc] groups (I'd forgotten they were also introduced in 3.10)
        • It's possible there are other 3.10 features - I haven't checked
    • numpy
      • This is currently only used for tube meshes, so if it's a problem to make it a project requirement then we could make it an "extra" dependency (installable via e.g. a [full] or [standard] target). That said, I expect this will come in handy for a variety of computation-heavy features later, so it might be nice to have available for those from the start.
  • Add numpy to requirements.txt
  • use_tubes boolean flag -> something like style="line"|"tube"
    • Could use StrEnum instead of arbitrary strings, but that requires Python 3.11+ if we don't want a manual workaround
    • Set default as None, and make it fall to "tube" if unspecified, with a printout on how to specify it if you want explicit control over the behaviour
  • Set lower default value on controls.tube_sides
    • possibly 4, but 6 is a bit more rounded (so more correct from more angles) and has a flat top (albeit at the expense of being slightly less correct from directly side-on)
  • Create FlowTubeMesh as a hybrid between the existing options, getting TubeMesh's flow behaviour for smooth rotations (+ve dot-product between successive path segment direction vectors?) and ChamferedTubeMesh's nice behaviour at sharper corners (dot-product <= 0?)
    • This scaliness is unpleasant to have as default behaviour, and should be pretty easily fixed if we're ok with having diameter and colour interpolation throughout segments
      Screenshot 2023-05-30 at 11 00 05 pm->Screenshot 2023-06-02 at 02 08 48 am
    • ChamferedTubeMesh -> CylindersMesh to better reflect its primary benefit/difference compared to FlowTubeMesh
  • Ensure all tube options work with just two path points
  • Rebase and confirm functionality with variable extrusion widths/heights
  • It may be worth @fullcontrol-xyz making a 0.0.1 tag/release before this PR gets merged (which "stable" users can be directed to for installation), and this can be an alpha/beta feature until it's evaluated some more, after which it can be turned on by default and released officially in a new version

@fullcontrol-xyz
Copy link
Contributor

This is all looking good 👍 only a quick reply now. But I like your suggestion about tubes/lines. A short name would be style="line"|"tube"

I also thought it could be good for the default value for 'style' (i.e. 'use_tubes') to be None. Then on initialisation of a PlotControls object, if style=None is detected, the value can set as "tubes" and the single-line printout about the new default style can be created (when it's implemented as default). This means 'designers' can get rid of the printout by explicitly choosing the value.

I think version 0.0.1 is logical.

If it's a quick modification to allow python 3.9, I reckon implement that. Just to be nice to people. But if its laborious, leave it.

@fullcontrol-xyz
Copy link
Contributor

Some notes/todos:

  • Update pyproject.toml

    • Python >= 3.10

      • I believe this is only for itertools.pairwise at the moment, so if there's a strong preference to keep the minimal version as 3.9 then we could instead catch the ImportError and just implement pairwise manually - it's not currently being used for anything performance critical
    • numpy

      • This is currently only used for tube meshes, so if it's a problem to make it a project requirement then we could make it an "extra" dependency (installable via e.g. a [full] or [standard] target). That said, I expect this will come in handy for a variety of computation-heavy features later, so it might be nice to have available for those from the start.
  • use_tubes boolean flag -> something like extrusion_display="line"|"tube"

    • Could use StrEnum instead of arbitrary strings, but that requires Python 3.11+ if we don't want a manual workaround 😂
    • Set default as "line" for now, and include a printout mentioning "tube" is available
  • Set lower default value on controls.tube_sides

    • possibly 4, but 6 is a bit more rounded (so more correct from more angles) and has a flat top (albeit at the expense of being slightly less correct from directly side-on)
  • It may be worth @fullcontrol-xyz making a 0.0.1 tag/release before this PR gets merged (which "stable" users can be directed to for installation), and this can be an alpha/beta feature until it's evaluated some more, after which it can be turned on by default and released officially in a new version

Can you edit this comment to include a point to add numpy to requirements.txt

@fullcontrol-xyz
Copy link
Contributor

I've just committed some minor changes to the visualize subpackage and combinations subpackage so that width/height data are generated directly from ExtrusionGeometry objects in the FullControl design and added to plot_data. It only uses 'width' and 'height' attributes at the moment but can easily be extended to include a 'circle' or 'manual' area_model for cross-sectional area in the future (rarely used). The changes implemented in this commit don't affect anything for current users.

To see the new data, run the plot_controls.ipynb tutorial notebook. You'll see the output data of the second-last code cell now shows width/height data for each 'path'.:

output:
second path (travel line of two points):
    xvals=[65.0, 50.0] yvals=[50.0, 50.0] zvals=[5.0, 0.0] colors=[[0.75, 0.5, 0.5], [0.75, 0.5, 0.5]] 
        extruder=Extruder(on=False) widths=[0.5, 0.5] heights=[0.2, 0.2]

The widths=[0.5, 0.5] heights=[0.2, 0.2] wasn't there previously.
Let me know if anything isn't clear. I've defaulted to 0.5 and 0.2 for width and height, respectively. That decision for default values can also change in the future easily - potentially with 'initialization_data', to make the structure of the visualize 'result' controls match those of the gcode 'result'.

FYI, future visualisation enhancements (which don't affect your tube model) are to allow the plots to be coloured according to model data for speed, volumetric flowrate, etc. And ideally include a legend. Plus show the print-bed as mentioned in an issue.

@fullcontrol-xyz
Copy link
Contributor

P.s. There is geometry data for every point, but it actually refers to the geometry of the entire tube before the point. So I think this would utilise the N-1 option in your tube-plotting code. The first point has no tube before it, so width/height should be ignored for that point. That applies the the first point of each new 'path'.

@fullcontrol-xyz
Copy link
Contributor

p.p.s. check out the CONVEX demo in the lab tutorial notebook for an example FullControl design with lots of different extrusion widths

@fullcontrol-xyz
Copy link
Contributor

I'm really loving it. I've testing it quite a bit - here are a few minor things before merge

For your comments:

This scaliness is unpleasant to have as default behaviour, and should be pretty easily fixed if we're ok with having diameter and colour interpolation throughout segments
ChamferedTubeMesh -> FixedSegmentCrossSectionTubeMesh (or something similar, like SegmentedTubeMesh) to better reflect its primary benefit/difference compared to FlowTubeMesh

for initial release, I think it's fine to choose the smooth/corner option. Messing with diameter and colour interpolation seems like it could introduce unforeseen issues. Thinking from a designer's perspective, how about the term 'smooth_tubes' = True/False[default]? Am I correct that it basically comes down to one being best for curved paths, and one for paths with corners? Another option I thought was 'prioritize_curves', but I don't think it's as obvious. Any other thoughts?

Can you read extrusion width and height data from plot_data by default rather that from line_width? For reference, when I merge your pr, I'll add 'initialization_data' to PlotControls to match GcodeControls and allow the user to define width/height of the printed lines. As with GcodeControls, the terms 'width' and 'height' will be used to set the initial values for the first point. I'll implement default values there and remove the ones currently hard coded in visualize.state.py. The initial values will remain for all subsequent points in the plot unless ExtrusionGeometry objects are included in the design. Therefore, please remove 'extrusion_width' and 'layer_height' from the PlotControls. Or if you think they're really useful there, rename them as 'tube_width_override' and 'tube_height_override' or 'custom_tube_width' and 'custom_tube_height', or feel free to make your own name suggestions. I think it's good to clearly imply that they cause the widths and heights in the 'design' to be ignored.

Note that immediately after merging the pr, I will change tutorials' model sizes so they look good with default width/height settings for tubes. And where necessary, add some PlotControls(smooth_tubes=True)

Depending on whether you agree with my suggested changes, how about the following output messages?

  • Extrusion display defaulting to "tube" - use PlotControls(style="line") if more interested in the path than the 3D volume, or specify PlotControls(style="tube") to disable this message
  • Plot style optimized for sharp corners - use PlotControls(smooth_tubes=True) for paths with smooth curves, or specify PlotControls(smooth_tubes=False) to disable this message

Thanks again for such a great addition

@ES-Alexander
Copy link
Contributor Author

The new FlowTubeMesh is smooth like TubeMesh but has corrected corners (like ChamferedTubeMesh) specifically when they’re sharp - I’ve just finished it and haven’t had time to test it much yet, but I believe it should be used basically all the time, unless there’s a specific application where constant diameters are more important than everything else (in which case ChamferedTubeMesh can be used). Feel free to try it out - hopefully there aren’t any major issues with it :-)

I haven’t got around to rebasing and using the new extrusion geometry stuff yet - that’s next on the checklist, and I’ll hopefully get to it on the weekend. I don’t expect it to be very complicated to get working.

Agreed that if geometry controls are available from the data then it’s not very important/valuable to also allow them to be overridden in the plot controls, so I’ll most likely remove those controls.

I’ll look at your printout suggestions on the weekend - for now I’m very much in need of some sleep 😂

@fullcontrol-xyz
Copy link
Contributor

😁 😁 😁
It's working so well now! The main issue I saw with the FlowTubeMesh option before was that when tubes twisted they became visibly narrower/wider (especially when tube_width <> tube_height). But now with FlowTubeMesh, they're looking perfect. All the designs are in the main overview tutorial notebook, and there's a pretty broad range of design types in there.

Since it seems like FlowTubeMesh achieves everything, the second warning message can potentially be removed now, along with the PlotControls(smooth_tubes) option (or equivalent name - currently tube_type). We can quickly activate that code/option if unknown limitations of FlowTubeMesh become apparent in the future. Once extrusion width is controlled by ExtrusionGeometry in the design, the CONVEX path will present a good challenge to see if the more accurate widths(?) of ChamferedTubeMesh are necessary. If FlowTubeMesh works for that, I'd suggest only enabling that option for users. Unless you can foresee issues.

I'll do the minor tweak for 'initialization_data' in PlotControls tomorrow, although I don't think that will affect you since you'll use the values generated in plot_data for tube-width/height and won't reference initialization_data

I'll have to make a YouTube video to tell people about this new plot style. Let me know your preference for when I talk about you... if you prefer me to just use your github id, or name, or anything else like affiliations

@ES-Alexander
Copy link
Contributor Author

Glad we seem similarly hyped about this plot style, and that FlowTubeMesh seems to be (at least so far) a good compromise between feature options :-)

Since it seems like FlowTubeMesh achieves everything, ... I'd suggest only enabling that option for users. Unless you can foresee issues.

As some context, FlowTubeMesh (like TubeMesh) controls sizing by the segment ends (e.g. creating cut cones), whereas ChamferedTubeMesh sets a consistent size for each full segment (e.g. using cylinders). If you've got short segments and/or don't mind smooth diameter changes across the length of a segment then FlowTubeMesh should be fine, but if you need each segment to have a consistent cross-section, with 'jump' transitions at the segment ends, then ChamferedTubeMesh could be required (I'm not really sure how likely it is people will need that, but I believe it's a bit more realistic to how gcode is parsed when doing filament extrusion).

I'll have to make a YouTube video to tell people about this new plot style. Let me know your preference for when I talk about you... if you prefer me to just use your github id, or name, or anything else like affiliations

My online activities are pretty consistently done under ES-Alexander (programming, reddit discussion, etc), so that's probably best :-)

@fullcontrol-xyz
Copy link
Contributor

fullcontrol-xyz commented Jun 2, 2023

I've added the initialization_data to plot controls now. I also added printer_name as a placeholder for the future when showing the print_bed will be added as an option. I've got not plans for any further changes before your addition - FYI, in case my commits cause you any headaches with conflicts (hopefully not).

What do you mean by:

controls sizing by the segment ends (e.g. creating cut cones)

I can't actually see any difference in the plots for chamfer and FlowTubeMesh (aside from the scale effects on smooth curves for chamfer).

Regardless, the CONVEX demo will reveal any issues since it will have lots of tube of continuously varying diameter that should always be touching edge-to-edge. Any gaps should be easy to spot. Don't worry too much if gaps do exist since that is a challenging geometry. If the chamfer options works well, that's great. But even if not, it's fine if the current tube plots are close to the true geometry but not yet fully accurate for validation purposes.

@ES-Alexander
Copy link
Contributor Author

ES-Alexander commented Jun 3, 2023

Ok, I've rebased over your update, fixed a couple of bugs, and made a few changes to the messages.

I can't actually see any difference in the plots for chamfer and FlowTubeMesh (aside from the scale effects on smooth curves for chamfer).

That was one of the bugs apparently - I'd messed up an offset so the segments were transitioning diameters instead of the joints, which made it more similar (visually) to FlowTubeMesh than it was supposed to be. This is the latest (fixed) behaviour, and is hopefully more clear as to the difference:
Screenshot 2023-06-03 at 11 13 59 pm

These are the outputs for the CONVEX demo (flow left, cylinders right), but it's not a great example for showing the diameter difference because there are no big diameter jumps:
flowtubecylinders


As something of a side note, I needed to add duplicate-successive point removal in order for the mesh to be able to generate for the CONVEX demo. If a user is pushing the limits of their machine and they know their path does not contain two of the same point in a row then it could be useful to have a control to turn off that processing, but it's a reasonably small fraction of the total so I've left that off for now to try to avoid having too many controls.

@ES-Alexander
Copy link
Contributor Author

ES-Alexander commented Jun 3, 2023

I'm now done everything I had written down on my notes checklist, so merging is now up to whenever you're ready to, i.e. once you're happy with the state of the visualisations and messages, and have decided whether you want to make a 0.0.1 release beforehand (to allow people to easily go/revert to a version from before the new code and visualisations were introduced).

I suppose it's probably a good idea to update the README and possibly add some examples to at least the plot_controls notebook. I can do that if you'd like, but likely won't have much capacity for this until at least a few days from now.

@ES-Alexander
Copy link
Contributor Author

In the process of trying to get to sleep, I’ve just realised that we might actually want to allow custom display widths for people using 2D plotters, since the most indicative part of width in their code will be the z height, and likely with some custom easing/modifier function applied to it since different heights may not linearly change the drawn width.

Anyway, that can be sorted out in a later PR (after some time to think it through more) - just noting it down here to not forget about the idea.

@fullcontrol-xyz
Copy link
Contributor

I've played with it a little and it's all seeming good. Amazing to see the CONVEX stuff previewed in this way! 😃

I noticed a minor bug. If you set tube_type="cylinders" while not specifying style="tube", the plot no longer defaults to tubes and plots lines. I can change:
if controls.style is None and controls.tube_type is None:
to
if controls.style is None:
when merging. But feel free to include that if you do any further revisions yourself.

I'll play around with getting all the sizes right in tutorials and will add some demos in the PlotControls tutorial.

Your comment about 2D plotters makes sense, but I think that might be best address along with any other minor changes to fullcontrol terminology to suit them. It might be the case that they define the relationship between width and Z (even if just a lookup table) but then never actually design z-height. That relationship could be implemented when fullcontrol processes their design to automatically calculate Z. But then we'd probably add a new object instead of ExtrusionGeometry called LineGeomtery or just LineWidth if there are no other properties of interest. Perhaps something to discuss after some plotting trials! There are lots of opportunities there when introducing capabilities for plotting with a few discrete colours/pens-sizes to get mixtures/shading/offset-color-lines, etc. But a bit of playing around with what feels nice when designing will be useful before releasing too much.

@ES-Alexander
Copy link
Contributor Author

I've played with it a little and it's all seeming good.

Great to hear :-)

Amazing to see the CONVEX stuff previewed in this way! 😃

Indeed - having a volumetric display is really nice for understanding how a design will look, and I'm happy I spent some time to make this a reality :-)

I noticed a minor bug. ...

Thanks - my bad, I've fixed that now. The idea was to avoid showing the message about not having specified a style if they've specified a type of tube, because intuitively it's pretty clear they wanted style='tube' at that point.


Fair points on the 2D plotter stuff. As mentioned I was just recording the thought - sorting out the specifics is likely better done in a later PR, where more relevant and focused testing can also take place :-)

@fullcontrol-xyz
Copy link
Contributor

Created the v0.0.1 release today and am a good way through tutorial updates.

What do you think about getting rid of the second warning message (it is covered in a new section in the tutorials)?
And shortening the first warning message to the following?
Plot shows printed line width - use fc.PlotControls(style="line") for a simple path, or fc.PlotControls(style="tube") to disable this message
Trying to find a balance between letting existing users know it's changed, whilst not filling up the screens of new users with information they don't need

@fullcontrol-xyz
Copy link
Contributor

p.s. I'm going to make it a v0.1.0 release since this is a pretty substantial upgrade

@ES-Alexander
Copy link
Contributor Author

Created the v0.0.1 release today and am a good way through tutorial updates.

Nice!

What do you think about getting rid of the second warning message (it is covered in a new section in the tutorials)? And shortening the first warning message to the following? Plot shows printed line width - use fc.PlotControls(style="line") for a simple path, or fc.PlotControls(style="tube") to disable this message

That seems reasonable to me - not much value in redundant info messages in general operation.
Done.

p.s. I'm going to make it a v0.1.0 release since this is a pretty substantial upgrade

Fair enough :-)

@fullcontrol-xyz
Copy link
Contributor

fullcontrol-xyz commented Jun 6, 2023

I noticed a minor bug. If you plot a single line (design = two points), the plot reverts to style='line', can you plot a tube for just one line?

This is also true if there are multiple 1-line paths (e.g. travel_to demo in design_tips.ipynb)

@ES-Alexander
Copy link
Contributor Author

I noticed a minor bug. If you plot a single line (design = two points), the plot reverts to style='line', can you plot a tube for just one line?

My bad - that was on my list of points, and I fixed it in tube_mesh.py, but seemingly forgot to remove the conditional guard for it in plotly.py.

Should be fixed now :-)

@AndyGlx AndyGlx merged commit dc4889c into FullControlXYZ:master Jun 8, 2023
@ES-Alexander ES-Alexander deleted the tube-mesh branch June 8, 2023 14:06
@fullcontrol-xyz
Copy link
Contributor

It's in FullControl!!! Created a new release too

Thanks so much for all your efforts! I already know several people who are excited to use the new visualization to preview extrusion widths in their designs. One of them was actually planning to use fusion 360 to generate a 3D model of their toolpath, and write scripts to do that. But they already have it here now!

I committed quite a lot of associated updates to overcome minor bugs and also take advantage of the new visuals (i.e. enhanced the convex function and added more demos). Let me know if you think anything could be more clear or done better.

How much effort would it be to add a PlotControl attribute for export_stl and then save the mesh as an stl file instead of feeding it to plotly? This would be really valuable!!!
If it would require some python packages that aren't really justified to be included as a general fullcontrol requirement, perhaps it would be possible to return the mesh data in a format that can easily be converted to stl by the designer after importing the stl package?

Thanks again!

@ES-Alexander
Copy link
Contributor Author

Thanks so much for all your efforts!

No worries - thanks for the encouragement and advice/suggestions/ideas throughout :-)

I already know several people who are excited to use the new visualization to preview extrusion widths in their designs. One of them was actually planning to use fusion 360 to generate a 3D model of their toolpath, and write scripts to do that. But they already have it here now!

Great to hear!

I committed quite a lot of associated updates to overcome minor bugs and also take advantage of the new visuals (i.e. enhanced the convex function and added more demos). Let me know if you think anything could be more clear or done better.

I'll try to take a look over the weekend :-)

How much effort would it be to add a PlotControl attribute for export_stl and then save the mesh as an stl file instead of feeding it to plotly? This would be really valuable!!!

Conceptually this should be pretty straightforward, especially since you mentioned it early enough that I've already implemented capped ends as a generation option. That said, I'm not sure of the use-case for the feature, so I've opened an issue (#20) where we can discuss it in a bit more depth - STL has some limitations, so if there's a more appropriate format we might want to focus more on that instead (although implementing several output options may be fine too).

@ES-Alexander ES-Alexander mentioned this pull request Jun 9, 2023
7 tasks
@fullcontrol-xyz
Copy link
Contributor

Thanks for raising that issue and the others. They all look super interesting and exciting! I have some ideas for first implementations of them and variations of them, but it might take me a couple of days to actually reply to them, depending on various things in non-FullControl-life. But in general, they all look like great things to introduce soon as!

And FYI, a big use-case for stl export is FEA simulation or fluid flow simulation. So there are very few requirements - potentially just as simple as having closed geometry. Even lots of separate closed cylinders should be fine since pymeshlab or FEA software can likely easily combine them with a CAD-like union function. But I definitely see the importance of assessing more complex use-cases.

@fullcontrol-xyz
Copy link
Contributor

Thanks to this new visualisation method, I've created the ASTM tensile test design that I have wanted to do for a while but didn't due to the challenge of new users effectively visualising the model. The preview and printed versions are shown below. It's so beautiful! This is how tensile testing specimens should be made! The design also includes an upright-printed option. I'm now planning to do a quick video to introduce the new visualisation method and use this as a professional example. FYI, the design is saved as a gist

Thanks again!

image

image

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