Skip to content

Commit

Permalink
Replace QTextDocumentResourceProvider with a std::function
Browse files Browse the repository at this point in the history
376e3bd added the new class for Qt 6.1,
but during header review we concluded that using a class introduces
complexity wrt instance ownership and API design that can be avoided by
using a std::function instead.

The functionality is tied to QTextDocument, so the type definition and
the default provider API is added there.

Since std::function is not trivially copyable, the atomicity of the
previous implementation is not maintained, and concurrent modifications
of and access to the global default provider from multiple threads is
not allowed. The relevant use case can be supported by implementing a
resource provider that is thread safe.

Task-number: QTBUG-90211
Fixes: QTBUG-92208
Change-Id: I39215c5e51c7bd27f1dd29e1d9d908aecf754fb7
Reviewed-by: Kai Koehne <[email protected]>
(cherry picked from commit ccf1a1a)
Reviewed-by: Qt Cherry-pick Bot <[email protected]>
  • Loading branch information
vohi authored and Qt Cherry-pick Bot committed Mar 30, 2021
1 parent 516316e commit 5e838e9
Show file tree
Hide file tree
Showing 12 changed files with 107 additions and 221 deletions.
1 change: 0 additions & 1 deletion src/gui/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ qt_internal_add_module(Gui
text/qtextobject.cpp text/qtextobject.h text/qtextobject_p.h
text/qtextoption.cpp text/qtextoption.h
text/qtexttable.cpp text/qtexttable.h text/qtexttable_p.h
text/qtextdocumentresourceprovider.cpp text/qtextdocumentresourceprovider.h
util/qabstractlayoutstyleinfo.cpp util/qabstractlayoutstyleinfo_p.h
util/qastchandler.cpp util/qastchandler_p.h
util/qdesktopservices.cpp util/qdesktopservices.h
Expand Down
62 changes: 52 additions & 10 deletions src/gui/text/qtextdocument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
#include "qtextdocumentfragment_p.h"
#include "qtexttable.h"
#include "qtextlist.h"
#include "qtextdocumentresourceprovider.h"
#include <qdebug.h>
#if QT_CONFIG(regularexpression)
#include <qregularexpression.h>
Expand Down Expand Up @@ -81,6 +80,9 @@ QT_BEGIN_NAMESPACE

Q_CORE_EXPORT Q_DECL_CONST_FUNCTION unsigned int qt_int_sqrt(unsigned int n);

namespace {
QTextDocument::ResourceProvider qt_defaultResourceProvider;
};

/*!
Returns \c true if the string \a text is likely to be rich text;
Expand Down Expand Up @@ -2080,7 +2082,11 @@ void QTextDocument::print(QPagedPaintDevice *printer) const
the resource. loadResource should then use addResource to add the
resource to the cache.
\sa QTextDocument::ResourceType
If loadResource does not load the resource, then the resourceProvider and
lastly the defaultResourceProvider will be called, if set. Note that the
result from the provider will not be added automatically to the cache.
\sa QTextDocument::ResourceType, resourceProvider()
*/
QVariant QTextDocument::resource(int type, const QUrl &name) const
{
Expand All @@ -2093,9 +2099,9 @@ QVariant QTextDocument::resource(int type, const QUrl &name) const
r = const_cast<QTextDocument *>(this)->loadResource(type, url);
if (!r.isValid()) {
if (d->resourceProvider)
r = d->resourceProvider->resource(url);
else if (auto defaultProvider = QTextDocumentResourceProvider::defaultProvider())
r = defaultProvider->resource(url);
r = d->resourceProvider(url);
else if (auto defaultProvider = defaultResourceProvider())
r = defaultProvider(url);
}
}
}
Expand Down Expand Up @@ -2131,26 +2137,62 @@ void QTextDocument::addResource(int type, const QUrl &name, const QVariant &reso
\since 6.1
Returns the resource provider for this text document.
\sa setResourceProvider(), defaultResourceProvider(), loadResource()
*/
QTextDocumentResourceProvider *QTextDocument::resourceProvider() const
QTextDocument::ResourceProvider QTextDocument::resourceProvider() const
{
Q_D(const QTextDocument);
return d->resourceProvider;
}

/*!
\since 6.1
\typealias QTextDocument::ResourceProvider
Type alias for std::function\<QVariant(const QUrl&)\>.
*/

/*!
\since 6.1
Sets the \a provider of resources for the text document.
Sets the provider of resources for the text document to \a provider.
\note The text document \e{does not} take ownership of the \a provider.
\sa resourceProvider(), loadResource()
*/
void QTextDocument::setResourceProvider(QTextDocumentResourceProvider *provider)
void QTextDocument::setResourceProvider(const ResourceProvider &provider)
{
Q_D(QTextDocument);
d->resourceProvider = provider;
}

/*!
\since 6.1
Sets the default resource provider to \a provider.
The default provider will be used by all QTextDocuments that don't have an
explicit provider set.
\sa setResourceProvider(), loadResource()
*/
void QTextDocument::setDefaultResourceProvider(const ResourceProvider &provider)
{
qt_defaultResourceProvider = provider;
}

/*!
\since 6.1
Returns the default resource provider.
\sa resourceProvider(), loadResource()
*/
QTextDocument::ResourceProvider QTextDocument::defaultResourceProvider()
{
return qt_defaultResourceProvider;
}

/*!
Loads data of the specified \a type from the resource with the
given \a name.
Expand All @@ -2168,7 +2210,7 @@ void QTextDocument::setResourceProvider(QTextDocumentResourceProvider *provider)
or a QTextDocument itself then the default implementation tries
to retrieve the data from the parent.
\sa QTextDocumentResourceProvider
\sa QTextDocument::ResourceProvider
*/
QVariant QTextDocument::loadResource(int type, const QUrl &name)
{
Expand Down
11 changes: 7 additions & 4 deletions src/gui/text/qtextdocument.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ class QVariant;
class QRectF;
class QTextOption;
class QTextCursor;
class QTextDocumentResourceProvider;


namespace Qt
{
Expand Down Expand Up @@ -240,8 +238,13 @@ class Q_GUI_EXPORT QTextDocument : public QObject
QVariant resource(int type, const QUrl &name) const;
void addResource(int type, const QUrl &name, const QVariant &resource);

QTextDocumentResourceProvider *resourceProvider() const;
void setResourceProvider(QTextDocumentResourceProvider *provider);
using ResourceProvider = std::function<QVariant(const QUrl&)>;

QTextDocument::ResourceProvider resourceProvider() const;
void setResourceProvider(const ResourceProvider &provider);

static QTextDocument::ResourceProvider defaultResourceProvider();
static void setDefaultResourceProvider(const ResourceProvider &provider);

QList<QTextFormat> allFormats() const;

Expand Down
1 change: 0 additions & 1 deletion src/gui/text/qtextdocument_p.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
#include "qtextdocumentlayout_p.h"
#include "qtexttable.h"
#include "qtextengine_p.h"
#include "qtextdocumentresourceprovider.h"

#include <stdlib.h>

Expand Down
2 changes: 1 addition & 1 deletion src/gui/text/qtextdocument_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ class Q_GUI_EXPORT QTextDocumentPrivate : public QObjectPrivate
QMap<int, QTextObject *> objects;
QMap<QUrl, QVariant> resources;
QMap<QUrl, QVariant> cachedResources;
QTextDocumentResourceProvider *resourceProvider;
QTextDocument::ResourceProvider resourceProvider;
QString defaultStyleSheet;

int lastBlockCount;
Expand Down
100 changes: 0 additions & 100 deletions src/gui/text/qtextdocumentresourceprovider.cpp

This file was deleted.

63 changes: 0 additions & 63 deletions src/gui/text/qtextdocumentresourceprovider.h

This file was deleted.

4 changes: 2 additions & 2 deletions src/widgets/widgets/qlabel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1429,7 +1429,7 @@ void QLabel::setTextFormat(Qt::TextFormat format)
Returns the resource provider for rich text of this label.
*/
QTextDocumentResourceProvider *QLabel::resourceProvider() const
QTextDocument::ResourceProvider QLabel::resourceProvider() const
{
Q_D(const QLabel);
return d->control ? d->control->document()->resourceProvider() : d->resourceProvider;
Expand All @@ -1442,7 +1442,7 @@ QTextDocumentResourceProvider *QLabel::resourceProvider() const
\note The label \e{does not} take ownership of the \a provider.
*/
void QLabel::setResourceProvider(QTextDocumentResourceProvider *provider)
void QLabel::setResourceProvider(const QTextDocument::ResourceProvider &provider)
{
Q_D(QLabel);
d->resourceProvider = provider;
Expand Down
6 changes: 3 additions & 3 deletions src/widgets/widgets/qlabel.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@
#include <QtWidgets/qtwidgetsglobal.h>
#include <QtWidgets/qframe.h>
#include <QtGui/qpicture.h>
#include <QtGui/qtextdocument.h>

QT_REQUIRE_CONFIG(label);

QT_BEGIN_NAMESPACE


class QTextDocumentResourceProvider;
class QLabelPrivate;

class Q_WIDGETS_EXPORT QLabel : public QFrame
Expand Down Expand Up @@ -93,8 +93,8 @@ class Q_WIDGETS_EXPORT QLabel : public QFrame
Qt::TextFormat textFormat() const;
void setTextFormat(Qt::TextFormat);

QTextDocumentResourceProvider *resourceProvider() const;
void setResourceProvider(QTextDocumentResourceProvider *provider);
QTextDocument::ResourceProvider resourceProvider() const;
void setResourceProvider(const QTextDocument::ResourceProvider &provider);

Qt::Alignment alignment() const;
void setAlignment(Qt::Alignment);
Expand Down
2 changes: 1 addition & 1 deletion src/widgets/widgets/qlabel_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class Q_AUTOTEST_EXPORT QLabelPrivate : public QFramePrivate
#endif
uint openExternalLinks : 1;
// <-- space for more bit field values here
QTextDocumentResourceProvider *resourceProvider;
QTextDocument::ResourceProvider resourceProvider;

friend class QMessageBoxPrivate;
};
Expand Down
Loading

0 comments on commit 5e838e9

Please sign in to comment.