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

More exports in rendering code or better save functions #35

Open
zostay opened this issue Jul 11, 2021 · 7 comments
Open

More exports in rendering code or better save functions #35

zostay opened this issue Jul 11, 2021 · 7 comments

Comments

@zostay
Copy link
Contributor

zostay commented Jul 11, 2021

I want to use this library in a situation where it would be better to save the output directly to an io.Writer than to be forced to save directly to disk. However, there's no RenderXXX function that takes an io.Writer as an argument. I was going to see if I could create my own writer using one of the sampling methods directly, but the sampling methods are not exported. So, basically, I'd have to do something convoluted and slow (write to disk, close, read from disk to write somewhere else) or copy and paste the sampling code to create my own render package.

I'd like to propose that the RenderXXX functions really ought to be handled in two phases:

  1. Render to raw object (either triangles or points depending on the output)
  2. Serialize raw object and write to io.Writer

The existing functions that combine these two functions should remain, but exist as a convenience.

I'd also like to eliminate the duplication between the two different file writing paths eliminated. The differences between SaveXXX and WriteXXX can be combined without sacrificing the performance gained by the async writing performed by WriteXXX. Just define render.XXXWriter that provides a WriteTriangle3() method and then provide a constructor that returns that when given an io.Writer.

Finally, it might be nice to have more access to the rendered triangles directly. You never know when someone might want to do their own post-processing on the triangle mesh to perform some specialized work. Splitting the steps up makes that possible.

There's no reason the existing render API needs to change. It could all be implemented in terms of this new API. Anyway, without filling in the functions, the new code for STL could look something like this:

package render

// Triangle3Writer allows someone to receive the triangles as a writer 
// and write their own thing and provide the general interface to be 
// implemented by STLWriter.
type Triangle3Writer interface {
    WriteTriangle3(t *Triangle3) (int, error)
    WriteTriangle3s(m *Triangle3) (int, error)
}

type STLWriter struct {
    io.Writer
    // plus whatever we need to track state, efficiently buffer, etc.
}

func (w *STLWriter) WriteTriangle3(t *Triangle3) (int, error) {
    // triangle writing stuff gets moved into here
}

func (w *STLWriter) WriteTriangle3s(m []*Triangle3) (int, error) {
    // write lots of triangles
}

func NewSTLWriter(w io.Writer) *STLWriter {
    // do other STLHeader setup and what-not....
    return &STLWriter{w}
}

func SaveSTL(path string, mesh []*Triangle3) error {
    w, err := os.Create(path)
    if err != nil {
        return err
    }
    defer w.Close()

    sw := NewSTLWriter(w)
    for _, t := range mesh {
        _, err := sw.WriteTriangle(t)
        if err != nil {
            return err
        }
    }

    return nil
}

func WriteSTL(wg *sync.WaitGroup, path string) (chan<- *Triangle3, error) {
    w, err := os.Create(path)
    if err != nil {
        return err
    }

    sw := NewSTLWriter(w)

    c := make(chan *Triangle3)

    wg.Add(1)
    go func() {
        wg.Done()
        defer w.Close()

        for _, t := range c {
            sw.WriteTriangle(t)
        }
    }()
}

func RenderTriangle3s(
    s sdf.SDF3,
    meshCells int,
    w Triangle3Writer,
) error { 
    // Implement RenderSTL's rendering stuff into the Triangl3Writer
}

func RenderTriangle3sSlow(
    s sd.SDF3,
    meshCells int,
    w Triangle3Writer,
) error {
    // Implement RenderSTLSlow rendering stuff into the Triangle3Writer
}

func RenderSTL(
	s sdf.SDF3, //sdf3 to render
	meshCells int, //number of cells on the longest axis. e.g 200
	path string, //path to filename
) {
    // Open a file, wrap it in STLWriter, call RenderSTLTriangle3s
}

func RenderSTLSlow(
	s sdf.SDF3, //sdf3 to render
	meshCells int, //number of cells on the longest axis. e.g 200
	path string, //path to filename
) {
    // Open a file, wrap it in STLWriter, call RenderSTLSlowTriangle3s
}

I'm going to see about working on a PR to flesh that out, if you're interested. Otherwise, I'm going to fork the render package because I don't want the overhead of writing to disk and then reading it again for what I'm trying to do.

@zostay
Copy link
Contributor Author

zostay commented Jul 11, 2021

After writing that up, I took a deeper dive and realized I'd made a couple of mistakes. One was that I'm talking about an encoder pattern, not a writer pattern, but that's mostly just nomenclature. They are both very similar.

The other mistake is that I made some assumptions before realizing that part of the WriteSTL function is that there's a seek performed while writing the file to fix the triangle count in the header, which isn't known when the process starts.

