Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resizable spinnies #1079

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/skin/legacyskinparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,8 @@ QWidget* LegacySkinParser::parseSpinny(const QDomElement& node) {
return dummy;
}
commonWidgetSetup(node, spinny);
// Pixmaps are rendered from SVGs upon resizing, so initialize the pixmaps
spinny->resize(spinny->size());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can stall the skin parsing process since it triggers a bunch of QtGui code (like layout) before the skin is done being created. Instead, we could do this inside WSpinny with a helper method that generates the pixmaps, then call that in setup() and resizeEvent().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried calling it in WSpinny::setup(), but that didn't work either. IIRC WSpinny's initial size was not set up at that point, but I'll try again.


WaveformWidgetFactory::instance()->addTimerListener(spinny);
connect(spinny, SIGNAL(trackDropped(QString, QString)),
Expand Down
27 changes: 27 additions & 0 deletions src/skin/pixmapsource.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#include <QtDebug>
#include <QPainter>
#include <QSvgRenderer>

#include "skin/pixmapsource.h"

Expand Down Expand Up @@ -48,6 +50,31 @@ void PixmapSource::setSVG(const QByteArray& content) {
m_eType = SVG;
}

QPixmap PixmapSource::toPixmap(const QSize& size) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably belongs on Paintable in src/widget/wpixmapstore.h/cpp. PixmapSource is a description of a pixmap/svg, not a means to render it. All the rendering code is currently kept in Paintable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I first looked into using Paintable's draw methods inside WSpinny::paintEvent(), but yeah moving the toPixmap() function to Paintable and calling that from WSpinny::resizeEvent() seems like a better idea than the current state.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I think I understand how to use Paintable::draw() for this. Create a QPainter in WSpinny::resizeEvent() that paints onto the QPixmap members of WSpinny instead of onto the widget like other parts of the skin code.

QPixmap pixmap = QPixmap(size);
if (pixmap.isNull()) {
qWarning() << "PixmapSource::toPixmap size is null";
return pixmap;
}
pixmap.fill(Qt::transparent);

if (isSVG()) {
QSvgRenderer renderer;
if (!m_baData.isEmpty()) {
renderer.load(m_baData);
} else {
renderer.load(m_path);
}
QPainter painter(&pixmap);
renderer.render(&painter);
} else if (isBitmap()) {
pixmap = QPixmap(m_path).scaled(
size, Qt::KeepAspectRatio, Qt::SmoothTransformation);
}

return pixmap;
}

