Skip to content

Commit

Permalink
engine: add rgbMapGetColors to RGBAlgorithm
Browse files Browse the repository at this point in the history 
In this way the UI can dinamycally load colors from scripts depending on presets (see updated plasma.js)

Reviewed inefficient code blocks.
On XML save "Color" tag + index attribute rather than changing tags (Color1, Color2, etc)
  • Loading branch information
@mcallegari
mcallegari committed Oct 24, 2024
1 parent 98c4a98 commit 6cd67bd
Show file tree
Hide file tree
Showing 17 changed files with 201 additions and 132 deletions.
3 changes: 3 additions & 0 deletions engine/src/rgbalgorithm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ class RGBAlgorithm
/** Set the colors for the RGBmap */
virtual void rgbMapSetColors(QVector<uint> &colors) = 0;

/** Get the colors from the RGB script */
virtual QVector<uint> rgbMapGetColors() = 0;

/** Load a RGBMap for the given step. */
virtual void rgbMap(const QSize& size, uint rgb, int step, RGBMap &map) = 0;

Expand Down
5 changes: 5 additions & 0 deletions engine/src/rgbaudio.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ void RGBAudio::rgbMapSetColors(QVector<uint> &colors)
Q_UNUSED(colors);
}

QVector<uint> RGBAudio::rgbMapGetColors()
{
return QVector<uint>();
}

void RGBAudio::rgbMap(const QSize& size, uint rgb, int step, RGBMap &map)
{
Q_UNUSED(step);
Expand Down
3 changes: 3 additions & 0 deletions engine/src/rgbaudio.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ protected slots:
/** @reimp */
void rgbMapSetColors(QVector<uint> &colors);

/** @reimp */
QVector<uint> rgbMapGetColors();

/** @reimp */
void rgbMap(const QSize& size, uint rgb, int step, RGBMap &map);

Expand Down
5 changes: 5 additions & 0 deletions engine/src/rgbimage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,11 @@ void RGBImage::rgbMapSetColors(QVector<uint> &colors)
Q_UNUSED(colors);
}

QVector<uint> RGBImage::rgbMapGetColors()
{
return QVector<uint>();
}

void RGBImage::rgbMap(const QSize& size, uint rgb, int step, RGBMap &map)
{
Q_UNUSED(rgb);
Expand Down
3 changes: 3 additions & 0 deletions engine/src/rgbimage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ class RGBImage : public RGBAlgorithm
/** @reimp */
void rgbMapSetColors(QVector<uint> &colors);

/** @reimp */
QVector<uint> rgbMapGetColors();

/** @reimp */
void rgbMap(const QSize& size, uint rgb, int step, RGBMap &map);

Expand Down
74 changes: 35 additions & 39 deletions engine/src/rgbmatrix.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@

#define KXMLQLCRGBMatrixStartColor QString("MonoColor")
#define KXMLQLCRGBMatrixEndColor QString("EndColor")
#define KXMLQLCRGBMatrixColorBase QString("Color")
#define KXMLQLCRGBMatrixColor QString("Color")
#define KXMLQLCRGBMatrixColorIndex QString("Index")

#define KXMLQLCRGBMatrixFixtureGroup QString("FixtureGroup")
#define KXMLQLCRGBMatrixDimmerControl QString("DimmerControl")
Expand Down Expand Up @@ -66,13 +67,6 @@ RGBMatrix::RGBMatrix(Doc* doc)
#if QT_VERSION < QT_VERSION_CHECK(5, 14, 0)
, m_algorithmMutex(QMutex::Recursive)
#endif
, m_rgbColors{
Qt::red,
QColor(),
QColor(),
QColor(),
QColor()
}
, m_stepHandler(new RGBMatrixStep())
, m_roundTime(new QElapsedTimer())
, m_stepsCount(0)
Expand All @@ -82,6 +76,9 @@ RGBMatrix::RGBMatrix(Doc* doc)
setName(tr("New RGB Matrix"));
setDuration(500);

m_rgbColors.fill(QColor(), RGBAlgorithmColorDisplayCount);
setColor(0, Qt::red);

RGBScript scr = doc->rgbScriptsCache()->script("Stripes");
setAlgorithm(scr.clone());
}
Expand Down Expand Up @@ -169,19 +166,14 @@ bool RGBMatrix::copyFrom(const Function* function)

setDimmerControl(mtx->dimmerControl());
setFixtureGroup(mtx->fixtureGroup());
foreach (QColor col, mtx->getColors())
m_rgbColors.append(col);

if (mtx->algorithm() != NULL)
setAlgorithm(mtx->algorithm()->clone());
else
setAlgorithm(NULL);