My proposal as written does not work very well. I'm going to have a think on it and see if I can come up with a fix.

@deadsy
Copy link
Owner

deadsy commented Jul 11, 2021

You are not really proposing an io.Writer, but rather something that is io.Writer-like. It's like an io.Writer that deals with triangle slices rather than byte slices. You've also identified the post write header fixup problem which further complicates the stateless "crank out the bytes" model. It's not clear to me why you have two interface methods for 1 triangle vs a slice of triangles. The slice method can do all the work.

Counter proposal:

The marching cubes functions conceptually take an sdf object and a few parameters and generate a stream of triangles on a channel. One of them (the octree renderer) actually does that. The slow renderer could (and probably should) do that also (thereby getting rid of SaveSTL vs WriteSTL). Doesn't that achieve your goal? ie- plug the triangle stream into anything you like? We'd have to expose those MarchingX functions, but that's not a problem.

@zostay
Copy link
Contributor Author

zostay commented Jul 13, 2021

Yes, that would solve my problem. Some of the urgency of the solution, though, goes away because of the nature of the STL format. That solution doesn't provide a straightforward win by just trying to generate an STL file I can stream out. I'm still going to need to write an STL file. My purpose was wanting to quickly stream the STL from a web server backend to a web frontend and I still need the STL. If I'm creative, though, I suppose I could still do that if I streamed the triangles to the frontend without the header and then send the header as a footer and let the front-end rewrite it or even just infer it by dividing the bytes received by 50 and rewrite it itself.

Though, it would also open up the ability to produce OBJ or 3MF files as alternate outputs, which could be an advantage.

@soypat
Copy link
Contributor

soypat commented Apr 25, 2022

I have rewritten sdfx to solve this issue among others. https://github.com/soypat/sdf

@deadsy
Copy link
Owner

deadsy commented Apr 25, 2022

Well, there are a few things to process ...

I would liken meshCells to an implementation detail of the renderer used. This can be passed as an argument when instantiating the renderer used.

Agreed!

An error on a function like Cylinder3D can only be handled one way really: correcting the argument to it in the source code as one generates the shape! This is even implied with the implementation of the ErrMsg function: it includes the line number of the function that yielded the error. panic already does that and saves us having to formally handle the error message.

Not agreed. Yes - it get's tedious. It was done this way because:

a) I want functions to either do what you tell them, or tell you they can't do it. No changing parameters under the covers.
b) I wanted to support the programmatic generation of shapes (E.g. evolutionary algorithms) and that implied bad parameter values needed a non-panic response.

@soypat
Copy link
Contributor

soypat commented Apr 25, 2022

I wanted to support the programmatic generation of shapes

Agreed on your second point- pretty ambitious goal but awesome to say the least.

As for the other changes it'd be great for you to take a look at the render package. The Renderer type I am proposing is inspired by Go's standard library io.Reader. Pretty simple to use and easy to multithread it- split up the triangle buffer into amount of cores and have goroutines working on each segment, bam, easy multi-core processing. It also solves this issue #35.

One thing that may be a harsh change is the replacement of the sdf.V3 type, but truth be told the use of methods makes for hard reading. I settled on gonum's r3 package for several reasons, main one being the high quality of their API. The lead maintainer is an active member and readily listens to any changes that are worth making and keeps a cool head about not adding extraneous API footprint. It is also a well maintained library which has quite some research put in behind API design.

There are some other changes I think are well deserved in the library:

  • I feel like sdf package has become a dumpster for any odd-ball idea for a function. Some changes I propose are:

    • Moving all non-SDF functions out of sdf package and into a better named directory like shape or form, in particular the shape generation functions such as Cylinder3D, Box3D. Core SDF functions like Union and Difference should be left inside sdf package.
    • While I was rewriting the library I found a huge amount of one-liner methods on the V3 type were used once or twice- which I feel hardly justifies a new method on a type. Same goes for many functions in the sdf package.
  • Replacing sdf.V2 in a similar fashion as sdf.V3

  • Move all mathy types (M44, M33, others?) into subpackage since these are imported from everywhere and do not really fit into the sdf namespace (bring about import cycle problems when using with gonum's r3 type).

@deadsy
Copy link
Owner

deadsy commented Aug 11, 2023

@zostay

I think recent changes have adressed your concern.

type Render3 interface {

type Render2 interface {

type Line2Writer interface {

type Triangle3Writer interface {

Conceptually the rendering code writes to an interface implementation that can do whatever it likes with the triangles/lines.

At the moment the only implementation of this interface is a buffering object that subsequently writes on a channel that gets read by a file writing routine but of course, it could be anything.

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

3 participants