QString PixmapSource::getId() const {
quint16 checksum;
if (m_baData.isEmpty()) {
Expand Down
2 changes: 2 additions & 0 deletions src/skin/pixmapsource.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include <QString>
#include <QByteArray>
#include <QPixmap>

// A class representing an image source for a pixmap
// A bundle of a file path, raw data or inline svg
Expand All @@ -20,6 +21,7 @@ class PixmapSource {
QString getPath() const;
QByteArray getData() const;
QString getId() const;
QPixmap toPixmap(const QSize& size);

private:
enum Type {
Expand Down
99 changes: 48 additions & 51 deletions src/widget/wspinny.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ WSpinny::WSpinny(QWidget* parent, const QString& group,
WBaseWidget(this),
m_group(group),
m_pConfig(pConfig),
m_pBgImage(nullptr),
m_pMaskImage(nullptr),
m_pFgImage(nullptr),
m_pGhostImage(nullptr),
m_pPlay(nullptr),
m_pPlayPos(nullptr),
m_pVisualPlayPos(nullptr),
Expand Down Expand Up @@ -83,10 +79,6 @@ WSpinny::~WSpinny() {
#ifdef __VINYLCONTROL__
m_pVCManager->removeSignalQualityListener(this);
#endif
WImageStore::deleteImage(m_pBgImage);
WImageStore::deleteImage(m_pMaskImage);
WImageStore::deleteImage(m_pFgImage);
WImageStore::deleteImage(m_pGhostImage);
}

void WSpinny::onVinylSignalQualityUpdate(const VinylSignalQualityReport& report) {
Expand Down Expand Up @@ -125,38 +117,25 @@ void WSpinny::onVinylSignalQualityUpdate(const VinylSignalQualityReport& report)
}

void WSpinny::setup(const QDomNode& node, const SkinContext& context) {
// Set images
QDomElement backPathElement = context.selectElement(node, "PathBackground");
m_pBgImage = WImageStore::getImage(context.getPixmapSource(backPathElement));
Paintable::DrawMode bgmode = context.selectScaleMode(backPathElement,
Paintable::FIXED);
if (m_pBgImage && !m_pBgImage->isNull() && bgmode == Paintable::FIXED) {
setFixedSize(m_pBgImage->size());
} else {
setSizePolicy(QSizePolicy::MinimumExpanding, QSizePolicy::MinimumExpanding);
}
m_pMaskImage = WImageStore::getImage(context.getPixmapSource(
context.selectNode(node, "PathMask")));
m_pFgImage = WImageStore::getImage(context.getPixmapSource(
context.selectNode(node,"PathForeground")));
if (m_pFgImage && !m_pFgImage->isNull()) {
m_fgImageScaled = m_pFgImage->scaled(
size(), Qt::KeepAspectRatio, Qt::SmoothTransformation);
}
m_pGhostImage = WImageStore::getImage(context.getPixmapSource(
context.selectNode(node,"PathGhost")));
if (m_pGhostImage && !m_pGhostImage->isNull()) {
m_ghostImageScaled = m_pGhostImage->scaled(
size(), Qt::KeepAspectRatio, Qt::SmoothTransformation);
}
m_bgSource = context.getPixmapSource(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should keep the legacy behavior (bg scalemode is FIXED, then setFixedSize, otherwise set MinimumExpanding) otherwise this can break old skins.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How could we determine when to revert to legacy behavior?

context.selectElement(node, "PathBackground"));
m_maskSource = context.getPixmapSource(
context.selectNode(node, "PathMask"));
m_fgSource = context.getPixmapSource(
context.selectNode(node,"PathForeground"));
m_ghostSource = context.getPixmapSource(
context.selectNode(node,"PathGhost"));

m_bShowCover = context.selectBool(node, "ShowCover", false);

// TODO: set default size policy here?

#ifdef __VINYLCONTROL__
// Find the vinyl input we should listen to reports about.
if (m_pVCManager) {
m_iVinylInput = m_pVCManager->vinylInputFromGroup(m_group);
}
// TODO: scale?
m_iVinylScopeSize = MIXXX_VINYL_SCOPE_SIZE;
m_qImage = QImage(m_iVinylScopeSize, m_iVinylScopeSize, QImage::Format_ARGB32);
// fill with transparent black
Expand Down Expand Up @@ -289,8 +268,8 @@ void WSpinny::paintEvent(QPaintEvent *e) {
p.setRenderHint(QPainter::SmoothPixmapTransform);
p.drawPrimitive(QStyle::PE_Widget, option);

if (m_pBgImage) {
p.drawImage(rect(), *m_pBgImage, m_pBgImage->rect());
if (!m_bgPixmap.isNull()) {
p.drawPixmap(rect(), m_bgPixmap);
}

if (m_bShowCover && !m_loadedCoverScaled.isNull()) {
Expand All @@ -300,11 +279,12 @@ void WSpinny::paintEvent(QPaintEvent *e) {
p.drawPixmap(x, y, m_loadedCoverScaled);
}

if (m_pMaskImage) {
p.drawImage(rect(), *m_pMaskImage, m_pMaskImage->rect());
if (!m_maskPixmap.isNull()) {
p.drawPixmap(rect(), m_maskPixmap);
}

#ifdef __VINYLCONTROL__
// TODO: scale
// Overlay the signal quality drawing if vinyl is active
if (m_bVinylActive && m_bSignalActive) {
// draw the last good image
Expand All @@ -318,7 +298,7 @@ void WSpinny::paintEvent(QPaintEvent *e) {
// and draw the image at the corner.
p.translate(width() / 2, height() / 2);

bool paintGhost = m_bGhostPlayback && m_pGhostImage && !m_pGhostImage->isNull();
bool paintGhost = m_bGhostPlayback && !m_ghostSource.isEmpty();
if (paintGhost) {
p.save();
}
Expand All @@ -333,19 +313,19 @@ void WSpinny::paintEvent(QPaintEvent *e) {
m_dGhostAngleLastPlaypos = m_dGhostAngleCurrentPlaypos;
}

if (m_pFgImage && !m_pFgImage->isNull()) {
if (!m_fgPixmap.isNull()) {
// Now rotate the image and draw it on the screen.
p.rotate(m_fAngle);
p.drawImage(-(m_fgImageScaled.width() / 2),
-(m_fgImageScaled.height() / 2), m_fgImageScaled);
p.drawPixmap(-(m_fgPixmap.width() / 2),
-(m_fgPixmap.height() / 2), m_fgPixmap);
}

if (paintGhost) {
p.restore();
p.save();
p.rotate(m_fGhostAngle);
p.drawImage(-(m_ghostImageScaled.width() / 2),
-(m_ghostImageScaled.height() / 2), m_ghostImageScaled);
p.drawPixmap(-(m_ghostPixmap.width() / 2),
-(m_ghostPixmap.height() / 2), m_ghostPixmap);

//Rotate back to the playback position (not the ghost positon),
//and draw the beat marks from there.
Expand All @@ -360,16 +340,33 @@ QPixmap WSpinny::scaledCoverArt(const QPixmap& normal) {
return normal.scaled(size(), Qt::KeepAspectRatio, Qt::SmoothTransformation);
}

void WSpinny::resizeEvent(QResizeEvent* /*unused*/) {
m_loadedCoverScaled = scaledCoverArt(m_loadedCover);
if (m_pFgImage && !m_pFgImage->isNull()) {
m_fgImageScaled = m_pFgImage->scaled(
size(), Qt::KeepAspectRatio, Qt::SmoothTransformation);
}
if (m_pGhostImage && !m_pGhostImage->isNull()) {
m_ghostImageScaled = m_pGhostImage->scaled(
size(), Qt::KeepAspectRatio, Qt::SmoothTransformation);
void WSpinny::resizeEvent(QResizeEvent* re) {
qDebug() << "RESIZING SPINNY, QResizeEvent size: " << re->size()
<< "WSpinny size:" << size()
<< "old size:" << re->oldSize();
// Keep it square
if (height() < width()) {
resize(height(), height());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think even with this check it isn't safe. The parent widget's layout has the final say over our size, so it can modify the request in a way that would trigger an infinite loop.

updateGeometry();
return;
} else if (height() > width()) {
resize(width(), width());
updateGeometry();
return;
}

// TODO: if cover art is off or unavailable, set to deck color
// specify deck color in QSS
m_loadedCoverScaled = scaledCoverArt(m_loadedCover);

m_bgPixmap = m_bgSource.toPixmap(size());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to reload the pixmap from file / parse the SVG every time (which will block the GUI thread every time the spinny gets resized). Instead, let's keep a Paintable in the WSpinny for each of the originals and use Paintable::draw to render the pixmap at the desired size (that way it's all CPU + some mallocs).

m_fgPixmap = m_fgSource.toPixmap(size());
m_maskPixmap = m_maskSource.toPixmap(size());
m_ghostPixmap = m_ghostSource.toPixmap(size());
}

QSize WSpinny::sizeHint() const {
return QSize(height(), height());
}

/* Convert between a normalized playback position (0.0 - 1.0) and an angle
Expand Down
17 changes: 11 additions & 6 deletions src/widget/wspinny.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class WSpinny : public QGLWidget, public WBaseWidget, public VinylSignalQualityL
void showEvent(QShowEvent* event) override;
void hideEvent(QHideEvent* event) override;
bool event(QEvent* pEvent) override;
QSize sizeHint() const override;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May also want to define heightForWidth.
https://doc.qt.io/qt-4.8/qwidget.html#heightForWidth

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried. It didn't do anything.


double calculateAngle(double playpos);
int calculateFullRotations(double playpos);
Expand All @@ -69,12 +70,16 @@ class WSpinny : public QGLWidget, public WBaseWidget, public VinylSignalQualityL
private:
QString m_group;
UserSettingsPointer m_pConfig;
QImage* m_pBgImage;
QImage* m_pMaskImage;
QImage* m_pFgImage;
QImage m_fgImageScaled;
QImage* m_pGhostImage;
QImage m_ghostImageScaled;

QPixmap m_bgPixmap;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some history for why WSpinny uses QImage.

https://bugs.launchpad.net/mixxx/+bug/981210
a15c268
#440

@daschuer -- do you still have this netbook? I wonder if this bug in Qt 4.6.2 was fixed in a later version.

QPixmap m_fgPixmap;
QPixmap m_maskPixmap;
QPixmap m_ghostPixmap;
PixmapSource m_bgSource;
PixmapSource m_fgSource;
PixmapSource m_maskSource;
PixmapSource m_ghostSource;

ControlProxy* m_pPlay;
ControlProxy* m_pPlayPos;
QSharedPointer<VisualPlayPosition> m_pVisualPlayPos;
Expand Down