QVectorIterator<QColor> it(mtx->getColors());
uint count = 0;
while (it.hasNext()) {
QColor color = it.next();
setColor(count, color);
count ++;
}

setControlMode(mtx->controlMode());

return Function::copyFrom(function);
Expand Down Expand Up @@ -303,8 +295,12 @@ void RGBMatrix::previewMap(int step, RGBMatrixStep *handler)

void RGBMatrix::setColor(int i, QColor c)
{
if (i < 0)
return;

if (i >= m_rgbColors.count())
m_rgbColors.resize(i + 1);

m_rgbColors.replace(i, c);
{
QMutexLocker algorithmLocker(&m_algorithmMutex);
Expand All @@ -320,8 +316,9 @@ void RGBMatrix::setColor(int i, QColor c)

QColor RGBMatrix::getColor(int i) const
{
if (i >= m_rgbColors.count())
if (i < 0 || i >= m_rgbColors.count())
return QColor();

return m_rgbColors.at(i);
}

Expand All @@ -347,19 +344,17 @@ void RGBMatrix::setMapColors()
if (m_group == NULL)
m_group = doc()->fixtureGroup(fixtureGroup());

if (m_group != NULL) {
if (m_group != NULL)
{
QVector<unsigned int> rawColors;
int accColors = m_algorithm->acceptColors();
rawColors.resize(accColors);
QVectorIterator<QColor> it(m_rgbColors);
int count = 0;
while (it.hasNext() && count < accColors) {
QColor color = it.next();
rawColors.replace(count, color.isValid() ? color.rgb() : 0);
count ++;
};
for (int i = 0; i < m_algorithm->acceptColors(); i++)
{
QColor col = m_rgbColors.at(i);
rawColors.append(col.isValid() ? col.rgb() : 0);
}

m_algorithm->rgbMapSetColors(rawColors);
};
}
}

/************************************************************************
Expand All @@ -374,6 +369,10 @@ void RGBMatrix::setProperty(QString propName, QString value)
{
RGBScript *script = static_cast<RGBScript*> (m_algorithm);
script->setProperty(propName, value);

QVector<uint> colors = script->rgbMapGetColors();
for (int i = 0; i < colors.count(); i++)
setColor(i, QColor::fromRgb(colors.at(i)));
}
m_stepsCount = stepsCount();
}
Expand Down Expand Up @@ -446,11 +445,9 @@ bool RGBMatrix::loadXML(QXmlStreamReader &root)
{
setColor(1, QColor::fromRgb(QRgb(root.readElementText().toUInt())));
}
else if (root.name().startsWith(KXMLQLCRGBMatrixColorBase, Qt::CaseSensitive))
else if (root.name() == KXMLQLCRGBMatrixColor)
{
QString colorNumText = root.name().string()->right(
root.name().length() - KXMLQLCRGBMatrixColorBase.length());
uint colorIdx = colorNumText.toUInt() - 1;
int colorIdx = root.attributes().value(KXMLQLCRGBMatrixColorIndex).toInt();
setColor(colorIdx, QColor::fromRgb(QRgb(root.readElementText().toUInt())));
}
else if (root.name() == KXMLQLCRGBMatrixControlMode)
Expand Down Expand Up @@ -506,13 +503,12 @@ bool RGBMatrix::saveXML(QXmlStreamWriter *doc)
doc->writeTextElement(KXMLQLCRGBMatrixDimmerControl, QString::number(dimmerControl()));

/* Colors */
QVectorIterator<QColor> colorIt(m_rgbColors);
uint count = 0;
while (colorIt.hasNext()) {
QColor color = colorIt.next();
QString elementName = KXMLQLCRGBMatrixColorBase.append(QString::number(count + 1));
doc->writeTextElement(elementName, QString::number(color.rgb()));
count ++;
for (int i = 0; i < m_rgbColors.count(); i++)
{
doc->writeStartElement(KXMLQLCRGBMatrixColor);
doc->writeAttribute(KXMLQLCRGBMatrixColorIndex, QString::number(i));
doc->writeCharacters(QString::number(m_rgbColors.at(i).rgb()));
doc->writeEndElement();
}

/* Control Mode */
Expand Down
5 changes: 5 additions & 0 deletions engine/src/rgbplain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ void RGBPlain::rgbMapSetColors(QVector<uint> &colors)
Q_UNUSED(colors);
}

QVector<uint> RGBPlain::rgbMapGetColors()
{
return QVector<uint>();
}

void RGBPlain::rgbMap(const QSize& size, uint rgb, int step, RGBMap &map)
{
Q_UNUSED(step);
Expand Down
3 changes: 3 additions & 0 deletions engine/src/rgbplain.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ class RGBPlain : public QObject, public RGBAlgorithm
/** @reimp */
void rgbMapSetColors(QVector<uint> &colors);

/** @reimp */
QVector<uint> rgbMapGetColors();

/** @reimp */
void rgbMap(const QSize& size, uint rgb, int step, RGBMap &map);

Expand Down
28 changes: 28 additions & 0 deletions engine/src/rgbscript.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ bool RGBScript::evaluate()
qWarning() << m_fileName << "is missing the rgbMapSetColors() function!";
return false;
}

// retrieve the non-mandatory get color function
m_rgbMapGetColors = m_script.property("rgbMapGetColors");
if (m_rgbMapGetColors.isFunction() == false)
qWarning() << m_fileName << "is missing the rgbMapGetColors() function!";
}
if (m_apiVersion >= 2)
return loadProperties();
Expand Down Expand Up @@ -265,6 +270,7 @@ void RGBScript::rgbMapSetColors(QVector<uint> &colors)
QMutexLocker engineLocker(s_engineMutex);
if (m_apiVersion <= 2)
return;

