jscad / OpenJSCAD.org

JSCAD is an open source set of modular, browser and command line tools for creating parametric 2D and 3D designs with JavaScript code. It provides a quick, precise and reproducible method for generating 3D models, and is especially useful for 3D printing applications.
https://openjscad.xyz/
MIT License
2.61k stars 511 forks source link

adding joostn csg.js to openjscad - open questions #74

Closed bebbi closed 8 years ago

bebbi commented 9 years ago
  1. AMF support - says experimental/rudimentary in many places in code - what's the overall status on current support? How well are import, export supported?
  2. STL - there's an addition to toStlBinary func - can you give some context on what this is needed for? I don't fully grasp the comment there: as binary string blobbing gives bad blob in web, we need binary string for CLI perhaps there is a way to make it working (see openjscad for stlb)
Spiritdude commented 9 years ago

AMF support, it's a relative new format, and not much supported (adoption is slow), so there isn't a lot of testing done as only few programs actually support it yet. Export and Import work, but particularly import hasn't been testing with large AMF files, as parsing is done not optimal in that case. But this should not concern you, since the AMF export part in csg.js is rather minimal, essentially I need access to the polygons and their material/color information. It could easily be in the future a sort of general exporter() providing the needed info, and format exporter code reside outside of csg.js.

STL, yes, "as binary string blobbing gives bad blob in web, we need binary string for CLI perhaps there is a way to make it working (see openjscad for stlb)": web (browser)/CLI wasn't consistent when creating a blob, so, when I recall correct, the blob creation was done later than in the original code, I don't know the cause of the problem, the first result was bad blob, wrong data; and post-poning blobbing resolved it and gave consistent data.

bebbi commented 9 years ago

