OBJLoader2 and WWOBJLoader2 #10631

kaisalmen · 2017-01-23T13:24:51Z

#9756 Added OBJLoader2.js and WWOBJLoader2.js V1.0.1 and three examples: obj2, wwobj2 and wwobj2_parallels.

Header now carry references to development repository.
Added checks for Blob and URL.createObjectURL

mrdoob · 2017-01-23T17:15:47Z

https://rawgit.com/kaisalmen/three.js/WWOBJLoader2/examples/#webgl_loader_obj2
https://rawgit.com/kaisalmen/three.js/WWOBJLoader2/examples/#webgl_loader_wwobj2
https://rawgit.com/kaisalmen/three.js/WWOBJLoader2/examples/#webgl_loader_wwobj2_parallels

Textures not loading due to CORS issues, but otherwise it looks good!

I guess we should handle CORS failures at some point, right now the console is full of warnings.

kaisalmen · 2017-01-23T22:27:33Z

The CORS I do not understand. I have tried to reproduce the issue locally and on my website's server, but it works fine in both cases.

A manual test with Chrome revealed that the following request:
https://rawgit.com/kaisalmen/three.js/WWOBJLoader2/examples/obj/female02/01_-_Default1noCulling.JPG
is returned as:
https://raw.githubusercontent.com/kaisalmen/three.js/WWOBJLoader2/examples/obj/female02/01_-_Default1noCulling.JPG

Edit 2017-01-24:
Ok, the problem seems to originate from the THREE.MTLLoader and default THREE.TextureLoader. Example webgl_loader_obj_mtl.html uses THREE.MTLLoader in combination with THREE.DDSLoader which does not have the CORS issues. I modified webgl_loader_obj2.html to use male02 and dds textures on branch WWOBJLoader2CORS and et voila it works fine:
https://rawgit.com/kaisalmen/three.js/WWOBJLoader2CORS/examples/webgl_loader_obj2.html

If I understand correctly nothing needs to be set regarding CORS as the expectation is to retrieve everything from the same URL, just the server answers unexpectedly with a redirect.
Do we have a new issue here or do I have a lack of knowledge?

Btw, DDSLoader uses FileLoader with arraybuffer and has its own parse method.

kaisalmen · 2017-01-26T19:56:10Z

@mrdoob did you have time to think about the issue? Where can I help here?

mrdoob · 2017-01-27T22:12:26Z

examples/js/loaders/OBJLoader2.js

+		this.fileLoader = new THREE.FileLoader( this.manager );
+
+		this.meshCreator = new THREE.OBJLoader2.MeshCreator();
+		this.parser = new THREE.OBJLoader2.Parser( this.meshCreator );


Is there a reason MeshCreator and Parser need to be public?

How about doing...

