Skip to content
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.

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

Dev #6649

Merged
merged 3 commits into from Jun 5, 2015
Merged

Dev #6649

merged 3 commits into from Jun 5, 2015

Conversation

SirNeuman
Copy link
Contributor

So I the main thing I added was returning the XMLHttpRequest object from the XHRLoader. This gives other loaders that use the XHRLoader object (such as STLLoader) access to the XMLHttpRequest object. Therefore allowing manipulation of the XMLHttpRequest object and access to the built in functions of the object. Mainly XMLHttpRequest.abort() so when using STLLoader one can cancel the load by calling abort on the request object. Issues #6641 and #6634 are relevant here.

Here's an example of code for how the changes can be useful.

var loader = new THREE.STLLoader();
var request = {};

function loadMyObj(url){
    request = loader.load(url, function(object){
        scene.add(object);
    }
}

function cancelLoad(){
    request.abort();
}

I only added to the STLLoader in examples > js but this can be easily added to the any other loaders that use XHRLoader.

@mrdoob
Copy link
Owner

mrdoob commented Jun 5, 2015

I like it! Thanks ^^

mrdoob added a commit that referenced this pull request Jun 5, 2015
@mrdoob mrdoob merged commit 584eca6 into mrdoob:dev Jun 5, 2015
@SirNeuman SirNeuman deleted the dev branch June 5, 2015 17:59
@WestLangley
Copy link
Collaborator

  1. XHRLoader.load() returns either cached or request. Is that what you want?
  2. Also, in the same method, should this.response be scope.response?

@mrdoob
Copy link
Owner

mrdoob commented Jun 5, 2015

  1. XHRLoader.load() returns either cached or request. Is that what you want?

Good catch. I think return cached should be removed...

  1. Also, in the same method, should this.response be scope.response?

I think it's correct as it is...?

@SirNeuman
Copy link
Contributor Author

  1. Hmmm. I think we want it to return the request consistently when calling load(). However, I'm not too familiar with THREE.Cache, though THREE.Cache seems straightforward enough.
if ( cached !== undefined ) {
    if ( onLoad ) onLoad( cached );
    return;
}

I'm not too sure what purpose this serves. Either way, for the purpose of calling abort() at least, when calling the XHRLoader to start loading, it will(?) pass the return onLoad and eventually get to returning request. So for canceling the load (which was the motivation behind updating this function) this will work as expected.

  1. I didn't make any changes there, so I don't know. But it seems to me like it should use scope.response though i'm not sure what difference that would make.

@mrdoob
Copy link
Owner

mrdoob commented Jun 5, 2015

Actually, I guess this evolved from ImageLoader where returning cached makes sense:

https://github.com/mrdoob/three.js/blob/dev/src/loaders/ImageLoader.js#L24
https://github.com/mrdoob/three.js/blob/dev/src/loaders/ImageLoader.js#L66

@mrdoob mrdoob mentioned this pull request Jun 5, 2015
@WestLangley
Copy link
Collaborator

Also, in the same method, should this.response be scope.response?

I think it's correct as it is...?

@mrdoob You are right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants