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

API refactoring or expansion to support assemblies, drawings, importers, external renderers, and CAM #22

Open
stevegt opened this issue Nov 18, 2020 · 6 comments

Comments

@stevegt
Copy link
Contributor

stevegt commented Nov 18, 2020

Jason, how stable were you planning to keep the sdfx public API at this stage? Are you considering uppercased things such as WriteDFX to be frozen, for instance? How simple do you want sdfx to remain -- do you want pull requests of higher-level features that would make sdfx more complex, or would you prefer to keep it as a simple low-level library that others call?

tl;dr I'm veering towards option (3) -- skip to the Proposal section below. This means keep it simple and avoid breaking changes to the public API by exporting some things that aren't yet exported. This would enable sdfx to function as a low-level library for other higher-level Go-based libraries and tools. This looks like a better way of addressing feature requests like #16 and #21, and would enable external implementations of alternate renderers as in #6, as well as third-party viewers, CAM, and physics engines.

Details

While struggling to implement #21, it finally dawned on me that labels, dimensions, and even features such as a title block or drawing frame are similar to subcomponents in assemblies. Similar to what #16 mentions, I'm finding that adding these features using the existing rendering pipeline means making some pretty drastic changes to the APIs, both internal and external.

Looking at #16, I see that @mrsimicsak, in trying to create assemblies, ran into some of the same issues I've been hitting in #21 the last few days -- specifically, a need to access the internals of SDF structs while encapsulating them such that they can be associated with non-SDF structs such as assembly connectors or text. Those encapsulated subcomponents then need to be able to be transformed in sync with each other and rendered into some external file format, emitting both SDF and non-SDF types into the rendered stream, using the right primitives for the target format, while preserving spatial relationships between components. Implementing this in the existing rendering code paths would cause a lot of refactoring of existing code.

For example, if the target format is DXF and we're assembling a 2D drawing, we would want to be able to emit Line{} as LINE and Text{} as TEXT, rather than converting Text{} to Line{} segments as in TextSDF2(). Text{} or an encapsulating struct would need to include X and Y fields, and if the Text{} is a label for an SDF object, then something like the connectors in #16 would then be used to associate the Text{} with the SDF object. Those X and Y fields in Text{} would then be altered in sync with the SDF coordinates during any transform. Later, when the WriteDXF goroutine is running, it would need to be able to handle an interface encapsulating both Text{} and Line{} types on its input chan, with either accessor methods or a type switch for dispatching to the right Line() and Text() calls in the yofu/dxf library. And so on.

This all feels like the wrong direction. Here are the available options that I can see:

Option 1

Option (1) would be that we implement the above as breaking changes to the public API. That alteration to the chan type, for instance, alters the WriteDXF signature. Internally, the chan type change also propagates all the way down from RenderDXF through marchingSquaresQuadtree() to processSquare(), since they all share that chan. Alternatively, RenderDXF becomes more of a chan router and type converter, getting Line{} types from marchingSquaresQuadTree() and converting them to the new encapsulating type that WriteDXF would need.

I don't know what other things would be impacted, but so far it looks like it would be a significant refactoring as opposed to a simple pull request.

This feels wrong to me.

Option 2

Option (2) avoids breaking changes by creating a whole new set of encapsulating and rendering structs and methods to implement the above. Much of this code would be redundant with what now exists in e.g. sdf*.go, render.go, dxf.go, and march*.go. For example, #16 led to an experimental and partially redundant connectors.go, and for #21 I'm finding myself writing a whole parallel universe in a new sdf/drawing.go file, with its own slightly altered versions of RenderDXF, WriteDXF, etc.

This is leading to a lot of copying and pasting of code, and just feels wrong.

Option 3

Option (3) avoids the breaking changes of (1) and code duplication of (2) by turning sdfx into more of a library for higher-level tools. We do this by uppercasing and exporting most of the existing SDF struct fields and more of the existing functions and methods, expanding the public API. (I acknowledge the paradox here -- proposing export of more things so we don't have to make breaking changes to what's already exported.)

Proposal

