D27805: Add a screenshot capture item and use it to test rendering

[David Edmundson]

davidedmundson added a comment.


  I love the direction. +++++

INLINE COMMENTS

> ItemCapture.cpp:59
> +        qWarning() << "No previous image found for capture" << d->name;
> +        QVERIFY2(result->saveToFile(dir.absoluteFilePath(filePath)), "Unable to store first version of capture.");
> +        return;

I don't like how we're saving files into the user's source directory directly.

I'm sure we'll get lots of people forgetting to add the files and claiming it works because make test works the second time.

And we'll get other people committing lots of _new.png files committed accidentally.

----

I think a new temp dir would be best, alternatively we check some env variable/arg that a user can set to specify they want to generate a baseline

> ItemCapture.cpp:81
> +
> +    QVERIFY2(dir.remove(filePath), "Could not remove old capture.");
> +    QVERIFY2(dir.rename(newFilePath, filePath), "Could not rename new capture.");

Why are we replacing the image if it's slightly different?

> ItemCapture.h:61
> +     */
> +    Q_PROPERTY(qreal errorMargin READ errorMargin WRITE setErrorMargin NOTIFY errorMarginChanged)
> +

specify if 1% is

0.01 or 1 ?

> qmltest.cpp:22
> +
> +QUICK_TEST_MAIN_WITH_SETUP(Charts, Setup)
> +

I don't know what QUICK_TEXT_MAIN does, but we probably want to make sure we have:

QGuiApplication::setAttribute(AA_DisableHighDpiScaling, true);
QGuiApplication::setDesktopSettingsAware(false);

QQuickWindow::setTextRenderType(the qt rendering)

that should help generating the same thing across machines

REPOSITORY
  R1049 KQuickCharts

REVISION DETAIL
  https://phabricator.kde.org/D27805

To: ahiemstra
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200303/17659af7/attachment.html>