Merge pull request #12666 from jherico/fix/web_overlay_crash

Fix invalid QML surface cache interaction

interface/src/ui/overlays/Web3DOverlay.cpp

@@ -63,6 +63,19 @@ static const float OPAQUE_ALPHA_THRESHOLD = 0.99f;
 
 const QString Web3DOverlay::TYPE = "web3d";
 const QString Web3DOverlay::QML = "Web3DOverlay.qml";
+
+static auto qmlSurfaceDeleter = [](OffscreenQmlSurface* surface) {
+    AbstractViewStateInterface::instance()->postLambdaEvent([surface] {
+        if (AbstractViewStateInterface::instance()->isAboutToQuit()) {
+            // WebEngineView may run other threads (wasapi), so they must be deleted for a clean shutdown
+            // if the application has already stopped its event loop, delete must be explicit
+            delete surface;
+        } else {
+            surface->deleteLater();
+        }
+    });
+};
+
 Web3DOverlay::Web3DOverlay() {
     _touchDevice.setCapabilities(QTouchDevice::Position);
     _touchDevice.setType(QTouchDevice::TouchScreen);
@@ -75,7 +88,8 @@ Web3DOverlay::Web3DOverlay() {
     connect(this, &Web3DOverlay::resizeWebSurface, this, &Web3DOverlay::onResizeWebSurface);
 
     //need to be intialized before Tablet 1st open
-    _webSurface = DependencyManager::get<OffscreenQmlSurfaceCache>()->acquire(_url);
+    _webSurface = DependencyManager::get<OffscreenQmlSurfaceCache>()->acquire(QML);
+    _cachedWebSurface = true;
     _webSurface->getSurfaceContext()->setContextProperty("HMD", DependencyManager::get<HMDScriptingInterface>().data());
     _webSurface->getSurfaceContext()->setContextProperty("Account", AccountServicesScriptingInterface::getInstance()); // DEPRECATED - TO BE REMOVED
     _webSurface->getSurfaceContext()->setContextProperty("GlobalServices", AccountServicesScriptingInterface::getInstance()); // DEPRECATED - TO BE REMOVED
@@ -114,6 +128,7 @@ void Web3DOverlay::destroyWebSurface() {
     if (!_webSurface) {
         return;
     }
+
     QQuickItem* rootItem = _webSurface->getRootItem();
 
     if (rootItem && rootItem->objectName() == "tabletRoot") {
@@ -135,10 +150,15 @@ void Web3DOverlay::destroyWebSurface() {
 
     QObject::disconnect(this, &Web3DOverlay::scriptEventReceived, _webSurface.data(), &OffscreenQmlSurface::emitScriptEvent);
     QObject::disconnect(_webSurface.data(), &OffscreenQmlSurface::webEventReceived, this, &Web3DOverlay::webEventReceived);
-    auto offscreenCache = DependencyManager::get<OffscreenQmlSurfaceCache>();
-    // FIXME prevents crash on shutdown, but we shoudln't have to do this check
-    if (offscreenCache) {
-        offscreenCache->release(QML, _webSurface);
+
+    // If the web surface was fetched out of the cache, release it back into the cache
+    if (_cachedWebSurface) {
+        auto offscreenCache = DependencyManager::get<OffscreenQmlSurfaceCache>();
+        // FIXME prevents crash on shutdown, but we shoudln't have to do this check
+        if (offscreenCache) {
+            offscreenCache->release(QML, _webSurface);
+        }
+        _cachedWebSurface = false;
     }
     _webSurface.reset();
 }
@@ -147,6 +167,8 @@ void Web3DOverlay::buildWebSurface() {
     if (_webSurface) {
         return;
     }
+    // FIXME the context save here is most likely unecessary since the QML surfaces now render
+    // off the main thread, and all GL context work is done off the main thread (I *think*)
     gl::withSavedContext([&] {
         // FIXME, the max FPS could be better managed by being dynamic (based on the number of current surfaces
         // and the current rendering load)
@@ -156,10 +178,13 @@ void Web3DOverlay::buildWebSurface() {
 
         if (isWebContent()) {
             _webSurface = DependencyManager::get<OffscreenQmlSurfaceCache>()->acquire(QML);
+            _cachedWebSurface = true;
             _webSurface->getRootItem()->setProperty("url", _url);
             _webSurface->getRootItem()->setProperty("scriptURL", _scriptURL);
         } else {
-            _webSurface = DependencyManager::get<OffscreenQmlSurfaceCache>()->acquire(_url);
+            _webSurface = QSharedPointer<OffscreenQmlSurface>(new OffscreenQmlSurface(), qmlSurfaceDeleter);
+            _webSurface->load(_url);
+            _cachedWebSurface = false;
             setupQmlSurface();
         }
         _webSurface->getSurfaceContext()->setContextProperty("globalPosition", vec3toVariant(getWorldPosition()));

interface/src/ui/overlays/Web3DOverlay.h

@@ -88,6 +88,7 @@ private:
 
     InputMode _inputMode { Touch };
     QSharedPointer<OffscreenQmlSurface> _webSurface;
+    bool _cachedWebSurface{ false };
     gpu::TexturePointer _texture;
     QString _url;
     QString _scriptURL;

scripts/developer/tests/webOverlayTool.js

@@ -0,0 +1,102 @@
+// webSpawnTool.js
+//
+// Stress tests the rendering of web surfaces over time
+//
+// Distributed under the Apache License, Version 2.0.
+// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+SPAWNER = function (properties) {
+    properties = properties || {};
+    var RADIUS = properties.radius || 5.0;   // Spawn within this radius (square)
+    var SPAWN_COUNT = properties.count || 10000; // number of entities to spawn
+    var SPAWN_LIMIT = properties.spawnLimit || 1;
+    var SPAWN_INTERVAL = properties.spawnInterval || properties.interval || 2;
+    var SPAWN_LIFETIME = properties.lifetime || 10;   // Entity timeout (when/if we crash, we need the entities to delete themselves)
+
+    function randomPositionXZ(center, radius) {
+        return {
+            x: center.x + (Math.random() * radius * 2.0) - radius,
+            y: center.y,
+            z: center.z + (Math.random() * radius * 2.0) - radius
+        };
+    }
+
+    function makeObject(properties) {
+        
+        var overlay = Overlays.addOverlay("web3d", {
+            name: "Web",
+            url: "https://www.reddit.com/r/random",
+            localPosition: randomPositionXZ( { x: 0, y: 0, z: -1 }, 1),
+            localRotation: Quat.angleAxis(180, Vec3.Y_AXIS),
+            dimensions: {x: .8, y: .45, z: 0.1},
+            color: { red: 255, green: 255, blue: 255 },
+            alpha: 1.0,
+            showKeyboardFocusHighlight: false,
+            visible: true
+        });
+        
+        var now = Date.now();
+
+        return {
+            destroy: function () {
+                Overlays.deleteOverlay(overlay)
+            },
+            getAge: function () {
+                return (Date.now() - now) / 1000.0;
+            }
+        };
+    }
+
+    
+    var items = [];
+    var toCreate = 0;
+    var spawned = 0;
+    var spawnTimer = 0.0;
+    var keepAliveTimer = 0.0;
+
+    function clear () {
+    }    
+    
+    function create() {
+        toCreate = SPAWN_COUNT;
+        Script.update.connect(spawn);
+    }
+
+    function spawn(dt) {
+        if (toCreate <= 0) {
+            Script.update.disconnect(spawn);
+            print("Finished spawning");
+        } 
+        else if ((spawnTimer -= dt) < 0.0){
+            spawnTimer = SPAWN_INTERVAL;
+
+            var n = Math.min(toCreate, SPAWN_LIMIT);
+            print("Spawning " + n + " items (" + (spawned += n) + ")");
+
+            toCreate -= n;
+            for (; n > 0; --n) {
+                items.push(makeObject());
+            }
+        }
+    }
+
+    function despawn() {
+        print("despawning");
+        items.forEach(function (item) {
+            item.destroy();
+        });
+        item = [];
+    }
+
+    function init () {
+        Script.update.disconnect(init);
+        Script.scriptEnding.connect(despawn);
+        clear();
+        create();
+    }
+    
+    Script.update.connect(init);
+};
+
+SPAWNER();