ShaderEffect: vertexShader and fragmentShader are now URLs

..and so fully compatible in behavior with properties like Image.source.

The main improvement here is that unlike with the old code, which incorrectly
handled local file URLs, one can now write fragmentShader: "myshader.frag.qsb"
which leads to picking up the file relative to the component (the .qml file),
as expected.

Existing code (that uses file: or qrc: URLs) will continue to work
as-is.

Change-Id: I89dd84b998b59ae6d5b6cb3d8a6353961ee8bb4a
Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>
Reviewed-by: Christian Strømme <christian.stromme@qt.io>

6 files changed, 60 insertions, 64 deletions

diff --git a/src/quick/items/qquickshadereffect.cpp b/src/quick/items/qquickshadereffect.cpp
index a19d881419..c8a43a84aa 100644
--- a/src/quick/items/qquickshadereffect.cpp
+++ b/src/quick/items/qquickshadereffect.cpp
@@ -45,6 +45,7 @@
 #include <private/qquickwindow_p.h>
 #include <private/qquickitem_p.h>
 #include "qquickshadereffectmesh_p.h"
+#include <QQmlAbstractUrlInterceptor>
 
 QT_BEGIN_NAMESPACE
 
@@ -330,11 +331,11 @@ public:
     QQuickShaderEffectImpl(QQuickShaderEffect *item);
     ~QQuickShaderEffectImpl();
 
-    QByteArray fragmentShader() const { return m_fragShader; }
-    void setFragmentShader(const QByteArray &src);
+    QUrl fragmentShader() const { return m_fragShader; }
+    void setFragmentShader(const QUrl &src);
 
-    QByteArray vertexShader() const { return m_vertShader; }
-    void setVertexShader(const QByteArray &src);
+    QUrl vertexShader() const { return m_vertShader; }
+    void setVertexShader(const QUrl &src);
 
     bool blending() const { return m_blending; }
     void setBlending(bool enable);
@@ -366,7 +367,7 @@ private slots:
     void markGeometryDirtyAndUpdate();
     void markGeometryDirtyAndUpdateIfSupportsAtlas();
     void shaderCodePrepared(bool ok, QSGGuiThreadShaderEffectManager::ShaderInfo::Type typeHint,
-                            const QByteArray &src, QSGGuiThreadShaderEffectManager::ShaderInfo *result);
+                            const QUrl &src, QSGGuiThreadShaderEffectManager::ShaderInfo *result);
 
 private:
     QSGGuiThreadShaderEffectManager *shaderEffectManager() const;
@@ -377,7 +378,7 @@ private:
 
         NShader
     };
-    bool updateShader(Shader shaderType, const QByteArray &src);
+    bool updateShader(Shader shaderType, const QUrl &src);
     void updateShaderVars(Shader shaderType);
     void disconnectSignals(Shader shaderType);
     void clearMappers(Shader shaderType);
@@ -392,9 +393,9 @@ private:
     bool m_blending;
     bool m_supportsAtlasTextures;
     mutable QSGGuiThreadShaderEffectManager *m_mgr;
-    QByteArray m_fragShader;
+    QUrl m_fragShader;
     bool m_fragNeedsUpdate;
-    QByteArray m_vertShader;
+    QUrl m_vertShader;
     bool m_vertNeedsUpdate;
 
     QSGShaderEffectNode::ShaderData m_shaders[NShader];
@@ -436,56 +437,47 @@ QQuickShaderEffect::~QQuickShaderEffect()
 }
 
 /*!
-    \qmlproperty string QtQuick::ShaderEffect::fragmentShader
+    \qmlproperty url QtQuick::ShaderEffect::fragmentShader
 
-    This property holds the fragment (pixel) shader's source code or a
-    reference to the pre-compiled bytecode. Some APIs, like OpenGL, always
-    support runtime compilation and therefore the traditional Qt Quick way of
-    inlining shader source strings is functional. Qt Quick backends for other
-    APIs may however limit support to pre-compiled bytecode like SPIR-V or D3D
-    shader bytecode. There the string is simply a filename, which may be a file
-    in the filesystem or bundled with the executable via Qt's resource system.
+    This property contains a reference to a file with the preprocessed fragment
+    shader package, typically with an extension of \c{.qsb}. The value is
+    treated as a \l{QUrl}{URL}, similarly to other QML types, such as Image. It
+    must either be a local file or use the qrc scheme to access files embedded
+    via the Qt resource system. The URL may be absolute, or relative to the URL
+    of the component.
 
-    With GLSL the default shader expects the texture coordinate to be passed
-    from the vertex shader as \c{varying highp vec2 qt_TexCoord0}, and it
-    samples from a sampler2D named \c source.
-
-    \sa vertexShader, GraphicsInfo
+    \sa vertexShader
 */
 
