need to check video element for both direct property and via getAttri…

…bute; fix shader substitution tests
aframevr · May 13, 2017 · ccd5e23 · ccd5e23
1 parent 8c43f5b
commit ccd5e23
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 16 deletions.
diff --git a/src/systems/material.js b/src/systems/material.js
@@ -146,7 +146,7 @@ module.exports.System = registerSystem('material', {
     setTextureProperties(texture, data);
 
     // if we're on iOS, and the video is HLS, we currently need to do some hacks
-    if (utils.device.isIOS() && isHLS(videoEl.getAttribute('src'), videoEl.getAttribute('type'))) {
+    if (utils.device.isIOS() && isHLS(videoEl.src || videoEl.getAttribute('src'), videoEl.type || videoEl.getAttribute('type'))) {
       // really it's BGRA, so this needs correction in shader
       texture.format = THREE.RGBAFormat;
       texture.needsCorrectionBGRA = true;

diff --git a/src/utils/material.js b/src/utils/material.js
@@ -121,16 +121,15 @@ function handleTextureEvents (el, texture) {
   // Video events.
   if (!texture.image || texture.image.tagName !== 'VIDEO') { return; }
 
-  // With Travis CI, the actual videos are never loaded,
-  // so for the iOS HLS shader adaptation to be testable,
-  // it needs to be done on materialtextureloaded not materialvideoloadeddata!
-
-  // Check to see if we need to use iOS 10 HLS shader.
-  if (texture.needsCorrectionBGRA && texture.needsCorrectionFlipY) {
-    el.setAttribute('material', 'shader', 'ios10hls');
-  }
-
   texture.image.addEventListener('loadeddata', function emitVideoTextureLoadedDataAll () {
+    // Check to see if we need to use iOS 10 HLS shader.
+    if (texture.needsCorrectionBGRA && texture.needsCorrectionFlipY) {
+      // Only override the shader if it is a stock (or test) shader that we know doesn't correct.
+      if (['standard', 'flat', 'testShader'].includes(el.components.material.data.shader)) {
+        el.setAttribute('material', 'shader', 'ios10hls');
+      }
+    }
+
     el.emit('materialvideoloadeddata', {src: texture.image, texture: texture});
   });
   texture.image.addEventListener('ended', function emitVideoTextureEndedAll () {

diff --git a/tests/core/shader.test.js b/tests/core/shader.test.js
@@ -165,20 +165,71 @@ suite('shader', function () {
       AFRAME.utils.device.isIOS = function () { return true; };
       assert.equal(AFRAME.utils.device.isIOS(), true);
 
+      // Set up and verify video element to be treated as HLS.
+      var videoEl = document.createElement('video');
+      videoEl.src = VIDEO;
+      videoEl.type = 'application/x-mpegurl';
+      assert.equal(AFRAME.utils.material.isHLS(videoEl.src, videoEl.type), true);
+
+      // With Travis CI, the actual videos are apparently never loaded fully,
+      // so checking shader instance in materialvideoloadeddata fails;
+      // however shader substition is still performed, so the test does not fail.
+      el.addEventListener('materialvideoloadeddata', function () {
+        var material = el.components.material;
+        if (!material) { return; }
+
+        // Verify system thought this was iOS HLS.
+        assert.equal(AFRAME.utils.device.isIOS(), true);
+        assert.equal(AFRAME.utils.material.isHLS(videoEl.src, videoEl.type), true);
+
+        // Wait for other handlers to fire.
+        setTimeout(function () {
+          // Undo mock of iOS.
+          AFRAME.utils.device.isIOS = realIsIOS;
+
+          // Verify shader was substituted.
+          assert.equal(material.data.shader, 'ios10hls');
+
+          done();
+        });
+      });
+      el.setAttribute('material', {shader: 'testShader', src: videoEl});
+      var material = el.components.material;
+      var instance = material.shader;
+      assert.ok(instance);
+      assert.ok(initSpy.calledOnce);
+      assert.ok(updateSpy.calledOnce);
+      // The value won't be assigned until the texture loads.
+      assert.ok(instance.uniforms['src']);
+      assert.notOk(instance.attributes && (instance.attributes['map'] ||
+                                           instance.attributes['src']));
+    });
+
+    test('iOS HLS video uses appropriate shader (setAttribute)', function (done) {
+      var shader = this.shader;
+      var el = this.el;
+      var initSpy = this.sinon.spy(shader.prototype, 'init');
+      var updateSpy = this.sinon.spy(shader.prototype, 'update');
+      assert.notOk(initSpy.called);
+      assert.notOk(updateSpy.called);
+
+      // Mock iOS.  NOTE: this doesn't work... el.sceneEl.isIOS = true;
+      var realIsIOS = AFRAME.utils.device.isIOS;
+      AFRAME.utils.device.isIOS = function () { return true; };
+      assert.equal(AFRAME.utils.device.isIOS(), true);
+
       // Set up and verify video element to be treated as HLS.
       var videoEl = document.createElement('video');
       videoEl.setAttribute('src', VIDEO);
       videoEl.setAttribute('type', 'application/x-mpegurl');
       assert.equal(AFRAME.utils.material.isHLS(videoEl.getAttribute('src'), videoEl.getAttribute('type')), true);
 
-      // With Travis CI, the actual videos are never loaded,
-      // so check for materialtextureloaded not materialvideoloadeddata,
-      // and don't try to assert the uniform values
-      el.addEventListener('materialtextureloaded', function () {
+      // With Travis CI, the actual videos are apparently never loaded fully,
+      // so checking shader instance in materialvideoloadeddata fails;
+      // however shader substition is still performed, so the test does not fail.
+      el.addEventListener('materialvideoloadeddata', function () {
         var material = el.components.material;
         if (!material) { return; }
-        var instance = material.shader;
-        assert.equal(instance.material['_texture_src'].image.getAttribute('src'), VIDEO);
 
         // Verify system thought this was iOS HLS.
         assert.equal(AFRAME.utils.device.isIOS(), true);