Skip to content

LDrawLoader: Fix stack overflow exceptions #16245

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Apr 16, 2019
Merged

LDrawLoader: Fix stack overflow exceptions #16245

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
760c5e6
resolve rebase conflicts
gkjohnson Apr 12, 2019
46a7f48
Fix name to lowercase
gkjohnson Apr 7, 2019
5f00c11
remove unused variable
gkjohnson Apr 12, 2019
8a4ca06
update name lowercase logic
gkjohnson Apr 12, 2019
4a683e2
comments
gkjohnson Apr 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 48 additions & 76 deletions examples/js/loaders/LDrawLoader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,18 +271,6 @@ THREE.LDrawLoader = ( function () {

}, onProgress, onError );

function subobjectLoad( url, onLoad, onProgress, onError, subobject ) {

var fileLoader = new THREE.FileLoader( scope.manager );
fileLoader.setPath( scope.path );
fileLoader.load( url, function ( text ) {

processObject( text, onLoad, subobject );

}, onProgress, onError );

}

function processObject( text, onProcessed, subobject ) {

var parseScope = scope.newParseScopeLevel();
Expand All @@ -292,6 +280,12 @@ THREE.LDrawLoader = ( function () {

// Add to cache
var currentFileName = parentParseScope.currentFileName;
if ( currentFileName !== null ) {

currentFileName = parentParseScope.currentFileName.toLowerCase();

}

if ( scope.subobjectCache[ currentFileName ] === undefined ) {

scope.subobjectCache[ currentFileName ] = text;
Expand All @@ -308,37 +302,39 @@ THREE.LDrawLoader = ( function () {
parseScope.numSubobjects = parseScope.subobjects.length;
parseScope.subobjectIndex = 0;

if ( parseScope.numSubobjects > 0 ) {
var finishedCount = 0;
onSubobjectFinish();

// Load the first subobject
var subobjectGroup = loadSubobject( parseScope.subobjects[ 0 ], true );
return objGroup;

// Optimization for loading pack: If subobjects are obtained from cache, keep loading them iteratively rather than recursively
if ( subobjectGroup ) {
function onSubobjectFinish() {

while ( subobjectGroup && parseScope.subobjectIndex < parseScope.numSubobjects - 1 ) {
finishedCount ++;

subobjectGroup = loadSubobject( parseScope.subobjects[ ++ parseScope.subobjectIndex ], true );
if ( finishedCount === parseScope.subobjects.length + 1 ) {

}

if ( subobjectGroup ) {
finalizeObject();

finalizeObject();
} else {

}
// Once the previous subobject has finished we can start processing the next one in the list.
// The subobject processing shares scope in processing so it's important that they be loaded serially
// to avoid race conditions.
// Promise.resolve is used as an approach to asynchronously schedule a task _before_ this frame ends to
// avoid stack overflow exceptions when loading many subobjects from the cache. RequestAnimationFrame
// will work but causes the load to happen after the next frame which causes the load to take significantly longer.
var subobject = parseScope.subobjects[ parseScope.subobjectIndex ];
Promise.resolve().then( function () {

}
loadSubobject( subobject );

} else {
} );
parseScope.subobjectIndex ++;

// No subobjects, finish object
finalizeObject();
}

}

return objGroup;

function finalizeObject() {

if ( ! scope.separateObjects && ! parentParseScope.isFromParse ) {
Expand Down Expand Up @@ -368,7 +364,7 @@ THREE.LDrawLoader = ( function () {

}

function loadSubobject( subobject, sync ) {
function loadSubobject( subobject ) {

parseScope.mainColourCode = subobject.material.userData.code;
parseScope.mainEdgeColourCode = subobject.material.userData.edgeMaterial.userData.code;
Expand All @@ -382,16 +378,15 @@ THREE.LDrawLoader = ( function () {
}

// If subobject was cached previously, use the cached one
var cached = scope.subobjectCache[ subobject.originalFileName ];
var cached = scope.subobjectCache[ subobject.originalFileName.toLowerCase() ];
if ( cached ) {

var subobjectGroup = processObject( cached, sync ? undefined : onSubobjectLoaded, subobject );
if ( sync ) {
processObject( cached, function ( subobjectGroup ) {

addSubobject( subobject, subobjectGroup );
return subobjectGroup;
onSubobjectLoaded( subobjectGroup, subobject );
onSubobjectFinish();

}
}, subobject );

return;

Expand Down Expand Up @@ -451,22 +446,6 @@ THREE.LDrawLoader = ( function () {
// All location possibilities have been tried, give up loading this object
console.warn( 'LDrawLoader: Subobject "' + subobject.originalFileName + '" could not be found.' );

// Try to read the next subobject
parseScope.subobjectIndex ++;

if ( parseScope.subobjectIndex >= parseScope.numSubobjects ) {

// All subojects have been loaded. Finish parent object
scope.removeScopeLevel();
onProcessed( objGroup );

} else {

// Load next subobject
loadSubobject( parseScope.subobjects[ parseScope.subobjectIndex ] );

}

return;

}
Expand All @@ -481,15 +460,22 @@ THREE.LDrawLoader = ( function () {
fileLoader.setPath( scope.path );
fileLoader.load( subobjectURL, function ( text ) {

processObject( text, onSubobjectLoaded, subobject );
processObject( text, function ( subobjectGroup ) {

}, undefined, onSubobjectError );
onSubobjectLoaded( subobjectGroup, subobject );
onSubobjectFinish();

}
}, subobject );

}, undefined, function ( err ) {

onSubobjectError( err, subobject );

}, subobject );

function onSubobjectLoaded( subobjectGroup ) {
}

var subobject = parseScope.subobjects[ parseScope.subobjectIndex ];
function onSubobjectLoaded( subobjectGroup, subobject ) {

if ( subobjectGroup === null ) {

Expand All @@ -502,20 +488,6 @@ THREE.LDrawLoader = ( function () {
// Add the subobject just loaded
addSubobject( subobject, subobjectGroup );

// Proceed to load the next subobject, or finish the parent object

parseScope.subobjectIndex ++;

if ( parseScope.subobjectIndex < parseScope.numSubobjects ) {

loadSubobject( parseScope.subobjects[ parseScope.subobjectIndex ] );

} else {

finalizeObject();

}

}

function addSubobject( subobject, subobjectGroup ) {
Expand All @@ -533,10 +505,10 @@ THREE.LDrawLoader = ( function () {

}

function onSubobjectError( err ) {
function onSubobjectError( err, subobject ) {

// Retry download from a different default possible location
loadSubobject( parseScope.subobjects[ parseScope.subobjectIndex ] );
loadSubobject( subobject );

}

Expand Down Expand Up @@ -1071,7 +1043,7 @@ THREE.LDrawLoader = ( function () {
if ( line.startsWith( '0 FILE ' ) ) {

// Save previous embedded file in the cache
this.subobjectCache[ currentEmbeddedFileName ] = currentEmbeddedText;
this.subobjectCache[ currentEmbeddedFileName.toLowerCase() ] = currentEmbeddedText;

// New embedded text file
currentEmbeddedFileName = line.substring( 7 );
Expand Down Expand Up @@ -1436,7 +1408,7 @@ THREE.LDrawLoader = ( function () {

if ( parsingEmbeddedFiles ) {

this.subobjectCache[ currentEmbeddedFileName ] = currentEmbeddedText;
this.subobjectCache[ currentEmbeddedFileName.toLowerCase() ] = currentEmbeddedText;

}

Expand Down