I think I prefer option (3), because it opens up the opportunity for an entire ecosystem around sdfx. This enables higher-level layers in other Go libraries, using sdfx as a backend for more complex assemblies, renderers, viewers, CAM pipelines, etc. I would expect the existing renderers in sdfx to remain as reference code, and new renderers to be implemented as separate libraries to be maintained by whoever needs them.

Uppercasing everything in sight would still be a nontrivial pull request as well as a policy decision for @deadsy to make, but at least would be straightforward to do and and easy to test.

Jason, what do you think? Should I go ahead and try (3) in my own fork and see how it goes?

Use case examples

Aside from the use cases mentioned in #16 and #21, I'm also thinking of other CAM pipelines as well as simulation code -- finite mesh and physics engines, for example, could hook into the more complete public API that option (3) would provide.

With option (3), it should be possible for a calling library to generate CNC milling G-code from SDF objects. The caller might be able to use Evaluate() and an exported marching cubes function, for example, to compare the SDF object being milled with another SDF object that represents the tool. (Efficient open-source milling tool path generation is something I've been after for more than a decade myself -- this could finally make it possible.)

Option (3) would enable slicers to be built on top of sdfx. For example, here's a description of direct slicer rendering from SDF objects to printer g-code, skipping the STL stage entirely: https://on-demand.gputechconf.com/gtc/2017/presentation/s7131-storti-modern-cad-cam-workflow.pdf.

@deadsy
Copy link
Owner

deadsy commented Nov 19, 2020

I'm not commited to a stable public API at this point- I do want everything in the examples directory to build, mostly because that's the stuff I mess about with :-)

Here's my (until now) mental TODO list:

  1. Split sdf into multiple packages:
    core: SDF primitives (ie- things with real eval functions)
    shapes: useful higher-level shapes made from primitives
    render: functions that render SDFs to verious output formats

  2. Cleanup the the public API to get rid of any things that aren't needed externally. Since I've started this project I've learnt more about good go package design so prior sins should be corrected. The core package should be clean with a stable API.

  3. Get rid of the panics. They should really be error returns from the functions.

For example, if the target format is DXF and we're assembling a 2D drawing, we would want to be able to emit Line{} as LINE >and Text{} as TEXT, rather than converting Text{} to Line{} segments as in TextSDF2().

This code is about signed distance fields. It's not a line, it's not text, it's just a signed distance field. The code lets you compose these in various ways and at the end you can plug the composed field into a renderer.

You can look at the #16 connectors stuff as being an oriented union of SDFs- so I think that belongs in core.

Putting a standard frame on DXF output #21 sounds like a rendering output decision. ie - pass a parameter set that controls the frame output of the RenderDXF function, or nil if you don't care. Yes - you could build the whole frame out of SDF objects but that seems a bit wasteful and then leads you down a dark path - ie- how do I convert an SDF back into a minimal representation in the DXF file? At that point you've overlapped rendering and SDF creation with a consequent loss of modularity.

Uppercasing everything in sight

Not a recipe for controlling complexity in my experience :-)

@stevegt
Copy link
Contributor Author

stevegt commented Nov 19, 2020

Okay, this is good then. That TODO list is the sort of roadmap that gives me the guidance I was looking for. If you have no objections I'll paste it into a ROADMAP.md and send you a pull request so other people can pitch in more constructively.

I'm not commited to a stable public API at this point. I do want everything in the examples directory to build, mostly because that's the stuff I mess about with :-)

I kinda figured that. ;-) Most of those examples are going to need rework as the API cleans up, and I wasn't sure how invasive you wanted fixes to be, so this flexibility helps a lot.

Here's my (until now) mental TODO list:

  1. Split sdf into multiple packages:
    core: SDF primitives (ie- things with real eval functions)
    shapes: useful higher-level shapes made from primitives
    render: functions that render SDFs to verious output formats

That's the way I would stack it. Putting each of those layers in separate package means that each of them publishes a public API for the higher layers to interact with, and opens up that ability for folks to create alternate renderers, third-party shape libraries, etc.

  1. Cleanup the the public API to get rid of any things that aren't needed externally. Since I've started this project I've learnt more about good go package design so prior sins should be corrected. The core package should be clean with a stable API.

Coming from Python, I've committed those same oversharing sins in Go plenty of times myself. ;-)

  1. Get rid of the panics. They should really be error returns from the functions.