if (m_rgbMapSetColors.isValid() == false)
return;

Expand All @@ -282,6 +288,28 @@ void RGBScript::rgbMapSetColors(QVector<uint> &colors)
displayError(value, m_fileName);
}

QVector<uint> RGBScript::rgbMapGetColors()
{
QMutexLocker engineLocker(s_engineMutex);
QVector<uint> colArray;

if (m_apiVersion <= 2)
return colArray;

if (m_rgbMapGetColors.isValid() == false)
return colArray;

QScriptValue colors = m_rgbMapGetColors.call();
if (colors.isValid() && colors.isArray())
{
QVariantList arr = colors.toVariant().toList();
foreach (QVariant color, arr)
colArray.append(color.toUInt());
}

return colArray;
}

void RGBScript::rgbMap(const QSize& size, uint rgb, int step, RGBMap &map)
{
QMutexLocker engineLocker(s_engineMutex);
Expand Down
4 changes: 4 additions & 0 deletions engine/src/rgbscript.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ class RGBScript : public RGBAlgorithm
/** @reimp */
void rgbMapSetColors(QVector<uint> &colors);

/** @reimp */
QVector<uint> rgbMapGetColors();

/** @reimp */
void rgbMap(const QSize& size, uint rgb, int step, RGBMap &map);

Expand Down Expand Up @@ -123,6 +126,7 @@ class RGBScript : public RGBAlgorithm
QScriptValue m_rgbMap; //! rgbMap() function
QScriptValue m_rgbMapStepCount; //! rgbMapStepCount() function
QScriptValue m_rgbMapSetColors; //! rgbMapSetColors() function
QScriptValue m_rgbMapGetColors; //! rgbMapSetColors() function

/************************************************************************
* Properties
Expand Down
19 changes: 19 additions & 0 deletions engine/src/rgbscriptv4.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,25 @@ void RGBScript::rgbMapSetColors(QVector<uint> &colors)
displayError(value, m_fileName);
}

QVector<uint> RGBScript::rgbMapGetColors()
{
QMutexLocker engineLocker(s_engineMutex);
QVector<uint> colArray;

if (m_rgbMap.isUndefined() == true)
return colArray;

QJSValue colors = m_rgbMapGetColors.call();
if (colors.isValid() && colors.isArray())
{
QVariantList arr = colors.toVariant().toList();
foreach (QVariant color, arr)
colArray.append(color.toUInt());
}

return colArray;
}

void RGBScript::rgbMap(const QSize& size, uint rgb, int step, RGBMap &map)
{
QMutexLocker engineLocker(s_engineMutex);
Expand Down
4 changes: 4 additions & 0 deletions engine/src/rgbscriptv4.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ class RGBScript : public RGBAlgorithm
/** @reimp */
void rgbMapSetColors(QVector<uint> &colors);

/** @reimp */
QVector<uint> rgbMapGetColors();

/** @reimp */
void rgbMap(const QSize& size, uint rgb, int step, RGBMap &map);

Expand Down Expand Up @@ -124,6 +127,7 @@ class RGBScript : public RGBAlgorithm
QJSValue m_rgbMap; //! rgbMap() function
QJSValue m_rgbMapStepCount; //! rgbMapStepCount() function
QJSValue m_rgbMapSetColors; //! rgbMapSetColors() function
QJSValue m_rgbMapGetColors; //! rgbMapSetColors() function

/************************************************************************
* Properties
Expand Down
5 changes: 5 additions & 0 deletions engine/src/rgbtext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,11 @@ void RGBText::rgbMapSetColors(QVector<uint> &colors)
Q_UNUSED(colors);
}

QVector<uint> RGBText::rgbMapGetColors()
{
return QVector<uint>();
}

void RGBText::rgbMap(const QSize& size, uint rgb, int step, RGBMap &map)
{
if (animationStyle() == StaticLetters)
Expand Down
3 changes: 3 additions & 0 deletions engine/src/rgbtext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ class RGBText : public RGBAlgorithm
/** @reimp */
void rgbMapSetColors(QVector<uint> &colors);

/** @reimp */
QVector<uint> rgbMapGetColors();

/** @reimp */
void rgbMap(const QSize& size, uint rgb, int step, RGBMap &map);

Expand Down
Loading

9 comments on commit 6cd67bd

@mcallegari
Copy link
Owner Author

Choose a reason for hiding this comment

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

@hjtappe please review this commit. I have optimized a few code blocks that were inefficient and redundant.
Also, please see the update plasma.js script. It's backward compatible and it also handles user defined colors by changing acceptColors upon preset change. Afterward, the UI will retrieve the color set from the script.
IMHO this gives further flexibility to color handling.
If you're OK with these changes, can you please help to review the other RGB scripts, dev tools, test units?
I think in this way we don't need "direct" versions of the scripts, right?

@hjtappe
Copy link
Contributor

@hjtappe hjtappe commented on 6cd67bd Oct 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Please also check if RGBAlgorithmColorDisplayCount could be removed. I had used that also as a marker towards the areas which require attention regarding the static 5 entries in UI/QMLUI. The one in ui/ might remain, but then, the #define should be moved there.

ui/src/rgbmatrixeditor.cpp:832:#if (5 != RGBAlgorithmColorDisplayCount)
engine/src/rgbalgorithm.h:43:#define RGBAlgorithmColorDisplayCount 5
engine/src/rgbmatrix.cpp:79: m_rgbColors.fill(QColor(), RGBAlgorithmColorDisplayCount);
qmlui/rgbmatrixeditor.cpp:137: Q_ASSERT(5 == RGBAlgorithmColorDisplayCount);

@hjtappe
Copy link
Contributor

Choose a reason for hiding this comment

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

While testing, I found an index overrun in ballscolors.js.
Would you just apply that or should I open a pull request?

--- a/resources/rgbscripts/ballscolors.js
+++ b/resources/rgbscripts/ballscolors.js
@@ -207,7 +207,7 @@ var testAlgo;
var yDirection = (Math.random() * 2) - 1; // and random directions
var xDirection = (Math.random() * 2) - 1;
algo.direction[i] = [yDirection, xDirection];
- algo.colour[i] = colorPalette.collection[algo.colorIndex[i]][1];
+ algo.colour[i] = colorPalette.collection[algo.colorIndex[i % algo.colorIndex.length]][1];
}
algo.initialized = true;
return;