OK. Will check the AMF stuff. Agree on export in separate file. Can anyone else jump in with some research on the blob issue? (possibly anyone interested in issue #33?)

z3dev commented 9 years ago

I agree that the conversion routines should reside in another library, outside of CSG.js. How about naming the library after what it does? conversion.js, or even more specific stl.js

Issue #58 includes some suggestions on improving the STL (and other) format conversions. In general, the format conversions should return exactly what is requested, nothing more. For example, AMF is actually XML. JSCAD is actually JavaScript code which calls CSG routines.

The conversion routines should only return the "format" requested, nothing more. Calling routines may want to do further processing, conversions, etc.

function STL.toJSCAD( stl_string ) { ...; return JSCAD_code; }

Spiritdude commented 9 years ago

@bebbi I would say, at first try to make new csg.js have the same functionality (AMF, STL Binary etc) as the current one (don't try to debug the blob thing, unless OpenJsCad csg.js is so different from OpenJSCAD.org's) as I think that's the least of work to do - once this is settled, then we can look at how to clean up the exports and remove format dependent code from csg.js, e.g. take it out to formats.js (new) where import/export of formats are implemented.

bebbi commented 9 years ago

I need highest prio on keeping joostn fork stable. The amf/stl non-blob formats code needs some finishing, and it won’t work with the joostn openjscad.js ootb I assume. So I don’t want to try merging that back. However, I think separating the current formats code from csg.js would be a clean next step, and what’s nicest, it would break down complexity here. So, how about we target a first objective of having same csg.js but different formats.js in both projects, and afterwards we can start moving things around in format.js? Do you agree with that, or does anyone have another idea? Actually, I plan to look into this eventually, but not in the next days. Is anyone willing to help move this forward here? (maybe @z3dev or @matthijskooijman)?

On 06 Mar 2015, at 11:48, Rene K. Mueller notifications@github.com wrote:

@bebbi I would say, at first try to make new csg.js have the same functionality (AMF, STL Binary etc) as the current one (don't try to debug the blob thing, unless OpenJsCad csg.js is so different from OpenJSCAD.org's) as I think that's the least of work to do - once this is settled, then we can look at how to clean up the exports and remove format dependent code from csg.js, e.g. take it out to formats.js (new) where import/export of formats are implemented.

— Reply to this email directly or view it on GitHub.

matthijskooijman commented 9 years ago

@bebbi, your proposal looks good. It touches a bit on my 2D improvement work, for which I haven't found the time to properly work on it. I did however start on separating exporting from csg.js. I can't quite recall the status, but looking in my git checkout I found two commits, which look ok to me: https://github.com/matthijskooijman/OpenJsCad/commits/improve-2d

Perhaps that'll save some work? :-)

bebbi commented 9 years ago

nice! - is it ready for a pull request?

On 06 Mar 2015, at 13:58, Matthijs Kooijman notifications@github.com wrote:

@bebbi, your proposal looks good. It touches a bit on my 2D improvement work, for which I haven't found the time to properly work on it. I did however start on separating exporting from csg.js. I can't quite recall the status, but looking in my git checkout I found two commits, which look ok to me: https://github.com/matthijskooijman/OpenJsCad/commits/improve-2d

Perhaps that'll save some work? :-)

— Reply to this email directly or view it on GitHub.

matthijskooijman commented 9 years ago

IIRC it doesn't change the structure at all, it just moves code (e.g. conversion functions are still set into the various prototypes directly). I was planning to make the exporters a bit more separate, so in that sense it's not ready yet. Also, I think I only updated processfile.html, breaking all other examples. I was thinking about using some kind of framework to allow require'ing other js file from within openjscad.js instead of harcocding the list in the HTML, but I never got around to discussing this.

In any case: You're welcome to use the code in any way you see fit. If you want to merge the commits as they are, feel free to pull and do so. I won't have any real time to invest into this, probably not until a month from now or so...

bebbi commented 9 years ago

Great. @z3dev, any interest in copying the same approach for the spi.dude repo?

On 06.03.2015, at 14:21, Matthijs Kooijman notifications@github.com wrote:

IIRC it doesn't change the structure at all, it just moves code (e.g. conversion functions are still set into the various prototypes directly). I was planning to make the exporters a bit more separate, so in that sense it's not ready yet. Also, I think I only updated processfile.html, breaking all other examples. I was thinking about using some kind of framework to allow require'ing other js file from within openjscad.js instead of harcocding the list in the HTML, but I never got around to discussing this.

In any case: You're welcome to use the code in any way you see fit. If you want to merge the commits as they are, feel free to pull and do so. I won't have any real time to invest into this, probably not until a month from now or so...

— Reply to this email directly or view it on GitHub.

z3dev commented 9 years ago

Please complete the split of csg.js and format.js. Once you are satisfied with the code, please post back to this thread. At that point, I'll start testing csg.js/format.js (new versions) with @Spiritdude. If there are any required changes then I'll send the pull request to @Spiritdude. That will bring the repositories into sync.

bebbi commented 9 years ago

If there are any required changes

yes, we need someone to do separate out formats.js in spiritdude repo, in analogy to mathijs work for joostn fork.

z3dev commented 9 years ago

I hate to nag, but trying to merge various changes from three different branches is nonsensical, i.e. a waste of time. All those branches need to be managed correctly, merging back into one "main" branch. That allows me (and others) to create another branch, make changes, synchronize differences, and submit changes for review/adoption.

So, what's the main branch for CSG.js? If not "main", why not? @matthijskooijman, can you submit a pull request for your changes to the main branch? Finally, are there any outstanding changes that need review before separating code into format.js?

FYI, I made a fork of joostn/OpenJsCad from gh-pages and can make changes. But this is NOT ideal.

z3dev commented 9 years ago

An alternative approach would be to create a branch. Then we (all of us) can contribute changes to the branch until we have a working version of format.js. @matthijskooijman can contribute those changes from improve-2d, and I can make changes necessary to support OpenJSCAD.org.

z3dev commented 9 years ago

I took a quick look at the formats supported by CSG.js.

Proposal:

Notes:

Questions:

z3dev commented 9 years ago

In addition, CSG.js (OpenJSCAD.org version) has.

openscad.js (OpenJSCAD.org) also has some functions for parsing files of various formats.

I suggest these become part of format.js.

Comments?

Spiritdude commented 9 years ago

@z3dev yes, that was my thought too, to move all format import/export to formats.js I think we need a way to exports the CAG & CSG, and have a low footprint in csg.js but do the heavy work in format.js as exportAMF(), exportOBJ() etc, perhaps @bebbi or @joostn may suggest what's preferable to implement in csg.js as universal kind of exporter, where other functions can extract the required information in formats.js.

@z3dev I think there are now several versions of csg.js floating around, http://evanw.github.io/csg.js/ seems the origin, then joost's, and then OpenJSCAD.org's; to combine joost's and OpenJSCAD.org makes sense, especially when there are speed improvements, whether going with Evan's I don't know. Perhaps Joost can comment on it. It would be preferable that work joost and bebbi is doing perhaps Evan could benefit as well, but I leave this decision up to joost and bebbi who likely know how far those different version of csg.js have gone.

bebbi commented 9 years ago

@z3dev to your points: Our objective is to make sure that as a first objective, joostn csg.js can be linked into spiritdude project. We are now splitting csg.js into csg.js (logic) and formats.js (import export), and we do that separately in both projects. Then, when that's done, we'll point spiritdude csg.js to joostn csg.js, but we don't sync formats.js in that same step yet. Like this, csg.js will be unified, but the different logic in formats.js will make sure both projects continue to work as before. I have now merged in @matthijskooijman changes into joostn split_formats branch. I'll merge this into joostn main branch when spiritdude project is ready. So I think joostn side can be considered ready for the sync.

Re branch choice, joostn uses gh-pages as main branch because that exposes the code as a running web page (google github gh-pages for more info). spiritdude uses master, not gh-pages because it's reachable by a dns record.

Re proposal: check out formats.js in split_formats branch for reference - there are a few lower level funcs (vertex, vector, poly) to be moved as well.

Re notes/questions: I would suggest to keep complexity as low as possible for the very first step, and optimize design (namespace, args etc) when we're done with the csg.js sync. We'll need to keep the export func signatures harmonized with joostn split_formats for the sync. I believe spiritdude repo adds a param to the stl export - IIRC, this reverses the default export behaviour of the func - maybe you can re-reverse that if it's the case. Good question on compact binary - I would suggest to leave that for now, to simplify the sync/as joostn side hasn't moved it (yet). Re keeping old functions, not sure what those are - again, as long as the sync works.. AMF yes, and parse - if they're only openjscad.js related, transfer is optional for syncing csg.js, but up to you - agree that they are probably a better fit fir formats.js

z3dev commented 9 years ago

@bebbi made a great start and enabled a branch (split_formats) for integration efforts. I'll grab CSG.js from that branch, and create a @Spiritdude version of format.js, which will be based on format.js in split_formats, and add AMF support.

Once again, I strongly suggest moving CSG.js (and format.js) to another repository. There will be multiple versions, and having one "master" would make life easier for all. Please consider. Maybe a discussion with Evan is required.

bebbi commented 9 years ago

@z3dev I have thought about it. The obvious tradeoff is the overhead, and maintaining a clear separation (e.g. where does formats.js belong). What are the main benefits of separating it in your view? Additionally, evanw csg.js is never same as openjscad csg.js, see e.g. https://github.com/evanw/csg.js/pull/6

z3dev commented 9 years ago

Given that evanw csg.js hasn't been changed since 2012, I would say that we can proceed separately. You might want to ping him just in case.

My view is that the presentation layer (website, application, script, etc) should be separated. (Just like separating "formats".) joostn is using branches to provide the separation, and that makes makes it difficult for people to know which branch to fork and change? Also, you'll have to merge changes from branch to branch to branch to branch. What about releases? I'm new to GitHub but there are probably other issues as well.

FYI, this is a great library. Take a look at what's possible with a little work to separate the presentation layer properly... http://153.142.230.27:7443/lab/index.php

Spiritdude commented 9 years ago

@z3dev see also https://github.com/Spiritdude/OpenJSCAD.org/pull/73 which addresses splitting up index.html - I haven't pulled it yet, as I thought to wait until csg.js is updated.

bebbi commented 9 years ago

@z3dev joostn code is in a single branch, gh-pages. Agree with separation, but it's more important to have it on code level than adding an org boundary. Also, the separation may not exactly follow csg.js file boundaries. At least for now, I think it's pragmatic to say joostn repos focus on core logic (csg.js/partly openjscad.js), and spiritdude repo (and any other of course) links to joostn for the files that are well separated already (csg.js for now, and going forward parts of openjscad.js/exporters etc as this gets better separated).

z3dev commented 9 years ago

Pull request for the AMF support was submitted for formats.js at joostn OpenJsCad. That should be the last change required before making csg.js binary compatible.

@Spiritdude OpenJSCAD.org now needs to be integrated with the new csg.js and the new formats.js. I hope @Spiritdude can spend some time on this as there's some major changes to move from LightGL.js to THREE.js (GL) library.

If @Spiritdude makes a branch on OpenJSCAD.org for integration then I can spend some time on this.

FYI, there are three additional formats; compact binary(inside CSG.js), GridPro (pull request), and SVG (issue). I suggest those be managed as part of joostn OpenJsCad.

bebbi commented 9 years ago

OK, I was originally suggesting to harmonize only csg.js, not formats.js at this point (mainly because of the difference in the STL export). But your AMF PR looks simple and clean, so I’ll merge that it in unless @joostn disagrees.

I suggest you then test the unified csg.js in spiritdude repo, to make sure everything works well in console and web. Especially try the different STL exports in both environments. Then we have same csg.js in both repos, that’s good progress already!

joostn csg.js has already evolved in the meantime (bufixes and new features). Instead of release cycle overhead, can I suggest that spiritdude repo just links the joostn csg.js in the script tag directly? This will also have the benefit of informing developers where to contribute to csg.js. @spiritdude, do you agree? The approach works here because the repos are essentially the same. Remember that our target is to unify the code base of the shared core logic anyway (and ideally, end up in a single codebase after all).

Re openjscad.js/three.js: Lets not deal with change of openjscad.js and render engine in this step. Lets first close this issue, then discuss next steps to harmonize in a new issue - this will have its own complexities.

On 12 Mar 2015, at 10:10, Z3 Development notifications@github.com wrote:

Pull request for the AMF support was submitted for formats.js at joostn OpenJsCad. That should be the last change required before making csg.js binary compatible.

@Spiritdude OpenJSCAD.org now needs to be integrated with the new csg.js and the new formats.js. I hope @Spiritdude can spend some time on this as there's some major changes to move from LightGL.js to THREE.js (GL) library.

If @Spiritdude makes a branch on OpenJSCAD.org for integration then I can spend some time on this.

FYI, there are three additional formats; compact binary(inside CSG.js), GridPro (pull request), and SVG (issue). I suggest those be managed as part of joostn OpenJsCad.

— Reply to this email directly or view it on GitHub.

z3dev commented 9 years ago

I have upgraded CSG.js and FORMATS.js to the @joostn versions. I produced files (STL ascii, STL binary, AMF, and X3D) for a simple two shape design.

I was able to verify STL ascii and STL binary files using Meshlab, which didn't have any issues and displayed the exported design fine.

The X3D file failed in Meshlab with said "Invalid XML file.", although Meshlab may have specific requirements to read X3D. X3D is a very large specification, and some changes made be required here.

The AMF file exported fine, but I cannot verify the contents. It looks like the two shapes are considered as one large shape. Some changes may be required here.

Any suggestions on how to test the formats.js?

Also, XML is just a string. I would hope that the toX3D() and toAMF() produce a string, which higher level routines can use directly. What's the advantage of BLOB?

bebbi commented 9 years ago

OK, and specifically CLI export to Stl binary works using the joostn version of format.js? It kind of sounds strange that it would work, as spiritdude version changes default behaviour with the p parameter. (see my very first post in this thread.) Can you verify that? Blob helps turn a string into a browser downloadable file, https://developer.mozilla.org/en/docs/Web/API/Blob. @spiritdude, any ideas on how to verify AMF, or what the X3D issues could be?

z3dev commented 9 years ago

@bebbi thanks for the tips. Correct. I was able to export STL binary formatted files using a browser. New version of toStlBinary() I reviewed the differences between the two versions of toStlBinary(). As mentioned above, @Spiritdude added some code to convert the binary arrays into a non-encoded string buffer. This may be related to the lack of BLOB in NODEJS, which I noticed when testing the new version of toStlBinary() using a NODEJS script. BLOB is only defined in browsers, for the purpose of handling binary data formats in JavaScript. However, I found an open source BLOB class, which adapts itself internally for different browsers. https://github.com/eligrey/Blob.js This might work for NODEJS as well.

richievos commented 9 years ago

Hi folks, I've got #73 queued up which is blocked on this PR. I fully support this standardization, so if there's any way I can assist with this please do let me know. Including if it's just some QA.

Has anyone given any thought to automated testing these generate scenarios? From the description in here, it sounds like the main issue is verifying the both the UI and the backend can generate the files. If we generated the file in the UI, and in the backend, and compared the two, I assume that'd cover these cases?

Pulling that off sounds like a PITA however.

z3dev commented 9 years ago

@richievos welcome. I've been using the @matthijskooijman version of CSG.js and formats.js for about 1 week now. No issues, but some slight changes to the HTML files to included formats.js. @Spiritdude, have you completed general testing? Also, i'd be glad to submit a pull request for these changes. is that fine?

z3dev commented 9 years ago

@richievos is correct. we need some test scripts for formats.js. I think that NODEJS be used as the test harness for these libraries. Is there any functionality that requires a browser window or document? If not then I'll try using the open source BLOB to provide the missing functionality.

Spiritdude commented 9 years ago

@z3dev sorry, testing what? @bebbi AMF testing: export and import and see if it works, also there is a AMF toolset, to import into AMF viewer http://amf.wikispaces.com/AMF+Editor which I used. And yes, test scripts would help and move eventually from "alpha" to "beta". :-)

bebbi commented 9 years ago

@z3dev in order to move forward, I suggest this: Check if the format issues, as well as the Blob issue are now only relating to formats.js, correct? Are we then ok to link the @spiritdude csg.js to joostn? If yes, please give green light and I'll merge in the split_formats branch, and we can close this issue. We also need for @spiritdude to agree for project to link csg.js instead of maintaining its own copy. I've got a suggestion for the Blob issue but lets first close this step.

Spiritdude commented 9 years ago

@bebbi linking csg.js - I hard link to github.com/joostn/.../csg.js - sure. I may a copy on the web-site and some kind of fallback in case something goes wrong at github. Source/git wise I am fine to have joostn's csg.js :-)

z3dev commented 9 years ago

I'm also fine. Please merge the changes for formats.js. Thanks for the great help.

Spiritdude commented 9 years ago

@bebbi on a second thought, a local copy is required for the openjscad CLI (nodejs) to work, so not sure how to make sure csg.js resides in the github here, yet is linked/originates from another git repository.

bebbi commented 9 years ago

Does anyone with node.js experience have a quick suggestion? e.g., can you require('http-link')?

Spiritdude commented 9 years ago

It has to work off-line, CLI and Web GUI; so a local copy has to be there. The question is, how to avoid people editing and commit changes of the (new) cgs.js to this git repository. We could add in csg.js a comment saying "changes to this source have to committed to joostn's repository" - not elegant. Any other suggestions?

matthijskooijman commented 9 years ago

git submodule could perhaps work, though it's not always very easy to work with...

richievos commented 9 years ago

The tried and true way to do this in node is to use npm. Once #73 is merged, I was planning on taking a pass at properly "npm"'ing up some of the javascript dependencies in the openjscad.org repo.

The simple short-term solution is just have a Makefile step that wget's csg.js.

The even simpler version is just drop a note in the readme that you have to run "wget .../csg.js" to use the command line client as part of this PR, and follow up immediately with a npm install solution.

bebbi commented 9 years ago

agree with npm-ifying stuff. So spiritdude repo can require a node module containing csg. I wanted to keep that option for when I'm more freed up. But as we have an expert on board, would you be interested in taking a stab at npm-ing the csg side of things (the joostn repo) and adapt the spiritdude cli require call? Makefile: sounds reasonable, would it make sense to add that to some git hook? Depending on your answer to 1, I agree with 3 too ;)

matthijskooijman commented 9 years ago

If we can npm-ify csg.js, can we then also use that in a browser environment? If not, should we perhaps use an alternative module framework (I think there's a few competing standards here).

Right now, in a browser you need to include all needed .js files in the HTML. Ideally, you'd just include "openjscad.js" (or whatever) and that should take care of loading any dependencies / other js files (like "formats.js"). If we can set this up so that this both allows including dependencies inside a browser, and install dependencies using npm or similar, that would be perfect.

I'm not experienced enough with this javascript module stuff to tell if this is at all possible, or what framework to best use, but I wanted to drop this idea into the discussion in case anyone else has a better overview :-)

richievos commented 9 years ago

agree with npm-ifying stuff. So spiritdude repo can require a node module containing csg. I wanted to keep that option for when I'm more freed up. But as we have an expert on board, would you be interested in taking a stab at npm-ing the csg side of things (the joostn repo) and adapt the spiritdude cli require call?

@bebbi if you were referring to me as an expert at javascript, that makes me laugh :). I'm happy to set it up though.

Makefile: sounds reasonable, would it make sense to add that to some git hook?

I'm not completely sure what you mean by that.

If we can npm-ify csg.js, can we then also use that in a browser environment? If not, should we perhaps use an alternative module framework (I think there's a few competing standards here).

Good question, I'm honestly not 100% sure the best practice there. I guess it depends on how openjscad.org is being deployed.

There's 2 simple options I'm aware of. In both options openjscad.org would depend on a specific version of csg.js. In both options it would serve the actual file from openjscad.org/something.../csg.js, but again, it's not the canonical source of that, it's just pulling it down from a remote repo.

Option 1 is you don't commit csg.js to this repo, and as part of the deploy you do an npm install, pulling down csg.js at time of deploy. This assumes there's a way to do that at the time of deploy (I have no idea how this is deployed).

Option 2 is anytime you want to update csg.js you update the version, then do an npm install, then commit the checked in files that it pulled down. This has very few assumptions as to how the deploy works.

Not sure if that's clear, and I'm sure there's other options too, I'll ask around.

z3dev commented 9 years ago

I'd say "keep it simple" for now. Adding a comment to the CSG.js/formats.js (where to make changes) as well as the README seems sufficient. Then, @Spiritdude can notify @bebbi if there are any changes to CSG.js/formats.js from others.

That should give someone enough time to work out the NODEJS, BLOB, and NPM'ing issues. I suspect this effort will require additional changes to classes, etc.

bebbi commented 9 years ago

there are 2 problems to solve: keeping release management of csg.js low (it's changing lots these times), and redirecting contributors to a single source. I think the Makefile + comment in README would satisfy that (as the temp solution). @Spiritdude, would you agree with the offline version requiring a simple make command in the terminal (which downloads the csg.js)? There could even be a verbose instruction when running openjscad and forgetting it. (I'm for Makefile over wget because wget is not a OSX default, and manual download adds complexity for users.)

matthijskooijman commented 9 years ago

@bebbi - probably better to just let the nodejs cli code download the csg.js, instead of using a Makefile (and yet another dependency)?

bebbi commented 9 years ago

that’s also an option if @spiritdude agrees. Means that offline would have to be online for its first use (only).

On 02 Apr 2015, at 14:27, Matthijs Kooijman notifications@github.com wrote:

@bebbi - probably better to just let the nodejs cli code download the csg.js, instead of using a Makefile (and yet another dependency)?

— Reply to this email directly or view it on GitHub.

Spiritdude commented 9 years ago

@bebbi, let me ponder on the release, e.g. right now people git clone and get a working web off-line and cli in one, yes, Makefile which instructs how to get csg.js (what did you come up now, NPMJS or git submodule?) NPMJS I would tend to favour, yet, it really depends how csg.js is going to be maintained now (as part of joost's git or separate).

bebbi commented 9 years ago

@bebbi, let me ponder on the release, e.g. right now people git clone and get a working web off-line and cli in one

yes

, Makefile which instructs how to get csg.js

2 options for user: a) first time cli is started, prompts “running first time - downloading csg.js” b) first time cli is started, prompt: “no csg.js available, please run make” With a slight pref for a) I guess.

(what did you come up now, NPMJS or git submodule?) NPMJS I would tend to favour, yet, it really depends how csg.js is going to be maintained now (as part of joost's git or separate).

Making joostns usable as a npm module sounds like a clean approach, but I guess will take a bit of time. We’ll keep csg.js inside joostn for now.

z3dev commented 9 years ago

I ran into RequireJS today, which can resolve dependencies between JS libraries, and supports both URL and local installations. I think this may solve the issue of "where" CSG.js lives during run time, which is the real issue being discussed now.

The creation of formats.js is complete. Let's close this.

I suggest that a new issue for "How to resolve the dependency on CSG.js?" be broken out, and discussed in another thread.

bebbi commented 9 years ago

@z3dev, my main concern in this thread is removing duplication with csg.js. I think it's not a big issue to maintain 2 formats.js if need be, while of course de-duplicating this also is very nice to have. I'm fine with a short term solution though. @Spiritdude, are you ok with the proposed quick fix options?