There are also a few places where I noticed some errors being printed with neither a panic nor an err return, probably somewhere in the render pipeline; I'll start fixing those as I encounter them. Not freezing the API now helps.

For example, if the target format is DXF and we're assembling a 2D drawing, we would want to be able to emit Line{} as LINE >and Text{} as TEXT, rather than converting Text{} to Line{} segments as in TextSDF2().

This code is about signed distance fields. It's not a line, it's not text, it's just a signed distance field. The code lets you compose these in various ways and at the end you can plug the composed field into a renderer.

I don't know if I'm expressing myself well, but I'm pretty sure it'll all sort out. What I'm finding is in the case of some renderers, the renderer wants the upstream source rather than the SDF object that was created from it. I think the package stack you're describing above might allow for that though.

You can look at the #16 connectors stuff as being an oriented union of SDFs- so I think that belongs in core.

I could be wrong, but I suspect it may turn out that unions and transforms want to be in a separate package so there's well-defined interfaces between them and SDF objects. It sounds like you're interested in sdfx remaining focused on SDF without adding the complexity of other object types. If that's true, then you'd want to avoid having to deal with raw text, fluids, application-specific constraints and connector types, animation and simulation, or object attributes like material, finish, or color. I'm looking for a data path that would allow those to exist, and for spatial relationships to be maintained, without having to pass through sdfx and making it more complicated than it needs to be. If we can pull this off, then that also will help keep crazy people like me from wandering in here asking for yet another tangential feature. ;-)

Another way of looking at it is that I'm hoping we can create a situation where sdfx is to frontends as CGAL is to OpenSCAD, but with more functionality than OpenSCAD will ever have. Having the modelling language be the same as the library language means that sdfx, with a clean API, will allow for per-model extensibility that OpenSCAD can't do.

Putting a standard frame on DXF output #21 sounds like a rendering output decision. ie - pass a parameter set that controls the frame output of the RenderDXF function, or nil if you don't care. Yes - you could build the whole frame out of SDF objects but that seems a bit wasteful and then leads you down a dark path - ie- how do I convert an SDF back into a minimal representation in the DXF file? At that point you've overlapped rendering and SDF creation with a consequent loss of modularity.

I think we're saying roughly the same thing; I may be saying it badly. In my case, I'm in a situation where I need to be able to programmatically generate hundreds of thousands of solid models, and 2D reference drawings of those solid models, on an ongoing basis. Have needed to for years, but that just wasn't going to happen with OpenSCAD or FreeCAD. I also tend to work with labs, aerospace, etc. and often need to do some of those other physics and simulation bits as well.

You've created a pretty powerful tool here. Part of what I'm looking for in these conversations is just how far you want to take it, what you want your workload to be, how much you'd prefer to calve off to others like me, how much should be in other repos so you don't have to maintain it, that sort of thing. I have some obvious reasons for running with this, but it could be a wild ride. ;-)

Uppercasing everything in sight

Not a recipe for controlling complexity in my experience :-)

ExPoRt All the ThiNGS. ;-)

stevegt added a commit to stevegt/sdfx that referenced this issue Nov 20, 2020
@stevegt stevegt mentioned this issue Nov 20, 2020
@mrsimicsak
Copy link

I'm glad to see more people taking in interest in expanding and improving the library!

  1. Get rid of the panics. They should really be error returns from the functions.

There are also a few places where I noticed some errors being printed with neither a panic nor an err return, probably somewhere in the render pipeline; I'll start fixing those as I encounter them. Not freezing the API now helps.

I think handling errors via panic or a similar immediate stop with a message is the appropriate way to handle errors for this library, even if it is not idiomatic go. Returning an error is great when you need the program to keep running and you have a meaningful way to continue past the error. I don't believe either of those is true for sdfx. I think most of the time the programs built using sdfx will be run once and any errors are going to require the user to change the program. In cases where the program is being run more than once the programs are likely to be models with user input parameters and the errors are likely to be caused by illogical input parameters or a set of parameters the programmer did not anticipate. In my opinion illogical input parameters are better handled via input sanitization and unanticipated inputs are likely to require changes to the program.

You can look at the #16 connectors stuff as being an oriented union of SDFs- so I think that belongs in core.

