Merge pull request #7306 from zzmp/fix/px-rapid

Allow procedural shaders to be reused, and prevent crashes from rapid updates

libraries/entities-renderer/src/EntityTreeRenderer.cpp

@@ -341,12 +341,7 @@ void EntityTreeRenderer::applyZonePropertiesToScene(std::shared_ptr<ZoneEntityIt
             static QString userData;
             if (userData != zone->getUserData()) {
                 userData = zone->getUserData();
-                auto procedural = std::make_shared<Procedural>(userData);
-                if (procedural->_enabled) {
-                    skybox->setProcedural(procedural);
-                } else {
-                    skybox->setProcedural(ProceduralPointer());
-                }
+                skybox->parse(userData);
             }
             if (zone->getSkyboxProperties().getURL().isEmpty()) {
                 skybox->setCubemap(gpu::TexturePointer());

libraries/entities-renderer/src/RenderableBoxEntityItem.cpp

@@ -31,7 +31,9 @@ EntityItemPointer RenderableBoxEntityItem::factory(const EntityItemID& entityID,
 void RenderableBoxEntityItem::setUserData(const QString& value) {
     if (value != getUserData()) {
         BoxEntityItem::setUserData(value);
-        _procedural.reset();
+        if (_procedural) {
+            _procedural->parse(value);
+        }
     }
 }
 
@@ -40,7 +42,6 @@ void RenderableBoxEntityItem::render(RenderArgs* args) {
     Q_ASSERT(getType() == EntityTypes::Box);
     Q_ASSERT(args->_batch);
 
-
     if (!_procedural) {
         _procedural.reset(new Procedural(this->getUserData()));
         _procedural->_vertexSource = simple_vert;
@@ -62,7 +63,7 @@ void RenderableBoxEntityItem::render(RenderArgs* args) {
     }
 
     batch.setModelTransform(transToCenter); // we want to include the scale as well
-    if (_procedural && _procedural->ready()) {
+    if (_procedural->ready()) {
         _procedural->prepare(batch, getPosition(), getDimensions());
         auto color = _procedural->getColor(cubeColor);
         batch._glColor4f(color.r, color.g, color.b, color.a);

libraries/entities-renderer/src/RenderableSphereEntityItem.cpp

@@ -36,7 +36,9 @@ EntityItemPointer RenderableSphereEntityItem::factory(const EntityItemID& entity
 void RenderableSphereEntityItem::setUserData(const QString& value) {
     if (value != getUserData()) {
         SphereEntityItem::setUserData(value);
-        _procedural.reset();
+        if (_procedural) {
+            _procedural->parse(value);
+        }
     }
 }
 

libraries/procedural/src/procedural/Procedural.cpp

@@ -62,6 +62,9 @@ QJsonValue Procedural::getProceduralData(const QString& proceduralJson) {
     return doc.object()[PROCEDURAL_USER_DATA_KEY];
 }
 
+Procedural::Procedural() {
+    _state = std::make_shared<gpu::State>();
+}
 
 Procedural::Procedural(const QString& userDataJson) {
     parse(userDataJson);
@@ -69,74 +72,110 @@ Procedural::Procedural(const QString& userDataJson) {
 }
 
 void Procedural::parse(const QString& userDataJson) {
-    _enabled = false;
     auto proceduralData = getProceduralData(userDataJson);
-    if (proceduralData.isObject()) {
-        parse(proceduralData.toObject());
+    // Instead of parsing, prep for a parse on the rendering thread
+    // This will be called by Procedural::ready
+    std::lock_guard<std::mutex> lock(_proceduralDataMutex);
+    _proceduralData = proceduralData.toObject();
+    _proceduralDataDirty = true;
+}
+
+bool Procedural::parseVersion(const QJsonValue& version) {
+    if (version.isDouble()) {
+        _version = (uint8_t)(floor(version.toDouble()));
+    } else {
+        // All unversioned shaders default to V1
+        _version = 1;
     }
+    return (_version == 1 || _version == 2);
+}
+
+bool Procedural::parseUrl(const QUrl& shaderUrl) {
+    if (!shaderUrl.isValid()) {
+        qWarning() << "Invalid shader URL: " << shaderUrl;
+        return false;
+    }
+
+    if (_shaderUrl == shaderUrl) {
+        return true;
+    }
+
+    _shaderUrl = shaderUrl;
+
+    if (_shaderUrl.isLocalFile()) {
+        _shaderPath = _shaderUrl.toLocalFile();
+        qDebug() << "Shader path: " << _shaderPath;
+        if (!QFile(_shaderPath).exists()) {
+            return false;;
+        }
+    } else {
+        qDebug() << "Shader url: " << _shaderUrl;
+        _networkShader = ShaderCache::instance().getShader(_shaderUrl);
+    }
+
+    return true;
+}
+
+bool Procedural::parseUniforms(const QJsonObject& uniforms) {
+    if (_parsedUniforms != uniforms) {
+        _parsedUniforms = uniforms;
+        _uniformsDirty = true;
+    }
+
+    return true;
+}
+
+bool Procedural::parseTextures(const QJsonArray& channels) {
+    if (_parsedChannels != channels) {
+        _parsedChannels = channels;
+
+        auto textureCache = DependencyManager::get<TextureCache>();
+        size_t channelCount = std::min(MAX_PROCEDURAL_TEXTURE_CHANNELS, (size_t)_parsedChannels.size());
+        for (size_t i = 0; i < channelCount; ++i) {
+            QString url = _parsedChannels.at((int)i).toString();
+            _channels[i] = textureCache->getTexture(QUrl(url));
+        }
+
+        _channelsDirty = true;
+    }
+
+    return true;
 }
 
 void Procedural::parse(const QJsonObject& proceduralData) {
-    // grab the version number
-    {
-        auto version = proceduralData[VERSION_KEY];
-        if (version.isDouble()) {
-            _version = (uint8_t)(floor(version.toDouble()));
-        }
+    _enabled = false;
+
+    if (proceduralData.isEmpty()) {
+        return;
     }
 
-    // Get the path to the shader
-    {
-        QString shaderUrl = proceduralData[URL_KEY].toString();
-        shaderUrl = ResourceManager::normalizeURL(shaderUrl);
-        _shaderUrl = QUrl(shaderUrl);
-        if (!_shaderUrl.isValid()) {
-            qWarning() << "Invalid shader URL: " << shaderUrl;
-            return;
-        }
+    auto version = proceduralData[VERSION_KEY];
+    auto shaderUrl = proceduralData[URL_KEY].toString();
+    shaderUrl = ResourceManager::normalizeURL(shaderUrl);
+    auto uniforms = proceduralData[UNIFORMS_KEY].toObject();
+    auto channels = proceduralData[CHANNELS_KEY].toArray();
 
-        if (_shaderUrl.isLocalFile()) {
-            _shaderPath = _shaderUrl.toLocalFile();
-            qDebug() << "Shader path: " << _shaderPath;
-            if (!QFile(_shaderPath).exists()) {
-                return;
-            }
-        } else {
-            qDebug() << "Shader url: " << _shaderUrl;
-            _networkShader = ShaderCache::instance().getShader(_shaderUrl);
-        }
+    if (parseVersion(version) &&
+        parseUrl(shaderUrl) &&
+        parseUniforms(uniforms) &&
+        parseTextures(channels)) {
+        _enabled = true;
     }
-
-    // Grab any custom uniforms
-    {
-        auto uniforms = proceduralData[UNIFORMS_KEY];
-        if (uniforms.isObject()) {
-            _parsedUniforms = uniforms.toObject();
-        }
-    }
-
-    // Grab any textures
-    {
-        auto channels = proceduralData[CHANNELS_KEY];
-        if (channels.isArray()) {
-            auto textureCache = DependencyManager::get<TextureCache>();
-            _parsedChannels = channels.toArray();
-            size_t channelCount = std::min(MAX_PROCEDURAL_TEXTURE_CHANNELS, (size_t)_parsedChannels.size());
-            for (size_t i = 0; i < channelCount; ++i) {
-                QString url = _parsedChannels.at((int)i).toString();
-                _channels[i] = textureCache->getTexture(QUrl(url));
-            }
-        }
-    }
-    _enabled = true;
 }
 
 bool Procedural::ready() {
+    // Load any changes to the procedural
+    if (_proceduralDataDirty) {
+        std::lock_guard<std::mutex> lock(_proceduralDataMutex);
+        parse(_proceduralData);
+        _proceduralDataDirty = false;
+    }
+
     if (!_enabled) {
         return false;
     }
 
-    // Do we have a network or local shader
+    // Do we have a network or local shader, and if so, is it loaded?
     if (_shaderPath.isEmpty() && (!_networkShader || !_networkShader->isLoaded())) {
         return false;
     }
@@ -160,15 +199,14 @@ void Procedural::prepare(gpu::Batch& batch, const glm::vec3& position, const glm
             QFile file(_shaderPath);
             file.open(QIODevice::ReadOnly);
             _shaderSource = QTextStream(&file).readAll();
-            _pipelineDirty = true;
+            _shaderDirty = true;
             _shaderModified = lastModified;
         }
     } else if (_networkShader && _networkShader->isLoaded()) {
         _shaderSource = _networkShader->_source;
     }
 
-    if (!_pipeline || _pipelineDirty) {
-        _pipelineDirty = true;
+    if (!_pipeline || _shaderDirty) {
         if (!_vertexShader) {
             _vertexShader = gpu::Shader::createVertex(_vertexSource);
         }
@@ -214,11 +252,15 @@ void Procedural::prepare(gpu::Batch& batch, const glm::vec3& position, const glm
 
     batch.setPipeline(_pipeline);
 
-    if (_pipelineDirty) {
-        _pipelineDirty = false;
+    if (_shaderDirty || _uniformsDirty) {
         setupUniforms();
     }
 
+    if (_shaderDirty || _uniformsDirty || _channelsDirty) {
+        setupChannels(_shaderDirty || _uniformsDirty);
+    }
+
+    _shaderDirty = _uniformsDirty = _channelsDirty = false;
 
     for (auto lambda : _uniforms) {
         lambda(batch);
@@ -359,8 +401,14 @@ void Procedural::setupUniforms() {
             batch._glUniform(_standardUniformSlots[POSITION], _entityPosition);
         });
     }
+}
 
+void Procedural::setupChannels(bool shouldCreate) {
     if (gpu::Shader::INVALID_LOCATION != _standardUniformSlots[CHANNEL_RESOLUTION]) {
+        if (!shouldCreate) {
+            // Instead of modifying the last element, just remove and recreate it.
+            _uniforms.pop_back();
+        }
         _uniforms.push_back([=](gpu::Batch& batch) {
             vec3 channelSizes[MAX_PROCEDURAL_TEXTURE_CHANNELS];
             for (size_t i = 0; i < MAX_PROCEDURAL_TEXTURE_CHANNELS; ++i) {

libraries/procedural/src/procedural/Procedural.h

@@ -28,27 +28,24 @@ const size_t MAX_PROCEDURAL_TEXTURE_CHANNELS{ 4 };
 // FIXME better encapsulation
 // FIXME better mechanism for extending to things rendered using shaders other than simple.slv
 struct Procedural {
+public:
     static QJsonValue getProceduralData(const QString& proceduralJson);
 
+    Procedural();
     Procedural(const QString& userDataJson);
     void parse(const QString& userDataJson);
-    void parse(const QJsonObject&);
+
     bool ready();
     void prepare(gpu::Batch& batch, const glm::vec3& position, const glm::vec3& size);
-    void setupUniforms();
+
     glm::vec4 getColor(const glm::vec4& entityColor);
 
-    bool _enabled{ false };
-    uint8_t _version{ 1 };
+    uint8_t _version { 1 };
 
     std::string _vertexSource;
     std::string _fragmentSource;
 
-    QString _shaderSource;
-    QString _shaderPath;
-    QUrl _shaderUrl;
-    quint64 _shaderModified{ 0 };
-    bool _pipelineDirty{ true };
+    gpu::StatePointer _state;
 
     enum StandardUniforms {
         DATE,
@@ -60,23 +57,50 @@ struct Procedural {
         NUM_STANDARD_UNIFORMS
     };
 
-    int32_t _standardUniformSlots[NUM_STANDARD_UNIFORMS];
+protected:
+    // Procedural metadata
+    bool _enabled { false };
+    uint64_t _start { 0 };
+    int32_t _frameCount { 0 };
 
-    uint64_t _start{ 0 };
-    int32_t _frameCount{ 0 };
+    // Rendering object descriptions, from userData
+    QJsonObject _proceduralData;
+    std::mutex _proceduralDataMutex;
+    QString _shaderSource;
+    QString _shaderPath;
+    QUrl _shaderUrl;
+    quint64 _shaderModified { 0 };
     NetworkShaderPointer _networkShader;
     QJsonObject _parsedUniforms;
     QJsonArray _parsedChannels;
+    bool _proceduralDataDirty { true };
+    bool _shaderDirty { true };
+    bool _uniformsDirty { true };
+    bool _channelsDirty { true };
 
+    // Rendering objects
     UniformLambdas _uniforms;
+    int32_t _standardUniformSlots[NUM_STANDARD_UNIFORMS];
     NetworkTexturePointer _channels[MAX_PROCEDURAL_TEXTURE_CHANNELS];
     gpu::PipelinePointer _pipeline;
     gpu::ShaderPointer _vertexShader;
     gpu::ShaderPointer _fragmentShader;
     gpu::ShaderPointer _shader;
-    gpu::StatePointer _state;
+
+    // Entity metadata
     glm::vec3 _entityDimensions;
     glm::vec3 _entityPosition;
+
+private:
+    // This should only be called from the render thread, as it shares data with Procedural::prepare
+    void parse(const QJsonObject&);
+    bool parseVersion(const QJsonValue& version);
+    bool parseUrl(const QUrl& url);
+    bool parseUniforms(const QJsonObject& uniforms);
+    bool parseTextures(const QJsonArray& channels);
+
+    void setupUniforms();
+    void setupChannels(bool shouldCreate);
 };
 
 #endif

libraries/procedural/src/procedural/ProceduralSkybox.cpp

@@ -19,22 +19,10 @@
 #include "ProceduralSkybox_frag.h"
 
 ProceduralSkybox::ProceduralSkybox() : model::Skybox() {
-}
-
-ProceduralSkybox::ProceduralSkybox(const ProceduralSkybox& skybox) :
-    model::Skybox(skybox),
-    _procedural(skybox._procedural) {
-
-}
-
-void ProceduralSkybox::setProcedural(const ProceduralPointer& procedural) {
-    _procedural = procedural;
-    if (_procedural) {
-        _procedural->_vertexSource = ProceduralSkybox_vert;
-        _procedural->_fragmentSource = ProceduralSkybox_frag;
-        // Adjust the pipeline state for background using the stencil test
-        _procedural->_state->setStencilTest(true, 0xFF, gpu::State::StencilTest(0, 0xFF, gpu::EQUAL, gpu::State::STENCIL_OP_KEEP, gpu::State::STENCIL_OP_KEEP, gpu::State::STENCIL_OP_KEEP));
-    }
+    _procedural._vertexSource = ProceduralSkybox_vert;
+    _procedural._fragmentSource = ProceduralSkybox_frag;
+    // Adjust the pipeline state for background using the stencil test
+    _procedural._state->setStencilTest(true, 0xFF, gpu::State::StencilTest(0, 0xFF, gpu::EQUAL, gpu::State::STENCIL_OP_KEEP, gpu::State::STENCIL_OP_KEEP, gpu::State::STENCIL_OP_KEEP));
 }
 
 void ProceduralSkybox::render(gpu::Batch& batch, const ViewFrustum& frustum) const {
@@ -42,12 +30,10 @@ void ProceduralSkybox::render(gpu::Batch& batch, const ViewFrustum& frustum) con
 }
 
 void ProceduralSkybox::render(gpu::Batch& batch, const ViewFrustum& viewFrustum, const ProceduralSkybox& skybox) {
-    if (!(skybox._procedural)) {
+    if (!(skybox._procedural.ready())) {
         skybox.updateDataBuffer();
         Skybox::render(batch, viewFrustum, skybox);
-    }
-
-    if (skybox._procedural && skybox._procedural->_enabled && skybox._procedural->ready()) {
+    } else {
         gpu::TexturePointer skymap = skybox.getCubemap();
         // FIXME: skymap->isDefined may not be threadsafe
         assert(skymap && skymap->isDefined());
@@ -62,8 +48,7 @@ void ProceduralSkybox::render(gpu::Batch& batch, const ViewFrustum& viewFrustum,
         batch.setModelTransform(Transform()); // only for Mac
         batch.setResourceTexture(0, skybox.getCubemap());
 
-        skybox._procedural->prepare(batch, glm::vec3(0), glm::vec3(1));
+        skybox._procedural.prepare(batch, glm::vec3(0), glm::vec3(1));
         batch.draw(gpu::TRIANGLE_STRIP, 4);
     }
 }
-

libraries/procedural/src/procedural/ProceduralSkybox.h

@@ -17,22 +17,18 @@
 
 #include "Procedural.h"
 
-typedef std::shared_ptr<Procedural> ProceduralPointer;
-
 class ProceduralSkybox: public model::Skybox {
 public:
     ProceduralSkybox();
-    ProceduralSkybox(const ProceduralSkybox& skybox); 
-    ProceduralSkybox& operator= (const ProceduralSkybox& skybox);
     virtual ~ProceduralSkybox() {};
 
-    void setProcedural(const ProceduralPointer& procedural);
+    void parse(const QString& userData) { _procedural.parse(userData); }
 
     virtual void render(gpu::Batch& batch, const ViewFrustum& frustum) const;
     static void render(gpu::Batch& batch, const ViewFrustum& frustum, const ProceduralSkybox& skybox);
 
 protected:
-    ProceduralPointer _procedural;
+    mutable Procedural _procedural;
 };
 typedef std::shared_ptr< ProceduralSkybox > ProceduralSkyboxPointer;