THREE.OBJLoader2 = (function () { function MeshCreator( manager ) { ... } function Parser() { ... } function OBJLoader2( manager ) { this.manager = ( manager == null ) ? THREE.DefaultLoadingManager : manager; this.path = ''; this.fileLoader = new THREE.FileLoader( this.manager ); this.meshCreator = new MeshCreator(); this.parser = new Parser( this.meshCreator ); this.validated = false; }

Parser is also used by WWOBJLoader2 to construct the web worker. WWOBJLoader2.prototype._buildWebWorkerCode builds a string, that is afterwards put into a blob to create the worker. It basically requires these classes (link to code), whereas the top block is common code for OBJLoader2 and WWOBJLoader2 and the bottom block is defined privately in WWOBJLoader2. I didn't want to copy the code into both packages and therefore Parser is public:

// parser re-construction this.workerCode += 'if ( THREE.OBJLoader2 === undefined ) { THREE.OBJLoader2 = {} }\n\n'; this.workerCode += buildObject( 'THREE.OBJLoader2.consts', THREE.OBJLoader2.consts ); this.workerCode += buildSingelton( 'THREE.OBJLoader2.Parser', 'Parser', THREE.OBJLoader2.Parser ); this.workerCode += buildSingelton( 'THREE.OBJLoader2._RawObject', '_RawObject', THREE.OBJLoader2._RawObject ); this.workerCode += buildSingelton( 'THREE.OBJLoader2.RawObjectDescription', 'RawObjectDescription', THREE.OBJLoader2.RawObjectDescription ); // web worker construction this.workerCode += buildSingelton( 'THREE.OBJLoader2.WW.OBJLoader', 'OBJLoader', wwDef ); this.workerCode += buildSingelton( 'THREE.OBJLoader2.WW.MeshCreator', 'MeshCreator', wwMeshCreatorDef ); this.workerCode += 'THREE.OBJLoader2.WW.OBJLoaderRef = new THREE.OBJLoader2.WW.OBJLoader();\n\n'; this.workerCode += buildSingelton( 'THREE.OBJLoader2.WW.OBJLoaderRunner', 'OBJLoaderRunner', wwLoaderRunnerDef ); this.workerCode += 'new THREE.OBJLoader2.WW.OBJLoaderRunner();\n\n';

MeshCreator on the other hand could be private as the worker has its own MeshCreator that basically supplies transferables back to WWOBJLoader2 which then creates the Meshes integrated into the scene graph. Initially, exposing MeshCreator seemed required, but this is no longer the case.

I could update the code in the dev repo and push updated bundles to this branch.
What do you think?

I could update the code in the dev repo and push updated bundles to this branch.
What do you think?

Yep, lets start by making MeshCreator private. By making that code private it'll make the code easier to read for someone that wants to improve it (so there are less cases to consider).

Just pushed an update. MeshCreator is now contained within OBJLoader2.
I might have an idea how to make the Parser private, too. OBJLoader2 would expose a builder function and OBJLoader2 needs then to be used internally by WWOBJLoader. Don't know if that is a good idea. Will do some prototyping...

OBJLoader2 would expose a builder function and OBJLoader2 needs then to be used internally by WWOBJLoader. Don't know if that is a good idea. Will do some prototyping...

I like the idea 👍

Builder function works! I pushed another update.
The minified versions of the bundles produce non-working worker code, but this is only a concern for the dev repo. I hope to find the error tomorrow.

Please, have a look at what I did, but don't merge it, yet as minified fix may introduce slight changes to builder functions.

Done! I have pushed V1.0.3.
Uglified/Minified version is built with mangle=false otherwise it becomes almost impossible to predict the names of internally renamed objects when building the code string.

@mrdoob Any further remarks or are we good to go?

I have fixed the CORS issue. WWOBJLoader2 and WWOBJLoader2Director can now pass crossOrigin to MTLLoader.
I read #10465 and also set "anonymous" in the examples and now it works fine with rawgit!

Now, I think all issues you brought up are addressed! 👍

mrdoob · 2017-02-02T23:51:11Z

examples/files.js

@@ -117,6 +118,8 @@ var files = {
 		"webgl_loader_utf8",
 		"webgl_loader_vrml",
 		"webgl_loader_vtk",
+		"webgl_loader_wwobj2",
+		"webgl_loader_wwobj2_parallels",


For clarity... do you mind renaming these to webgl_loader_obj2_ww and webgl_loader_obj2_ww_parallels?

No problem, done.

mrdoob · 2017-02-02T23:52:26Z

examples/webgl_loader_wwobj2_parallels.html

+
+			'use strict';
+
+			if ( THREE.examples === undefined ) THREE.examples = {};


This is not a good practice... We shouldn't add stuff to the THREE namespace like this...

I removed it in the namespace from all examples as it is really not required.

I do the following in OBJLoader2:

if ( THREE.OBJLoader2 === undefined ) { THREE.OBJLoader2 = {} } THREE.OBJLoader2.version = '1.0.4';

Do you generally object to this practice or just for examples. What do you suggest instead?

In the case of the loader itself is fine. It was just THREE.examples as I didn't think it was necessary.

mrdoob · 2017-02-03T23:07:08Z

What's OBJLoader2Verify and WWOBJLoader2Verify for?

kaisalmen · 2017-02-04T19:52:31Z

My personal preference is to separate the what (demo or app) from how it is run. The "how" can even be generalized when the app follows conventions (if you are interested see AppRunner and ThreeJsApp which is used by every demo on my website). This approach helps me to store what I learned in a structured way and to ease restart with code after a longer break.
OBJLoader2Verify and WWOBJLoader2Verify are basically left-overs from that concept.

As I understand it three.js examples should whenever possible follow the single page approach to ease understanding for other users. In the dev repo I use some template + gulp code to create the single page examples meaning the external content is baked into the files.

I guess, the names I chose weren't the best. What about OBJLoader2Example and WWOBJLoader2Example. If you say, no, please make it simpler and flaltten the classes and make it as simple as possible like for instance the original example webgl_loader_obj then I will do that, because you are the owner of the bar. ;-)

Is there anything else that should be discussed or modified?

kaisalmen · 2017-02-06T14:39:49Z

Hey, I added a forgotten feature: WWOBJLoader2 can now be configured to stream or not stream in the mesh data (new: show only once all meshes are loaded from file).
Example "webgl_loader_obj2_ww" can now load user OBJ&MTL files (file dialogue with multi selection) and clear all meshes if the user wants to.
All examples now use dat.gui.

@mrdoob I am done with extending the examples. I will stop changing things now and wait for your feedback. Looking forward to have this merge request completed. 😊

kaisalmen · 2017-02-13T21:46:00Z

@mrdoob do you plan to include this in r85?

kaisalmen · 2017-02-16T14:03:47Z

I have adjusted OBJLoader2, WWOBJLoader and the original OBJLoaderto be compatible with the MultiMaterial removal. Locally, I have rebased branch WWOBJLoader2 to dev head (this WWOBJLoader2Alt is how it will look like).
Are you ok with that and shall then I force push WWOBJLoader2?
Or, should I better merge the dev head into WWOBJLoader2 and put the last commit on top?
Or, none of the above?
What do you think?
Thanks!

kaisalmen · 2017-03-22T07:24:07Z

@mrdoob, @jonnenauha I have combined all commits into one with larger comment listing all changes and put it on top of dev commit 6776a15
A second commit upgrades WWOBJLoader2 to V1.1.0 which aligns it to MultiMaterial removal.

jonnenauha · 2017-03-22T08:49:16Z

Again I'm hoping the new loader will be included in the repo at some point. I just don't have the power to do that. I'm sure @mrdoob will eventually get here to give some feedback on the situation, has a lot on his plate.

From my perspective the code looks good, demos are great and the perf should be there.

mrdoob · 2017-03-22T16:29:50Z

examples/js/loaders/OBJLoader2.js

+			this.rawObjectDescriptions[ index ] = this.rawObjectDescriptionInUse;
+		}
+
+		RawObject.prototype._buildIndex = function ( materialName, smoothingGroup) {


Minor thing... why are you underscoring these methods?

RawObject and MeshCreator were initially public and all non-public methods were underscored. This is no longer necessary and it is likely misleading. I was just lazy. Should be fairly easy to adjust... Give me half an hour.
Thanks for changing the title, btw. 😉

kaisalmen · 2017-03-22T23:04:22Z

I forced/combined another commit:

OBJLoader2 and WWOBJLoader2: Removed underscores from methods in private classes. Methods of public classes are untouched.
Class names in generated worker code are now distinguishable from classes in OBJLoader2. This became clear during refactoring with WebStorm as it identified false-positives all the time.

I carefully checked for regressions. It looks good as far as I can see! 😄 Good night! 😴

Edit 2017-03-23: All changes were just related to names (functions and classes). I did not perform any functional code changes. I think it is more logical now. Thank you for pointing at it!

Improvements since initial external V1.0.0 release: OBJLoader2: - Removed need for making Parser public. OBJLoader2 has a build function for web worker code. - MeshCreator is now private to OBJLoader2 WWOBJLoader2: - Added checks for Blob and URL.createObjectURL - Worker code build: Removed need to adjust constructor and some new Object calls - Allow to properly set CORS to MTLLoader via WWOBJLoader2 and WWOBJLoader2Director - Now allows to enable/disable mesh streaming Example webgl_loader_obj - Added GridHelper - resources to load are now defined outside example classes Example webgl_loader_obj2_ww - Allow to clear all meshes in - Allows to load user OBJ/MTL files - Added GridHelper - resources to load are now defined outside example classes All Examples: - Created one page examples and tuned naming - All examples now use dat.gui - Removed namespace "THREE.examples" - Fixed comment typos - Fixed some code formatting issues - Fixed tabs in examples General: - Headers now carry references to development repository

OBJLoader2: - Removed underscores from functions of private classes WWOBJLoader2: - Adjusted naming of web worker classes

Adjusted to removal of MultiMaterial

kaisalmen · 2017-03-25T12:16:45Z

@mrdoob, combing all commits again was a little to harsh (late night ideas aren't always the best). I did some git magic. Now, the following commit shows exactly what I did on Wednesday: 75de261

mrdoob · 2017-03-26T18:34:42Z

Sweet!

mrdoob · 2017-03-26T18:34:47Z

Many thanks!

kaisalmen · 2017-03-26T20:01:31Z

Thank you! 😄

kaisalmen force-pushed the WWOBJLoader2 branch from 6f2e7af to 8da8b72 Compare January 23, 2017 22:16

kaisalmen force-pushed the WWOBJLoader2 branch from 8da8b72 to 8e20478 Compare January 23, 2017 22:32

mrdoob reviewed Jan 27, 2017

View reviewed changes

mrdoob reviewed Feb 2, 2017

View reviewed changes

kaisalmen force-pushed the WWOBJLoader2 branch from 55d8498 to 367b264 Compare March 22, 2017 07:19

mrdoob changed the title ~~#9756 As proposed in latest comment of issue~~ OBJLoader2 and WWOBJLoader2 Mar 22, 2017

mrdoob reviewed Mar 22, 2017

View reviewed changes

kaisalmen force-pushed the WWOBJLoader2 branch from 367b264 to 8e2e6d5 Compare March 22, 2017 22:57

kaisalmen added 3 commits March 25, 2017 13:07

mrdoob#9756

75de261

OBJLoader2: - Removed underscores from functions of private classes WWOBJLoader2: - Adjusted naming of web worker classes

mrdoob#9756 Updated to WWOBJLoader2 V1.1.0:

0ff805f

Adjusted to removal of MultiMaterial

kaisalmen force-pushed the WWOBJLoader2 branch from 8e2e6d5 to 0ff805f Compare March 25, 2017 12:13

mrdoob merged commit ff25afe into mrdoob:dev Mar 26, 2017

kaisalmen deleted the WWOBJLoader2 branch March 26, 2017 20:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OBJLoader2 and WWOBJLoader2 #10631

OBJLoader2 and WWOBJLoader2 #10631

kaisalmen commented Jan 23, 2017

mrdoob commented Jan 23, 2017 •

edited

Loading

kaisalmen commented Jan 23, 2017 •

edited

Loading

kaisalmen commented Jan 26, 2017

mrdoob Jan 27, 2017 •

edited

Loading

mrdoob Jan 27, 2017 •

edited

Loading

kaisalmen Jan 28, 2017

mrdoob Jan 28, 2017

kaisalmen Jan 29, 2017

mrdoob Jan 29, 2017

kaisalmen Jan 29, 2017

kaisalmen Jan 30, 2017

kaisalmen Feb 1, 2017

kaisalmen Feb 2, 2017

mrdoob Feb 2, 2017

kaisalmen Feb 3, 2017

mrdoob Feb 2, 2017

kaisalmen Feb 3, 2017

mrdoob Feb 3, 2017

mrdoob commented Feb 3, 2017 •

edited

Loading

kaisalmen commented Feb 4, 2017 •

edited

Loading

kaisalmen commented Feb 6, 2017 •

edited

Loading

kaisalmen commented Feb 13, 2017

kaisalmen commented Feb 16, 2017 •

edited

Loading

kaisalmen commented Mar 22, 2017

jonnenauha commented Mar 22, 2017

mrdoob Mar 22, 2017 •

edited

Loading

kaisalmen Mar 22, 2017

kaisalmen commented Mar 22, 2017 •

edited

Loading

kaisalmen commented Mar 25, 2017 •

edited

Loading

mrdoob commented Mar 26, 2017

mrdoob commented Mar 26, 2017 •

edited

Loading

kaisalmen commented Mar 26, 2017


		'use strict';

		if ( THREE.examples === undefined ) THREE.examples = {};

OBJLoader2 and WWOBJLoader2 #10631

OBJLoader2 and WWOBJLoader2 #10631

Conversation

kaisalmen commented Jan 23, 2017

mrdoob commented Jan 23, 2017 • edited Loading

kaisalmen commented Jan 23, 2017 • edited Loading

kaisalmen commented Jan 26, 2017

mrdoob Jan 27, 2017 • edited Loading

Choose a reason for hiding this comment

mrdoob Jan 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrdoob commented Feb 3, 2017 • edited Loading

kaisalmen commented Feb 4, 2017 • edited Loading

kaisalmen commented Feb 6, 2017 • edited Loading

kaisalmen commented Feb 13, 2017

kaisalmen commented Feb 16, 2017 • edited Loading

kaisalmen commented Mar 22, 2017

jonnenauha commented Mar 22, 2017

mrdoob Mar 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaisalmen commented Mar 22, 2017 • edited Loading

kaisalmen commented Mar 25, 2017 • edited Loading

mrdoob commented Mar 26, 2017

mrdoob commented Mar 26, 2017 • edited Loading

kaisalmen commented Mar 26, 2017

mrdoob commented Jan 23, 2017 •

edited

Loading

kaisalmen commented Jan 23, 2017 •

edited

Loading

mrdoob Jan 27, 2017 •

edited

Loading

mrdoob Jan 27, 2017 •

edited

Loading

mrdoob commented Feb 3, 2017 •

edited

Loading

kaisalmen commented Feb 4, 2017 •

edited

Loading

kaisalmen commented Feb 6, 2017 •

edited

Loading

kaisalmen commented Feb 16, 2017 •

edited

Loading

mrdoob Mar 22, 2017 •

edited

Loading

kaisalmen commented Mar 22, 2017 •

edited

Loading

kaisalmen commented Mar 25, 2017 •

edited

Loading

mrdoob commented Mar 26, 2017 •

edited

Loading