From 2f0f0abf828ea9b35ce3ceaa845303354ffcb09f Mon Sep 17 00:00:00 2001
From: Noeri Huisman <mrxz@users.noreply.github.com>
Date: Wed, 29 Nov 2023 17:11:16 +0100
Subject: [PATCH] Only enable shadowMap if an entity uses the shadow component
 and the shadow system is enabled

---
 src/systems/shadow.js        | 26 +++++++++++++++++------
 tests/systems/shadow.test.js | 41 ++++++++++++++++++++++++++++++++++--
 2 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/src/systems/shadow.js b/src/systems/shadow.js
index 4843901fab2..d709417edc8 100644
--- 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,25 @@ 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;
+
+      // If scene has already rendered, materials must be updated.
+      if (sceneEl.hasLoaded) {
+        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
index 77ef2d1bb08..d23685ef924 100644
--- 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 () {