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

Array buffer asset item #2442

Merged
merged 3 commits into from
Mar 2, 2017
Merged

Array buffer asset item #2442

merged 3 commits into from
Mar 2, 2017

Conversation

takahirox
Copy link
Collaborator

@takahirox takahirox commented Mar 1, 2017

Description:

This PR adds responseType attribute to a-asset-item.
It's necessary for THREE.GLTFLoader in r84. #2378
And it'll be helpful for user components which need arraybuffer data.

It has a limitation.
If we attempt to load the same data as different responseTypes,
only the responseType of the first attempt is applied
because it loads from cache if exists.

I think it won't cause any problems because
there's no situation that we wanna load the same data with different responseTypes.
(If exists, let me know.)

Changes proposed:

  • Add responseType attribute to a-asset-item.
  • Add responseType tests

@@ -98,6 +98,9 @@ registerElement('a-asset-item', {
value: function () {
var self = this;
var src = this.getAttribute('src');
var responseType = this.getAttribute('responseType');
if (responseType === null) responseType = 'text';
Copy link
Member

@ngokevin ngokevin Mar 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fileLoader.setResponseType(this.getAttribute('response-type') || 'text');

html attribute case insensitive

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, thanks! I'll update.

@ngokevin ngokevin merged commit 947fe2c into aframevr:master Mar 2, 2017
@ngokevin
Copy link
Member

ngokevin commented Mar 2, 2017

I've added docs and made the quick update. Thanks!

@takahirox takahirox deleted the ArrayBufferAssetItem branch March 3, 2017 00:01
@takahirox
Copy link
Collaborator Author

takahirox commented Mar 3, 2017

Thanks for the update!

I have a question.

You changed the order of

THREE.Cache.remove(XHR_SRC);
var assetItem = document.createElement('a-asset-item');

Is there any special reasons you needed to change?
And if so, we should change the order here, too?

THREE.Cache.remove(XHR_SRC);

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.

2 participants