@mcallegari
Copy link
Owner Author

Choose a reason for hiding this comment

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

Since the stagecolors branch is still a work in progress, please do open a new pull request with your findings.
I guess there will be many more changes to be applied there.
In this way we can comment on a proper PR rather than a past commit 😉
Thanks!

@hjtappe
Copy link
Contributor

Choose a reason for hiding this comment

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

The plasma script does not seem right in the devtools, the color transitions are not smooth. I'll take a deeper look next week.

@hjtappe
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I have covered the scripts, devscript and QLC functionality through review.
Let me know if I can assist with anything else.

@mcallegari
Copy link
Owner Author

@mcallegari mcallegari commented on 6cd67bd Oct 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@hjtappe thanks!
Will you send a new PR with some fixes for the stagecolor branch? Thank you

@hjtappe
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll create a PR.

  • Can I remove plasmacolorsdirect.js already in that PR?
  • Should that approach also be included in e.g. ballscolors.js?
  • What is your plan with alternate.js (include both or overwrite the relatively youg script)?
  • For the relatively young marquee.js script, should we overwrite this or also include both approaches?

@mcallegari
Copy link
Owner Author

Choose a reason for hiding this comment

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

Can I remove plasmacolorsdirect.js already in that PR?

Yes please

Should that approach also be included in e.g. ballscolors.js?

Yes, please make the basic "balls.js" script backward compatible with color support. Basically I would not add the "direct" version

What is your plan with alternate.js (include both or overwrite the relatively youg script)?
For the relatively young marquee.js script, should we overwrite this or also include both approaches?

To be honest I would overwrite the existing and even remove redundant scripts like plasmacolors.
I can add a warning in the next release notes to review a project if it uses those scripts.

As I mentioned before, less scripts, less confusion among users.
In any case users can use old scripts from previous QLC+ versions.

Please sign in to comment.