Minified OBJLoader2 Async fails on building worker code

Question

Minified OBJLoader2 Async fails on building worker code

mrz5018 opened this issue 7 years ago · 20 comments

When OBJLoader2 is used async it builds the worker code by building up a string and passing it to the worker. Unfortunately some of the string is hard coded. That means that when it is minified (as all of my code is when I deploy) the strings aren't the correct class/variable name any more.

In the following code you can see that LoaderSupport is hard-coded (eg. var LoaderSupport) but the Parser class definition has been minified, so where it should say LoaderSupport.Validator in prototype.setMaterials it says mh.Validator instead. I can't think of a good way to handle this. Any ideas???

    var LoaderSupport = {};

    LoaderSupport.Validator = {
	    isValid: function(e){return null!==e&&void 0!==e},
	    verifyInput: function(e,t){return null===e||void 0===e?t:e},
    }

    Parser = (function () {

	function e(){this.callbackProgress=null,this.callbackMeshBuilder=null,this.contentRef=null,this.legacyMode=!1,this.materials={},this.useAsync=!1,this.materialPerSmoothingGroup=!1,this.useIndices=!1,this.disregardNormals=!1,this.vertices=[],this.colors=[],this.normals=[],this.uvs=[],this.rawMesh={objectName:"",groupName:"",activeMtlName:"",mtllibName:"",faceType:-1,subGroups:[],subGroupInUse:null,smoothingGroup:{splitMaterials:!1,normalized:-1,real:-1},counts:{doubleIndicesCount:0,faceCount:0,mtlCount:0,smoothingGroupCount:0}},this.inputObjectCount=1,this.outputObjectCount=1,this.globalCounts={vertices:0,faces:0,doubleIndicesCount:0,lineByte:0,currentByte:0,totalBytes:0},this.logging={enabled:!0,debug:!1}}

	e.prototype.resetRawMesh = function(){this.rawMesh.subGroups=[],this.rawMesh.subGroupInUse=null,this.rawMesh.smoothingGroup.normalized=-1,this.rawMesh.smoothingGroup.real=-1,this.pushSmoothingGroup(1),this.rawMesh.counts.doubleIndicesCount=0,this.rawMesh.counts.faceCount=0,this.rawMesh.counts.mtlCount=0,this.rawMesh.counts.smoothingGroupCount=0};

	e.prototype.setUseAsync = function(e){this.useAsync=e};

	e.prototype.setMaterialPerSmoothingGroup = function(e){this.materialPerSmoothingGroup=e};

	e.prototype.setUseIndices = function(e){this.useIndices=e};

	e.prototype.setDisregardNormals = function(e){this.disregardNormals=e};

	e.prototype.setMaterials = function(e){this.materials=mh.Validator.verifyInput(e,this.materials),this.materials=mh.Validator.verifyInput(this.materials,{})};

	e.prototype.setCallbackMeshBuilder = function(e){if(!mh.Validator.isValid(e))throw'Unable to run as no "MeshBuilder" callback is set.';this.callbackMeshBuilder=e};

	e.prototype.setCallbackProgress = function(e){this.callbackProgress=e};

And just for reference, here is the code that is causing the problem (the build worker code):

      var buildCode = function(funcBuildObject, funcBuildSingleton) {
            var workerCode = '';
            workerCode += '\n\n';
            workerCode += 'var LoaderSupport = {};\n\n';
            workerCode += funcBuildObject('LoaderSupport.Validator', Validator);
            workerCode += funcBuildSingleton('Parser', Parser);
            return workerCode;
      };
      this.workerSupport.validate(buildCode, 'Parser');
      this.workerSupport.setCallbacks(scopedOnMeshLoaded, scopedOnLoad);
      if (scope.terminateWorkerOnLoad) { this.workerSupport.setTerminateRequested(true); }

edit: add javascript tag to code

Answer 1 · 2018-09-05T20:43:06.000Z

+1

Having this exact problem when I generate a build on Angular 6.

Answer 2 · 2018-09-05T21:03:39.000Z

I will take a look deeper on this case but... i think this is a bug for three js library not this converter.

I aware that this "bug" appear due to the es6 portage of OBJLoader2, but unfortunately i'm not able to determine what will be the minimified version of such code...

Currently the best thing to do could be a manual find an replace against var LoaderSupport with the minimified version of it.

Answer 3 · 2018-09-06T18:47:16.000Z

@Itee your suggestion is a nice work around! I saw the PR hopefully that gets merged.

Answer 4 · 2018-09-06T20:50:10.000Z

I hope too ^^ and thanks.

Answer 5 · 2018-09-07T14:00:41.000Z

Good news it is merged ! This will be fixed in the next release. Wait and see before close ( i will try to apply a patch before the next threejs release )

Answer 6 · 2018-09-11T20:20:28.000Z

This should be fixed in latest release v11.3.0. If not please reopen.

Answer 7 · 2018-09-18T19:36:51.000Z

@Itee I think that this should in theory take care of the issue, but it fails for me because of a different issue. LoaderSupport.constructor.name is actually just "Object" on my build. I don't know if this has something to do with the fact that I'm using meteor, but somewhere in the build process there is some issue if this works for you but not for me. Any ideas?

Answer 8 · 2018-09-18T22:14:21.000Z

MMmmmhh... You're right !

Here a piece of code of LoaderSupport:

var LoaderSupport = {}
LoaderSupport.Validator = {
	
	isValid: function( input ) {
		return ( input !== null && input !== undefined );
	},
	
	verifyInput: function( input, defaultValue ) {
		return ( input === null || input === undefined ) ? defaultValue : input;
	}
};

So yes it is an Object... and Validator too...

Are you able to provide a working example with the minification process ? In this way i will maybe able to find a better way to fix this.

At least, are you able to provide the minified version of THREE.LoaderSupport and THREE.LoaderSupport.Validator.

Maybe the template string could be the solution...

Answer 9 · 2018-09-21T18:39:05.000Z

@Itee I wanted to forward port your fix for three.js and it broke non-minified code. I will help to come up with a solution...

Answer 10 · 2018-09-21T19:48:49.000Z

Thanks for the quick response. And to clear up any confusion, I was accidentally logged in as @mm3djordan, so the previous should have been from this account. Whoops!

Regardless, here is the code that shows up when I try to use the V11.3.0:

THREE = { Object: {} };

Object.Object = {
	isValid: function( input ) {
  		return ( input !== null && input !== undefined );
  	},
	verifyInput: function( input, defaultValue ) {
  		return ( input === null || input === undefined ) ? defaultValue : input;
  	},
}

edit: add javascript tag to code

Answer 11 · 2018-09-23T00:02:53.000Z

@kaisalmen Yes we clearly should revert this PR and thanks for help !

@mrz5018 Thank for the output.

So just to clarify. We need to find a solution to be able to include a reference about a namespaced class in a string used by a worker that work for minified or not code...

and the incriminated piece of code:

var buildCode = function ( funcBuildObject, funcBuildSingleton ) {
			var workerCode = '';
			workerCode += '/**\n';
			workerCode += '  * This code was constructed by OBJLoader2 buildCode.\n';
			workerCode += '  */\n\n';
			workerCode += 'THREE = { ' + LoaderSupport.constructor.name + ': {} };\n\n';
			workerCode += funcBuildObject( 'THREE.' + LoaderSupport.constructor.name + '.' + LoaderSupport.Validator.constructor.name, Validator );
			workerCode += funcBuildSingleton( 'Parser', Parser );

			return workerCode;
		};

here my minified version of it:

function(e,t){var n="";return n+="\n\n",n+="THREE = { "+LoaderSupport.constructor.name+": {} };\n\n",n+=e(LoaderSupport.constructor.name+"."+LoaderSupport.Validator.constructor.name,r),n+=t("Parser",i)};

and here the minimified version of LoaderSupport:

LoaderSupport={};LoaderSupport.Validator={isValid:function(e){return null!==e&&void 0!==e},verifyInput:function(e,t){return null===e||void 0===e?t:e}}

So with the minimized version build along Three-Full this should work... But ! In case you're including OBJLoader2 in your own building process this will break !

But... I think i found a solution:

////////// Tada !!!
var variableNameToString = function( object, childIndex ) {
	return Object.keys( object )[childIndex]
}
//////////

var buildCode = function ( funcBuildObject, funcBuildSingleton ) {
	var workerCode = '';
	workerCode += '\n\n';
	workerCode += 'THREE = { ' + variableNameToString( {LoaderSupport}, 0 ) + ': {} };\n\n';
	workerCode += funcBuildObject( '' + variableNameToString( {LoaderSupport}, 0 ) + '.' + variableNameToString( LoaderSupport, 0 ), Validator );
	workerCode += funcBuildSingleton( 'Parser', Parser );

	return workerCode;
};

Unfortunately this won't work in threejs due to wanted backward compatibility and because this code use ES6's object destructuring.

But i will implement it in three-full as a patch.

Answer 12 · 2018-09-23T00:10:40.000Z

Damned ! Uglify convert it to:

h=function(t,e){return Object.keys(t)[e]},l=function(t,e){var s="";return s+="\n\n",s+="THREE = { "+h({LoaderSupport:Pt},0)+": {} };\n\n",s+=t(h({LoaderSupport:Pt},0)+"."+h(Pt,0),i),s+=e("Parser",r)};

So the output will still contain 'LoaderSupport'...

The only things that i/we can do is to avoid mangling LoaderSupport in minimified version in case you're using uglifyjs in your build process...

Answer 13 · 2018-09-24T18:34:52.000Z

This is nasty, but works for non-minified code:
THREE.OBJLoader2.Parser defines this method:

getOwnDefinition: function () {
	return THREE.OBJLoader2.Parser;
},

buildWorkerCode takes the complete function string and extracts the words from the function (here [ 'function', 'return', 'THREE', 'OBJLoader2', 'Parser' ]:

var funcGetOwnDefinition = THREE.OBJLoader2.Parser.prototype[ 'getOwnDefinition' ] + '';
var words = funcGetOwnDefinition.match( /\w+/g );
var workerCode = '';
workerCode += '/**\n';
workerCode += '  * This code was constructed by THREE.OBJLoader2.buildWorkerCode.\n';
workerCode += '  */\n\n';
workerCode += words[ 2 ] + ' = { ' + words[ 3 ] + ': {} };\n\n';
workerCode += codeSerializer.serializeClass( words[ 2 ] + '.' + words[ 3 ] + '.' + words[ 4 ], THREE.OBJLoader2.Parser );

A minifier should always replace the definition in getOwnDefinition. If we outfit all objects that should be serialized with such a method it should work.
What do you think?

Answer 14 · 2018-09-26T21:18:17.000Z

Ok, this is how it could work more generically. Variables will contain the distilled name parts (minified or not). This whole approach only works with a method in a prototype referencing the object directly:

var MinifySafeDef = {};
MinifySafeDef.prototype = {

	getParserDef: function () {
		return THREE.OBJLoader2.Parser;
	},

	getWorkerLoaderDef: function () {
		return THREE.WorkerLoader;
	}
};
var partsParser = codeSerializer.extractObjectNamesArray( 'getParserDef', MinifySafeDef  );
var partsWorkerLoaderDef = codeSerializer.extractObjectNamesArray( 'getWorkerLoaderDef', MinifySafeDef  );

Suggestion from previous post transformed to a function used above.

extractObjectNamesArray: function ( methodName, protoMinifyDef ) {
	var funcGetOwnDefinition = '';
	var words = [];
	if ( typeof protoMinifyDef.prototype[ methodName ] === 'function' ) {

		funcGetOwnDefinition = protoMinifyDef.prototype[ methodName ] + '';
		words = funcGetOwnDefinition.match( /\w+/g );

		if ( words.length > 2 ) {

			words = words.slice( 2 );

		} else {

			throw 'Unable to extract names properly from "' + methodName + '"!';

		}

	} else {

		throw 'Prototype method "' + methodName + '" is not available!';

	}
	return words;
}

Answer 15 · 2018-09-27T11:35:42.000Z

Ok, nice experiments, but it seems I am on the wrong track. Those names I suspected do not get mangled at all. So, what I wanted to achieve/protect had no point.

But, I made one important observation. In latest dev code (V3.x) everything resides under the same namespace and I no longer use singletons. In V2.x Parser resides inside OBJLoader2 singleton and I assume that is where the problem originates: Uglify seems to treat code from different namespaces differently.

Answer 16 · 2018-09-28T20:00:25.000Z

@kaisalmen

In latest dev code (V3.x) everything resides under the same namespace and I no longer use singletons. In V2.x Parser resides inside OBJLoader2 singleton and I assume that is where the problem originates: Uglify seems to treat code from different namespaces differently.

When should be release the v3 ? and/or how could give a minification try with it ?

About your current methods, this approch could work except that i make a global find/replace against THREE namespace.

So your code will/should become like this:

var MinifySafeDef = {};
MinifySafeDef.prototype = {

	getParserDef: function () {
		return OBJLoader2.Parser;
	},

	getWorkerLoaderDef: function () {
		return WorkerLoader;
	}
};
var partsParser = codeSerializer.extractObjectNamesArray( 'getParserDef', MinifySafeDef  );
var partsWorkerLoaderDef = codeSerializer.extractObjectNamesArray( 'getWorkerLoaderDef', MinifySafeDef  );

Answer 17 · 2018-09-30T19:24:55.000Z

If possible, I want to open a PR for V3.x for the next three.js release, but it depends on my time and the review, of course. V3 removes LoaderSupport and introduces WorkerLoader. The change is quite substantial, but it makes building other worker based loaders easier (see issue 13664).
Code is here:
https://github.com/kaisalmen/WWOBJLoader/tree/Issue36
Bundle + Minified version can be build with gulp. There still is an issue with minification similar to this one that I need to fix (only sub-pieces of three.js in Worker Blob for file loading lead to broken code)

Answer 18 · 2018-10-09T20:13:11.000Z

@Itee I decided that there will be another V2 release. You find a pre-release here:
https://github.com/kaisalmen/WWOBJLoader/releases/tag/V2.5.0-beta
(contains LoaderSupport[.min].js and OBJLoader2[.min].js)

Singletons are gone here (backport from V3 dev). Everything is now under namespace THREE.OBJLoader2 or THREE.LoaderSupport. No more private objects are put to Worker Blob.
Let me know if this issue still exists with the new code structure. Thank you.

Edit: The minified code I create with gulp and uglify.js works fine.

Answer 19 · 2018-11-01T16:16:16.000Z

@kaisalmen and @Itee thanks for working on this so much! I've just been hanging out at V10.0.0 until this was fully completed.

I see that @kaisalmen released V2.5.0 of WWOBJLoader. What's the process for pulling that into three-full? Just a drag and drop? I'd be happy to set up the pull request if that would help out.

Answer 20 · 2018-12-07T19:35:35.000Z

@Itee Any updates?