Skip to content

allshader RGB: Show a ghosted version of the original waveform behind… #12547

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
63 changes: 62 additions & 1 deletion src/waveform/renderers/allshader/waveformrendererrgb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ void WaveformRendererRGB::onSetup(const QDomNode& node) {
void WaveformRendererRGB::initializeGL() {
WaveformRendererSignalBase::initializeGL();
m_shader.init();
m_shader_ghost.init();
}

void WaveformRendererRGB::paintGL() {
Expand Down Expand Up @@ -94,6 +95,10 @@ void WaveformRendererRGB::paintGL() {
m_vertices.reserve(reserved);
m_colors.clear();
m_colors.reserve(reserved);
m_vertices_ghost.clear();
m_vertices_ghost.reserve(reserved);
m_colors_ghost.clear();
m_colors_ghost.reserve(reserved);

m_vertices.addRectangle(0.f,
halfBreadth - 0.5f * devicePixelRatio,
Expand Down Expand Up @@ -140,21 +145,29 @@ void WaveformRendererRGB::paintGL() {
float maxMid = static_cast<float>(u8maxMid);
float maxHigh = static_cast<float>(u8maxHigh);
float maxAllChn[2]{static_cast<float>(u8maxAllChn[0]), static_cast<float>(u8maxAllChn[1])};

float ghostLow = maxLow;
float ghostMid = maxMid;
float ghostHigh = maxHigh;
float maxAllChn_ghost[2]{static_cast<float>(u8maxAllChn[0]),
static_cast<float>(u8maxAllChn[1])};
// Uncomment to undo scaling with pow(value, 2.0f * 0.316f) done in analyzerwaveform.h
// float maxAllChn[2]{unscale(u8maxAllChn[0]), unscale(u8maxAllChn[1])};

// Calculate the squared magnitude of the maxLow, maxMid and maxHigh values.
// We take the square root to get the magnitude below.
const float sum = math_pow2(maxLow) + math_pow2(maxMid) + math_pow2(maxHigh);

// Apply the gains
// Apply the gains (non-ghosted version only)
maxLow *= lowGain;
maxMid *= midGain;
maxHigh *= highGain;

// Calculate the squared magnitude of the gained maxLow, maxMid and maxHigh values
// We take the square root to get the magnitude below.
const float sumGained = math_pow2(maxLow) + math_pow2(maxMid) + math_pow2(maxHigh);
const float sumGained_ghost = math_pow2(ghostLow) +
Copy link
Contributor

Choose a reason for hiding this comment

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

You are overcomplicating things. You don't need to sum the low+mid+high components because ... (see below)

math_pow2(ghostMid) + math_pow2(ghostHigh);

// The maxAll values will be used to draw the amplitude. We scale them according to
// magnitude of the gained maxLow, maxMid and maxHigh values
Expand All @@ -164,6 +177,10 @@ void WaveformRendererRGB::paintGL() {
const float factor = std::sqrt(sumGained / sum);
maxAllChn[0] *= factor;
maxAllChn[1] *= factor;

const float factor_ghost = std::sqrt(sumGained_ghost / sum);
Copy link
Contributor

Choose a reason for hiding this comment

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

... sumGained_ghost == sum, so factor_ghost is always 1.

maxAllChn_ghost[0] *= factor_ghost;
maxAllChn_ghost[1] *= factor_ghost;
}

// Use the gained maxLow, maxMid and maxHigh values to calculate the color components
Expand All @@ -185,13 +202,35 @@ void WaveformRendererRGB::paintGL() {
blue *= normFactor;
}

float red_ghost = ghostLow * low_r + ghostMid * mid_r + ghostHigh * high_r;
float green_ghost = ghostLow * low_g + ghostMid * mid_g + ghostHigh * high_g;
float blue_ghost = ghostLow * low_b + ghostMid * mid_b + ghostHigh * high_b;
const float maxComponent_ghost = math_max3(red_ghost, green_ghost, blue_ghost);
if (maxComponent_ghost == 0.f) {
// Avoid division by 0
red_ghost = 0.f;
green_ghost = 0.f;
blue_ghost = 0.f;
} else {
const float normFactor_ghost = 1.f / maxComponent_ghost;
red_ghost *= normFactor_ghost;
green_ghost *= normFactor_ghost;
blue_ghost *= normFactor_ghost;
}

// Lines are thin rectangles
m_vertices.addRectangle(fpos - 0.5f,
halfBreadth - heightFactor * maxAllChn[0],
fpos + 0.5f,
halfBreadth + heightFactor * maxAllChn[1]);
m_colors.addForRectangle(red, green, blue);

m_vertices_ghost.addRectangle(fpos - 0.5f,
halfBreadth - heightFactor * maxAllChn_ghost[0],
fpos + 0.5f,
halfBreadth + heightFactor * maxAllChn_ghost[1]);
m_colors_ghost.addForRectangle(red_ghost, green_ghost, blue_ghost, ghost_alpha);

Copy link
Contributor

Choose a reason for hiding this comment

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

Since the alpha doesn't change it would be better to use a uniform for that. It would be a simple to add the uniform for the alpha to the RGBShader, and that way you could use the same shader for normal and ghost.

xVisualFrame += visualIncrementPerPixel;
}

Expand All @@ -200,6 +239,28 @@ void WaveformRendererRGB::paintGL() {

const QMatrix4x4 matrix = matrixForWidgetGeometry(m_waveformRenderer, true);

// Draw the ghost version first so the solid version is on top.
const int matrixLocation_ghost = m_shader_ghost.matrixLocation();
const int positionLocation_ghost = m_shader_ghost.positionLocation();
const int colorLocation_ghost = m_shader_ghost.colorLocation();

m_shader_ghost.bind();
m_shader_ghost.enableAttributeArray(positionLocation_ghost);
m_shader_ghost.enableAttributeArray(colorLocation_ghost);

m_shader_ghost.setUniformValue(matrixLocation_ghost, matrix);

m_shader_ghost.setAttributeArray(
positionLocation_ghost, GL_FLOAT, m_vertices_ghost.constData(), 2);
m_shader_ghost.setAttributeArray(
colorLocation_ghost, GL_FLOAT, m_colors_ghost.constData(), 4);

glDrawArrays(GL_TRIANGLES, 0, m_vertices_ghost.size());

m_shader_ghost.disableAttributeArray(positionLocation_ghost);
m_shader_ghost.disableAttributeArray(colorLocation_ghost);
m_shader_ghost.release();

const int matrixLocation = m_shader.matrixLocation();
const int positionLocation = m_shader.positionLocation();
const int colorLocation = m_shader.colorLocation();
Expand Down
7 changes: 7 additions & 0 deletions src/waveform/renderers/allshader/waveformrendererrgb.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#pragma once

#include "shaders/rgbashader.h"
#include "shaders/rgbshader.h"
#include "util/class.h"
#include "waveform/renderers/allshader/rgbadata.h"
#include "waveform/renderers/allshader/rgbdata.h"
#include "waveform/renderers/allshader/vertexdata.h"
#include "waveform/renderers/allshader/waveformrenderersignalbase.h"
Expand All @@ -22,8 +24,13 @@ class allshader::WaveformRendererRGB final : public allshader::WaveformRendererS

private:
mixxx::RGBShader m_shader;
mixxx::RGBAShader m_shader_ghost;
VertexData m_vertices;
RGBData m_colors;
VertexData m_vertices_ghost;
RGBAData m_colors_ghost;

Copy link
Contributor

Choose a reason for hiding this comment

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

The convention is to use camel case, not snake case.

const float ghost_alpha = 0.25f;

DISALLOW_COPY_AND_ASSIGN(WaveformRendererRGB);
};