Only enable shadowMap if an entity uses the shadow component and the …

…shadow system is enabled (#5399) Co-authored-by: Noeri Huisman <mrxz@users.noreply.github.com>
aframevr · Dec 1, 2023 · 78ca91e · 78ca91e
1 parent a8dbcb1
commit 78ca91e
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 9 deletions.
diff --git a/src/systems/shadow.js b/src/systems/shadow.js
@@ -26,16 +26,13 @@ module.exports.System = registerSystem('shadow', {
 
     this.shadowMapEnabled = false;
 
-    if (!sceneEl.renderer) { return; }  // For tests.
-
     sceneEl.renderer.shadowMap.type = SHADOW_MAP_TYPE_MAP[data.type];
     sceneEl.renderer.shadowMap.autoUpdate = data.autoUpdate;
-    this.setShadowMapEnabled(this.shadowMapEnabled);
   },
 
   update: function (prevData) {
     if (prevData.enabled !== this.data.enabled) {
-      this.setShadowMapEnabled(this.data.enabled);
+      this.setShadowMapEnabled(this.shadowMapEnabled);
     }
   },
 
@@ -44,10 +41,29 @@ module.exports.System = registerSystem('shadow', {
    * @param {boolean} enabled
    */
   setShadowMapEnabled: function (enabled) {
+    var sceneEl = this.sceneEl;
     var renderer = this.sceneEl.renderer;
-    this.shadowMapEnabled = this.data.enabled && enabled;
-    if (renderer) {
-      renderer.shadowMap.enabled = this.shadowMapEnabled;
+
+    this.shadowMapEnabled = enabled;
+    var newEnabledState = this.data.enabled && this.shadowMapEnabled;
+    if (renderer && newEnabledState !== renderer.shadowMap.enabled) {
+      renderer.shadowMap.enabled = newEnabledState;
+
+      // Materials must be updated for the change to take effect.
+      updateAllMaterials(sceneEl);
     }
   }
 });
+
+function updateAllMaterials (sceneEl) {
+  if (!sceneEl.hasLoaded) { return; }
+
+  sceneEl.object3D.traverse(function (node) {
+    if (node.material) {
+      var materials = Array.isArray(node.material) ? node.material : [node.material];
+      for (var i = 0; i < materials.length; i++) {
+        materials[i].needsUpdate = true;
+      }
+    }
+  });
+}
diff --git a/tests/systems/shadow.test.js b/tests/systems/shadow.test.js
@@ -7,12 +7,12 @@ suite('shadow system', function () {
   });
 
   suite('init', function () {
-    test('enabled by default', function (done) {
+    test('shadowMap disabled by default', function (done) {
       var el = this.el;
       this.el.addEventListener('loaded', function () {
         var sceneEl = el.sceneEl;
         var renderer = sceneEl.renderer;
-        assert.ok(renderer.shadowMap.enabled);
+        assert.notOk(renderer.shadowMap.enabled);
         done();
       });
     });
@@ -32,6 +32,43 @@ suite('shadow system', function () {
 
       document.body.appendChild(div);
     });
+
+    test('shadwMap automatically enables', function (done) {
+      var el = this.el;
+      el.setAttribute('shadow', 'receive: false');
+      el.addEventListener('loaded', function () {
+        var renderer = el.sceneEl.renderer;
+        assert.ok(renderer.shadowMap.enabled);
+        done();
+      });
+    });
+  });
+
+  suite('enabled', function () {
+    test('shadowMap remains disabled when system is explicitly disabled', function (done) {
+      var el = this.el;
+      el.setAttribute('shadow', 'receive: false');
+      el.sceneEl.setAttribute('shadow', 'enabled: false');
+      el.addEventListener('loaded', function () {
+        var renderer = el.sceneEl.renderer;
+        assert.notOk(renderer.shadowMap.enabled);
+        done();
+      });
+    });
+
+    test('shadowMap enables when system is (re)enabled', function (done) {
+      var el = this.el;
+      el.setAttribute('shadow', 'receive: false');
+      el.sceneEl.setAttribute('shadow', 'enabled: false');
+      el.addEventListener('loaded', function () {
+        var renderer = el.sceneEl.renderer;
+        assert.notOk(renderer.shadowMap.enabled);
+
+        el.sceneEl.setAttribute('shadow', 'enabled: true');
+        assert.ok(renderer.shadowMap.enabled);
+        done();
+      });
+    });
   });
 
   suite('setShadowMapEnabled', function () {