-QByteArray QQuickShaderEffect::fragmentShader() const
+QUrl QQuickShaderEffect::fragmentShader() const
 {
     return m_impl->fragmentShader();
 }
 
-void QQuickShaderEffect::setFragmentShader(const QByteArray &code)
+void QQuickShaderEffect::setFragmentShader(const QUrl &code)
 {
     m_impl->setFragmentShader(code);
 }
 
 /*!
-    \qmlproperty string QtQuick::ShaderEffect::vertexShader
-
-    This property holds the vertex shader's source code or a reference to the
-    pre-compiled bytecode. Some APIs, like OpenGL, always support runtime
-    compilation and therefore the traditional Qt Quick way of inlining shader
-    source strings is functional. Qt Quick backends for other APIs may however
-    limit support to pre-compiled bytecode like SPIR-V or D3D shader bytecode.
-    There the string is simply a filename, which may be a file in the
-    filesystem or bundled with the executable via Qt's resource system.
+    \qmlproperty url QtQuick::ShaderEffect::vertexShader
 
-    With GLSL the default shader passes the texture coordinate along to the
-    fragment shader as \c{varying highp vec2 qt_TexCoord0}.
+    This property contains a reference to a file with the preprocessed vertex
+    shader package, typically with an extension of \c{.qsb}. The value is
+    treated as a \l{QUrl}{URL}, similarly to other QML types, such as Image. It
+    must either be a local file or use the qrc scheme to access files embedded
+    via the Qt resource system. The URL may be absolute, or relative to the URL
+    of the component.
 
-    \sa fragmentShader, GraphicsInfo
+    \sa fragmentShader
 */
 
-QByteArray QQuickShaderEffect::vertexShader() const
+QUrl QQuickShaderEffect::vertexShader() const
 {
     return m_impl->vertexShader();
 }
 
-void QQuickShaderEffect::setVertexShader(const QByteArray &code)
+void QQuickShaderEffect::setVertexShader(const QUrl &code)
 {
     m_impl->setVertexShader(code);
 }
@@ -708,7 +700,7 @@ QQuickShaderEffectImpl::~QQuickShaderEffectImpl()
     delete m_mgr;
 }
 
-void QQuickShaderEffectImpl::setFragmentShader(const QByteArray &src)
+void QQuickShaderEffectImpl::setFragmentShader(const QUrl &src)
 {
     // Compare the actual values since they are often just filenames.
     // Optimizing by comparing constData() is a bad idea since seemingly static
@@ -726,7 +718,7 @@ void QQuickShaderEffectImpl::setFragmentShader(const QByteArray &src)
     emit m_item->fragmentShaderChanged();
 }
 
