GCodeLoader and example #12897
GCodeLoader and example #12897
Conversation
|
I think you should follow the Code Style https://github.com/mrdoob/three.js/wiki/Mr.doob's-Code-Style%E2%84%A2 |
|
Updated code to follow the format used in repository! @takahirox Thanks a lot! |
examples/js/loaders/GCodeLoader.js
Outdated
| var self = this; | ||
|
|
||
| var loader = new THREE.FileLoader( self.manager ); | ||
| loader.setPath( this.path ); |
There was a problem hiding this comment.
This line can be deleted, see #12731
|
I still have 83 ESLint errors with the current loader. Please use https://zz85.github.io/mrdoobapproves/ or a linter plugin to resolve these problems. |
examples/webgl_loader_gcode.html
Outdated
|
|
||
| function animate() { | ||
|
|
||
| controls.update(); |
There was a problem hiding this comment.
Updating the controls is not necessary...
There was a problem hiding this comment.
You can also change the scope of controls. This variable does not need to be global...
|
Reformated the code and removed controls update from the example. Cumps |
examples/js/loaders/GCodeLoader.js
Outdated
|
|
||
| this.manager = ( manager !== undefined ) ? manager : THREE.DefaultLoadingManager; | ||
|
|
||
| } |
There was a problem hiding this comment.
Sorry for being so pedantic 😇 . Missing semicolon here...
There was a problem hiding this comment.
No problem i think you should be 👍,
examples/js/loaders/GCodeLoader.js
Outdated
|
|
||
| if ( currentLayer === undefined ) { | ||
| newLayer( line ); | ||
| } |
There was a problem hiding this comment.
Like so...
if ( currentLayer === undefined ) {
newLayer( line );
}
|
Updated the code, removed trailing spaces and fixed spacing in if/for |
|
The example renders with 242 draw calls. Also, why is the model rendered with lines? I assume it is intentional... |
|
Yes it is intentional, GCode stores paths used by CNC / 3D printer machines, can be usefull to preview data for these kind of systems. For convenience i have splitted it by layers, thats why it renders with so many draw calls. |
I assume the layers are only required by the CNC... Can you fairly easily create only one instance of |
|
I can but i think its not worth it. But i can add an option to have the path all in one Object. |
examples/js/loaders/GCodeLoader.js
Outdated
| type: grouptype, | ||
| feed: line.e, | ||
| extruding: line.extruding, | ||
| geometry: new THREE.Geometry(), |
There was a problem hiding this comment.
BTW: It would be actually better to produce BufferGeometry since we try to remove Geometry from the loaders.
|
Moved to buffer geometry, but im also adding this to a single LineSegments object ;) |
|
Moved to a single geometry, with option to split by layer! |
| function addObject(vertex, extruding) { | ||
|
|
||
| var geometry = new THREE.BufferGeometry(); | ||
| geometry.addAttribute( 'position', new THREE.BufferAttribute( new Float32Array( vertex ), 3 ) ); |
There was a problem hiding this comment.
Instead of
new THREE.BufferAttribute( new Float32Array( vertex ), 3 )write it like this
new THREE.Float32BufferAttribute( vertex, 3 )There was a problem hiding this comment.
Also, please check the code style again. There are some new formatting issues.
|
There is no need for an animation loop with a static scene. Please use this pattern, instead: controls = new THREE.OrbitControls( camera, renderer.domElement );
controls.addEventListener( 'change', render );
controls.minDistance = 5; // whatever
controls.maxDistance = 100; // whatever... and add a call to |
|
@WestLangley Good point! |
|
Merging for now. |
|
Thanks! |
Hi
Added a GCodeLoader and a example page using the benchy model http://www.3dbenchy.com/.
This loader was based on the loader made by @joewalnes.
Thanks a lot!
Cumps