... I'm looking for a data path that would allow those to exist, and for spatial relationships to be maintained, without having to pass through sdfx and making it more complicated than it needs to be. ...

After deadsy's last response to #16 I spent a lot of time thinking about what the minimum functionality connectors requires of the underling SDF. I came to the conclusion that at minimum required was a way to map coordinates and rotations between frames of reference. This in turns requires a way to refer to frames of reference. Everything else was storing data, calculating transforms, and applying them.

A way to map coordinates and rotations between frames of reference, and to refer to frames of reference would be useful for more than just connectors. I also think it dovetails neatly into what you are talking about.

Another way of looking at it is that I'm hoping we can create a situation where sdfx is to frontends as CGAL is to OpenSCAD, but with more functionality than OpenSCAD will ever have. Having the modelling language be the same as the library language means that sdfx, with a clean API, will allow for per-model extensibility that OpenSCAD can't do.

I dream of a hybrid graphical/code environment where I can rough out designs graphically and then fine tune in code...

Uppercasing everything in sight

Not a recipe for controlling complexity in my experience :-)

ExPoRt All the ThiNGS. ;-)

If we are not committed to a stable public API, I'll submit a pull request to standardize some of the capitalization inconsistencies i noticed.

@stevegt
Copy link
Contributor Author

stevegt commented Nov 20, 2020

I'm glad to see more people taking in interest in expanding and improving the library!

I'm bringing in a couple more -- we'll start working on the panics and error returns over the next few days.

I think handling errors via panic or a similar immediate stop with a message is the appropriate way to handle errors for this library, even if it is not idiomatic go. Returning an error is great when you need the program to keep running and you have a meaningful way to continue past the error.

Abending makes sense for a standalone batch program, but lemme inspire everyone's imagination a bit:

  1. If sdfx core is embedded in a larger interactive third-party program, then you'd want the author of that program to be able to escalate errors to the user. In the case of an internal error, they'd want to provide debug info to the user rather than crashing without save. In the render pipeline, there will be errors such as low disk space during WriteSTL() that they'd want to catch and escalate to the user so the user can correct and retry the save.
  2. If sdfx core is embedded in a daemon process that accepts input parameters over the network and sends g-code to a 3D printer, then the author of that daemon would want to be able to safe the printer and provide an error response back to the client rather than crash leaving the heaters on.
  3. For core to be used as part of the fitness function for a genetic algorithm -- evolving millions of objects with random crossover and mutation of a DNA-like parameter set -- we'd need to be able to gracefully discard non-viable individuals that return errors, setting their fitness to zero before the cull step during each generation.

I could mention a few more, but I think you get the idea. ;-)

@stevegt
Copy link
Contributor Author

stevegt commented Nov 20, 2020

After deadsy's last response to #16 I spent a lot of time thinking about what the minimum functionality connectors requires of the underling SDF. I came to the conclusion that at minimum required was a way to map coordinates and rotations between frames of reference. This in turns requires a way to refer to frames of reference. Everything else was storing data, calculating transforms, and applying them.

I'm glad you've been working on this -- I'm still trying to get my head wrapped around it. I'm still trying to figure out if the union, transform, connector, and constraint stuff can be in core or if it needs to be in one or more separate packages. I'm afraid that if it's in core, then we'll accidentally limit what it can do while at the same time not provide a clean API for third parties to replace or extend it. I'll probably understand better after I've played with your connector.go code.

This was referenced Nov 21, 2020
stevegt added a commit to stevegt/sdfx that referenced this issue Nov 24, 2020
…anics_02

- resolves merge conflict in deadsy#27 between panic cleanup (deadsy#24) and API refactor (deadsy#22)
@soypat
Copy link
Contributor

soypat commented May 4, 2022

I'm way out of my element, but reading this:

if the target format is DXF and we're assembling a 2D drawing, we would want to be able to emit Line{} as LINE and Text{} as TEXT, rather than converting Text{} to Line{} segments as in TextSDF2().

Sure sounds like reflect package could be of use. It should be easy enough to add a unexported method to all SDFs that returns a slice of underlying concrete SDFs and thus traverse the SDF stack like so

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

No branches or pull requests

4 participants