-void QQuickShaderEffectImpl::setVertexShader(const QByteArray &src)
+void QQuickShaderEffectImpl::setVertexShader(const QUrl &src)
 {
     if (m_vertShader == src)
         return;
@@ -1034,27 +1026,27 @@ static inline QVariant getValueFromProperty(QObject *item, const QMetaObject *it
 
 struct ShaderInfoCache
 {
-    bool contains(const QByteArray &key) const
+    bool contains(const QUrl &key) const
     {
         return m_shaderInfoCache.contains(key);
     }
 
-    QSGGuiThreadShaderEffectManager::ShaderInfo value(const QByteArray &key) const
+    QSGGuiThreadShaderEffectManager::ShaderInfo value(const QUrl &key) const
     {
         return m_shaderInfoCache.value(key);
     }
 
-    void insert(const QByteArray &key, const QSGGuiThreadShaderEffectManager::ShaderInfo &value)
+    void insert(const QUrl &key, const QSGGuiThreadShaderEffectManager::ShaderInfo &value)
     {
         m_shaderInfoCache.insert(key, value);
     }
 
-    QHash<QByteArray, QSGGuiThreadShaderEffectManager::ShaderInfo> m_shaderInfoCache;
+    QHash<QUrl, QSGGuiThreadShaderEffectManager::ShaderInfo> m_shaderInfoCache;
 };
 
 Q_GLOBAL_STATIC(ShaderInfoCache, shaderInfoCache)
 
-bool QQuickShaderEffectImpl::updateShader(Shader shaderType, const QByteArray &src)
+bool QQuickShaderEffectImpl::updateShader(Shader shaderType, const QUrl &src)
 {
     QSGGuiThreadShaderEffectManager *mgr = shaderEffectManager();
     if (!mgr)
@@ -1080,11 +1072,14 @@ bool QQuickShaderEffectImpl::updateShader(Shader shaderType, const QByteArray &s
             const QSGGuiThreadShaderEffectManager::ShaderInfo::Type typeHint =
                     shaderType == Vertex ? QSGGuiThreadShaderEffectManager::ShaderInfo::TypeVertex
                                          : QSGGuiThreadShaderEffectManager::ShaderInfo::TypeFragment;
+            QUrl loadUrl = src;
+            if (const QQmlEngine *engine = qmlEngine(m_item))
+                loadUrl = engine->interceptUrl(loadUrl, QQmlAbstractUrlInterceptor::UrlString);
             // Figure out what input parameters and variables are used in the
-            // shader. For file-based shader source/bytecode this is where the data
-            // is pulled in from the file. Some backends may choose to do
-            // source->bytecode compilation as well in this step.
-            mgr->prepareShaderCode(typeHint, src, m_inProgress[shaderType]);
+            // shader. This is where the data is pulled in from the file.
+            // (however, if there is compilation involved, that happens at a
+            // later stage, up to the QRhi backend)
+            mgr->prepareShaderCode(typeHint, loadUrl, m_inProgress[shaderType]);
             // the rest is handled in shaderCodePrepared()
             return true;
         }
@@ -1112,7 +1107,7 @@ bool QQuickShaderEffectImpl::updateShader(Shader shaderType, const QByteArray &s
 }
 
 void QQuickShaderEffectImpl::shaderCodePrepared(bool ok, QSGGuiThreadShaderEffectManager::ShaderInfo::Type typeHint,
-                                                   const QByteArray &src, QSGGuiThreadShaderEffectManager::ShaderInfo *result)
+                                                const QUrl &src, QSGGuiThreadShaderEffectManager::ShaderInfo *result)
 {
     const Shader shaderType = typeHint == QSGGuiThreadShaderEffectManager::ShaderInfo::TypeVertex ? Vertex : Fragment;
 
@@ -1128,7 +1123,8 @@ void QQuickShaderEffectImpl::shaderCodePrepared(bool ok, QSGGuiThreadShaderEffec
     m_inProgress[shaderType] = nullptr;
 
     if (!ok) {
-        qWarning("ShaderEffect: shader preparation failed for %s\n%s\n", src.constData(), qPrintable(log()));
+        qWarning("ShaderEffect: shader preparation failed for %s\n%s\n",
+                 qPrintable(src.toString()), qPrintable(log()));
         m_shaders[shaderType].hasShaderCode = false;
         return;
     }
diff --git a/src/quick/items/qquickshadereffect_p.h b/src/quick/items/qquickshadereffect_p.h
index b895353272..f0abeb8cb7 100644
--- a/src/quick/items/qquickshadereffect_p.h
+++ b/src/quick/items/qquickshadereffect_p.h
@@ -66,8 +66,8 @@ class QQuickShaderEffectPrivate;
 class Q_QUICK_PRIVATE_EXPORT QQuickShaderEffect : public QQuickItem
 {
     Q_OBJECT
-    Q_PROPERTY(QByteArray fragmentShader READ fragmentShader WRITE setFragmentShader NOTIFY fragmentShaderChanged)
-    Q_PROPERTY(QByteArray vertexShader READ vertexShader WRITE setVertexShader NOTIFY vertexShaderChanged)
+    Q_PROPERTY(QUrl fragmentShader READ fragmentShader WRITE setFragmentShader NOTIFY fragmentShaderChanged)
+    Q_PROPERTY(QUrl vertexShader READ vertexShader WRITE setVertexShader NOTIFY vertexShaderChanged)
     Q_PROPERTY(bool blending READ blending WRITE setBlending NOTIFY blendingChanged)
     Q_PROPERTY(QVariant mesh READ mesh WRITE setMesh NOTIFY meshChanged)
     Q_PROPERTY(CullMode cullMode READ cullMode WRITE setCullMode NOTIFY cullModeChanged)
@@ -95,11 +95,11 @@ public:
     QQuickShaderEffect(QQuickItem *parent = nullptr);
     ~QQuickShaderEffect() override;
 
-    QByteArray fragmentShader() const;
-    void setFragmentShader(const QByteArray &code);
+    QUrl fragmentShader() const;
+    void setFragmentShader(const QUrl &code);
 
-    QByteArray vertexShader() const;
-    void setVertexShader(const QByteArray &code);
+    QUrl vertexShader() const;
+    void setVertexShader(const QUrl &code);
 
     bool blending() const;
     void setBlending(bool enable);
diff --git a/src/quick/scenegraph/qsgadaptationlayer_p.h b/src/quick/scenegraph/qsgadaptationlayer_p.h
index 461a7013d5..512986556d 100644
--- a/src/quick/scenegraph/qsgadaptationlayer_p.h
+++ b/src/quick/scenegraph/qsgadaptationlayer_p.h
@@ -295,10 +295,10 @@ public:
         // QSGGeometry::defaultAttributes_TexturedPoint2D()).
     };
 
-    virtual void prepareShaderCode(ShaderInfo::Type typeHint, const QByteArray &src, ShaderInfo *result) = 0;
+    virtual void prepareShaderCode(ShaderInfo::Type typeHint, const QUrl &src, ShaderInfo *result) = 0;
 
 Q_SIGNALS:
-    void shaderCodePrepared(bool ok, ShaderInfo::Type typeHint, const QByteArray &src, ShaderInfo *result);
+    void shaderCodePrepared(bool ok, ShaderInfo::Type typeHint, const QUrl &src, ShaderInfo *result);
     void logAndStatusChanged();
 };
 
diff --git a/src/quick/scenegraph/qsgrhishadereffectnode.cpp b/src/quick/scenegraph/qsgrhishadereffectnode.cpp
index 8ba307bca6..5c756721f0 100644
--- a/src/quick/scenegraph/qsgrhishadereffectnode.cpp
+++ b/src/quick/scenegraph/qsgrhishadereffectnode.cpp
@@ -793,15 +793,14 @@ QSGGuiThreadShaderEffectManager::Status QSGRhiGuiThreadShaderEffectManager::stat
     return m_status;
 }
 
-void QSGRhiGuiThreadShaderEffectManager::prepareShaderCode(ShaderInfo::Type typeHint, const QByteArray &src, ShaderInfo *result)
+void QSGRhiGuiThreadShaderEffectManager::prepareShaderCode(ShaderInfo::Type typeHint, const QUrl &src, ShaderInfo *result)
 {
-    QUrl srcUrl(QString::fromUtf8(src));
-    if (!srcUrl.scheme().compare(QLatin1String("qrc"), Qt::CaseInsensitive) || srcUrl.isLocalFile()) {
+    if (!src.scheme().compare(QLatin1String("qrc"), Qt::CaseInsensitive) || src.isLocalFile()) {
         if (!m_fileSelector) {
             m_fileSelector = new QFileSelector(this);
             m_fileSelector->setExtraSelectors(QStringList() << QStringLiteral("qsb"));
         }
-        const QString fn = m_fileSelector->select(QQmlFile::urlToLocalFileOrQrc(srcUrl));
+        const QString fn = m_fileSelector->select(QQmlFile::urlToLocalFileOrQrc(src));
         QFile f(fn);
         if (!f.open(QIODevice::ReadOnly)) {
             qWarning("ShaderEffect: Failed to read %s", qPrintable(fn));
diff --git a/src/quick/scenegraph/qsgrhishadereffectnode_p.h b/src/quick/scenegraph/qsgrhishadereffectnode_p.h
index 8f717d4292..0bef0d571d 100644
--- a/src/quick/scenegraph/qsgrhishadereffectnode_p.h
+++ b/src/quick/scenegraph/qsgrhishadereffectnode_p.h
@@ -53,6 +53,7 @@
 
 #include <private/qsgadaptationlayer_p.h>
 #include <qsgmaterial.h>
+#include <QUrl>
 
 QT_BEGIN_NAMESPACE
 
@@ -151,7 +152,7 @@ public:
     bool hasSeparateSamplerAndTextureObjects() const override;
     QString log() const override;
     Status status() const override;
-    void prepareShaderCode(ShaderInfo::Type typeHint, const QByteArray &src, ShaderInfo *result) override;
+    void prepareShaderCode(ShaderInfo::Type typeHint, const QUrl &src, ShaderInfo *result) override;
 
 private:
     bool reflect(ShaderInfo *result);
diff --git a/tests/auto/quick/qquickshadereffect/tst_qquickshadereffect.cpp b/tests/auto/quick/qquickshadereffect/tst_qquickshadereffect.cpp
index 0a9b92375f..9bc4b20968 100644
--- a/tests/auto/quick/qquickshadereffect/tst_qquickshadereffect.cpp
+++ b/tests/auto/quick/qquickshadereffect/tst_qquickshadereffect.cpp
@@ -172,7 +172,7 @@ void tst_qquickshadereffect::withoutQmlEngine()
     // using a shader without QML engine used to crash
     auto window = new QQuickWindow;
     auto shaderEffect = new TestShaderEffect(window->contentItem());
-    shaderEffect->setVertexShader("");
+    shaderEffect->setVertexShader(QUrl());
     QVERIFY(shaderEffect->isComponentComplete());
     delete window;
 }