From c3ba57620c2c40d285004af2e1809e581e6bb7e7 Mon Sep 17 00:00:00 2001 From: Eirik Aavitsland Date: Fri, 8 Nov 2019 14:32:51 +0100 Subject: Fix rendering of SVG with translating viewBox This is a modification of14fa4591eb34a35cf3d485fd901e3f1e2caa7770. That change correctly implemented support for scaling and centering viewBox. However, the viewBox scaling and x-min/y-min translation was applied in the wrong order, so SVGs with a non-zero x-min/y-min viewBox would be rendered with an offset. The autotest reflected this error, and is also fixed here. (In user space, the black rectangle's upper left corner (0,0) is in the center of the viewBox and so should end up in the center of the produced image, independently of the scaling. All other tested SVG renderers place it there.) This change fixes all the reported regression cases from the above commit, while not re-breaking the original cases fixed by that commit. Fixes: QTBUG-79333 Fixes: QTBUG-78200 Task-number: QTBUG-70256 Change-Id: I0d3394e3caf6ec7edf16a10992c2fbfdac7398e5 Reviewed-by: Allan Sandfeld Jensen --- src/svg/qsvgtinydocument.cpp | 8 ++++---- tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/svg/qsvgtinydocument.cpp b/src/svg/qsvgtinydocument.cpp index 3143ad2..7e35508 100644 --- a/src/svg/qsvgtinydocument.cpp +++ b/src/svg/qsvgtinydocument.cpp @@ -434,10 +434,6 @@ void QSvgTinyDocument::mapSourceToTarget(QPainter *p, const QRectF &targetRect, // but the entire document. This attempts to emulate the default values of the // tag that's implicitly defined when is used. - // Apply the view box translation if specified. - p->translate(target.x() - source.x(), - target.y() - source.y()); - // Scale the view box into the view port (target) by preserve the aspect ratio. QSizeF viewBoxSize = source.size(); viewBoxSize.scale(target.width(), target.height(), Qt::KeepAspectRatio); @@ -448,6 +444,10 @@ void QSvgTinyDocument::mapSourceToTarget(QPainter *p, const QRectF &targetRect, p->scale(viewBoxSize.width() / source.width(), viewBoxSize.height() / source.height()); + + // Apply the view box translation if specified. + p->translate(target.x() - source.x(), + target.y() - source.y()); } } } diff --git a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp index cf19213..eb3ecba 100644 --- a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp +++ b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp @@ -269,14 +269,13 @@ void tst_QSvgRenderer::testMapViewBoxToTarget() } { // Viewport and viewBox specified -> scale 500x500 square to 1000x750 while preserving aspect ratio gives 750x750 - // however the box is centered at 375, 250 data = ""; QPicture picture; QPainter painter(&picture); QSvgRenderer rend(data); rend.render(&painter); painter.end(); - QCOMPARE(picture.boundingRect(), QRect(375, 250, 750, 750)); + QCOMPARE(picture.boundingRect(), QRect(500, 375, 750, 750)); } } -- cgit v1.2.1 From 4d64f2c803b36313449a7b8a894e413c322a9b16 Mon Sep 17 00:00:00 2001 From: Robert Loehning Date: Tue, 12 Nov 2019 11:29:27 +0100 Subject: Fuzzing: Don't copy input data to QByteArray Change-Id: Ifb28491916bd30725bc460f7561a62b1933527c7 Reviewed-by: Edward Welbourne --- tests/libfuzzer/svg/qsvgrenderer/render/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/libfuzzer/svg/qsvgrenderer/render/main.cpp b/tests/libfuzzer/svg/qsvgrenderer/render/main.cpp index 0197115..2439e52 100644 --- a/tests/libfuzzer/svg/qsvgrenderer/render/main.cpp +++ b/tests/libfuzzer/svg/qsvgrenderer/render/main.cpp @@ -36,7 +36,7 @@ extern "C" int LLVMFuzzerTestOneInput(const char *Data, size_t Size) { static QApplication a(c, nullptr); static QImage image(377, 233, QImage::Format_RGB32); static QPainter painter(&image); - QSvgRenderer renderer(QByteArray(Data, Size)); + QSvgRenderer renderer(QByteArray::fromRawData(Data, Size)); renderer.render(&painter); return 0; } -- cgit v1.2.1 From beb0d74195ddc642524fe1f5a2d65d944527d1b5 Mon Sep 17 00:00:00 2001 From: Alessandro Portale Date: Wed, 13 Nov 2019 00:02:50 +0100 Subject: QSvgRenderer: Allow to remove a viewBox m_implicitViewBox needs to be true if the explicit viewBox is null. That relationship between m_viewBox and m_implicitViewBox can be see in QSvgTinyDocument::viewBox(). Task-number: QTBUG-79933 Change-Id: I6ea5a849479b10117128bcaf1799e3770b145e77 Reviewed-by: Simon Hausmann --- src/svg/qsvgtinydocument.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/svg/qsvgtinydocument.cpp b/src/svg/qsvgtinydocument.cpp index 7e35508..a2bf1c7 100644 --- a/src/svg/qsvgtinydocument.cpp +++ b/src/svg/qsvgtinydocument.cpp @@ -338,7 +338,7 @@ void QSvgTinyDocument::setHeight(int len, bool percent) void QSvgTinyDocument::setViewBox(const QRectF &rect) { m_viewBox = rect; - m_implicitViewBox = false; + m_implicitViewBox = rect.isNull(); } void QSvgTinyDocument::addSvgFont(QSvgFont *font) -- cgit v1.2.1 From e489a325769d295ee3b3948d98f5d07814dffd97 Mon Sep 17 00:00:00 2001 From: Alessandro Portale Date: Tue, 12 Nov 2019 22:10:34 +0100 Subject: Re-fix scaling when rendering a specific element by id This is a modification of 14fa4591eb34a35cf3d485fd901e3f1e2caa7770 That patch corrected the handling of the view box when rendering an entire SVG document. However, contrary to intention stated in the comment of that patch, it turns out that the new viewbox handling code path can be taken also for the case of rendering only a single element by id. Instead, we want to keep the original behavior where the element's origin bounds are transformed to fit into the user requested target bounds, allowing non-proportional scaling. Since the render-single-element case is easily and uniqely identifiable by the sourceRect parameter being non-null, just add an explicit check for that to the code path branching. Done-with: Eirik Aavitsland Fixes: QTBUG-79933 Change-Id: I64a35bbb193db22c33670b48ea165a91df8e719e Reviewed-by: Simon Hausmann --- src/svg/qsvgtinydocument.cpp | 4 +++- tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp | 36 ++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/svg/qsvgtinydocument.cpp b/src/svg/qsvgtinydocument.cpp index a2bf1c7..56960bf 100644 --- a/src/svg/qsvgtinydocument.cpp +++ b/src/svg/qsvgtinydocument.cpp @@ -420,7 +420,9 @@ void QSvgTinyDocument::mapSourceToTarget(QPainter *p, const QRectF &targetRect, source = viewBox(); if (source != target && !source.isNull()) { - if (m_implicitViewBox) { + if (m_implicitViewBox || !sourceRect.isNull()) { + // Code path used when no view box is set, or when an explicit source size is given which + // overrides it (which is the case when we're rendering only a specific element by id). QTransform transform; transform.scale(target.width() / source.width(), target.height() / source.height()); diff --git a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp index eb3ecba..309c646 100644 --- a/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp +++ b/tests/auto/qsvgrenderer/tst_qsvgrenderer.cpp @@ -59,6 +59,7 @@ private slots: void testStrokeWidth(); void testMapViewBoxToTarget(); void testRenderElement(); + void testRenderElementToBounds(); void constructorQXmlStreamReader() const; void loadQXmlStreamReader() const; void nestedQXmlStreamReader() const; @@ -336,6 +337,41 @@ void tst_QSvgRenderer::testRenderElement() } +void tst_QSvgRenderer::testRenderElementToBounds() +{ + // QTBUG-79933 + QImage reference(400, 200, QImage::Format_ARGB32); + { + reference.fill(Qt::transparent); + QPainter p(&reference); + p.fillRect( 0, 0, 200, 100, Qt::blue); + p.fillRect(200, 100, 200, 100, Qt::blue); + p.fillRect( 0, 0, 100, 50, Qt::red); + p.fillRect(100, 50, 100, 50, Qt::red); + p.fillRect(200, 100, 100, 50, Qt::red); + p.fillRect(300, 150, 100, 50, Qt::red); + } + + QImage rendering(400, 200, QImage::Format_ARGB32); + { + const char *const src = + "" // Presence of a viewBox triggered QTBUG-79933 + "" + "" + ""; + const QByteArray data(src); + QSvgRenderer rend(data); + rendering.fill(Qt::transparent); + QPainter p(&rendering); + rend.render(&p, "el1", QRectF( 0, 0, 200, 100)); + rend.render(&p, "el2", QRectF( 0, 0, 200, 100)); + rend.render(&p, "el1", QRectF(200, 100, 200, 100)); + rend.render(&p, "el2", QRectF(200, 100, 200, 100)); + } + + QCOMPARE(reference, rendering); +} + void tst_QSvgRenderer::constructorQXmlStreamReader() const { const QByteArray data(src); -- cgit v1.2.1