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.

Sign up for GitHub

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

make scaling of GLSL RGB and RGB L/R waveform amplitudes consistent with simple waveform #12205

Merged
merged 4 commits into from
Nov 25, 2023
Merged

make scaling of GLSL RGB and RGB L/R waveform amplitudes consistent with simple waveform #12205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d26931f
scale amplitude drawn with ratio between the low+mid+high magnitude w…
m0dB Oct 21, 2023
f036b0a
removed unused variable
m0dB Oct 22, 2023
52a0956
optimization: use 1 sqrt instead of two (thanks daschuer for the sugg…
m0dB Nov 24, 2023
20bbf26
avoid confusion between visual waveform samples and frames by better …
m0dB Nov 25, 2023
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
134 changes: 65 additions & 69 deletions src/waveform/renderers/allshader/waveformrendererlrrgb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,15 @@ void WaveformRendererLRRGB::paintGL() {
const float devicePixelRatio = m_waveformRenderer->getDevicePixelRatio();
const int length = static_cast<int>(m_waveformRenderer->getLength() * devicePixelRatio);

// Not multiplying with devicePixelRatio will also work. In that case, on
// High-DPI-Display the lines will be devicePixelRatio pixels wide (which is
// also what is used for the beat grid and the markers), or in other words
// each block of samples is represented by devicePixelRatio pixels (width).
const int visualFramesSize = dataSize / 2;
const double firstVisualFrame =
m_waveformRenderer->getFirstDisplayedPosition() * visualFramesSize;
const double lastVisualFrame =
m_waveformRenderer->getLastDisplayedPosition() * visualFramesSize;

const double firstVisualIndex = m_waveformRenderer->getFirstDisplayedPosition() * dataSize;
const double lastVisualIndex = m_waveformRenderer->getLastDisplayedPosition() * dataSize;

// Represents the # of waveform data points per horizontal pixel.
// Represents the # of visual frames per horizontal pixel.
const double visualIncrementPerPixel =
(lastVisualIndex - firstVisualIndex) / static_cast<double>(length);
(lastVisualFrame - firstVisualFrame) / static_cast<double>(length);

// Per-band gain from the EQ knobs.
float allGain(1.0), lowGain(1.0), midGain(1.0), highGain(1.0);
Expand All @@ -74,7 +72,7 @@ void WaveformRendererLRRGB::paintGL() {
const float breadth = static_cast<float>(m_waveformRenderer->getBreadth()) * devicePixelRatio;
const float halfBreadth = breadth / 2.0f;

const float heightFactorAbs = allGain * halfBreadth / std::sqrt(3.f * 256.f * 256.f);
const float heightFactorAbs = allGain * halfBreadth / 255.f;
const float heightFactor[2] = {-heightFactorAbs, heightFactorAbs};

const float low_r = static_cast<float>(m_rgbLowColor_r);
Expand All @@ -87,18 +85,17 @@ void WaveformRendererLRRGB::paintGL() {
const float mid_b = static_cast<float>(m_rgbMidColor_b);
const float high_b = static_cast<float>(m_rgbHighColor_b);

// Effective visual index of x
double xVisualSampleIndex = firstVisualIndex;
// Effective visual frame for x
double xVisualFrame = firstVisualFrame;

const int numVerticesPerLine = 6; // 2 triangles

const int linesReserved = numVerticesPerLine * (length * 2 + 1);
const int colorsReserved = numVerticesPerLine * (length * 2 + 1);
const int reserved = numVerticesPerLine * (length * 2 + 1);

m_vertices.clear();
m_vertices.reserve(linesReserved);
m_vertices.reserve(reserved);
m_colors.clear();
m_colors.reserve(colorsReserved);
m_colors.reserve(reserved);

m_vertices.addRectangle(0.f,
halfBreadth - 0.5f * devicePixelRatio,
Expand All @@ -109,79 +106,78 @@ void WaveformRendererLRRGB::paintGL() {
static_cast<float>(m_axesColor_g),
static_cast<float>(m_axesColor_b));

double maxSamplingRange = visualIncrementPerPixel / 2.0;

for (int pos = 0; pos < length; ++pos) {
// Our current pixel (x) corresponds to a number of visual samples
// (visualSamplerPerPixel) in our waveform object. We take the max of
// all the data points on either side of xVisualSampleIndex within a
// window of 'maxSamplingRange' visual samples to measure the maximum
// data point contained by this pixel.
double maxSamplingRange = visualIncrementPerPixel / 2.0;

// Since xVisualSampleIndex is in visual-samples (e.g. R,L,R,L) we want
// to check +/- maxSamplingRange frames, not samples. To do this, divide
// xVisualSampleIndex by 2. Since frames indices are integers, we round
// to the nearest integer by adding 0.5 before casting to int.
int visualFrameStart = int(xVisualSampleIndex / 2.0 - maxSamplingRange + 0.5);
int visualFrameStop = int(xVisualSampleIndex / 2.0 + maxSamplingRange + 0.5);
const int lastVisualFrame = dataSize / 2 - 1;

// We now know that some subset of [visualFrameStart, visualFrameStop]
// lies within the valid range of visual frames. Clamp
// visualFrameStart/Stop to within [0, lastVisualFrame].
visualFrameStart = math_clamp(visualFrameStart, 0, lastVisualFrame);
visualFrameStop = math_clamp(visualFrameStop, 0, lastVisualFrame);

int visualIndexStart = visualFrameStart * 2;
int visualIndexStop = visualFrameStop * 2;

visualIndexStart = std::max(visualIndexStart, 0);
visualIndexStop = std::min(visualIndexStop, dataSize);
const int visualFrameStart = int(xVisualFrame - maxSamplingRange + 0.5);
const int visualFrameStop = int(xVisualFrame + maxSamplingRange + 0.5);

const int visualIndexStart = math_clamp(visualFrameStart * 2, 0, dataSize - 1);
const int visualIndexStop =
math_clamp(std::max(visualFrameStart + 1, visualFrameStop) * 2,
0,
dataSize - 1);

const float fpos = static_cast<float>(pos);

for (int chn = 0; chn < 2; chn++) {
float maxLow{};
float maxMid{};
float maxHigh{};
float maxAll{};

// Find the max values for low, mid, high and all in the waveform data
uchar u8maxLow{};
uchar u8maxMid{};
uchar u8maxHigh{};
uchar u8maxAll{};
// data is interleaved left / right
for (int i = visualIndexStart + chn; i < visualIndexStop + chn; i += 2) {
const WaveformData& waveformData = data[i];

const float filteredLow = static_cast<float>(waveformData.filtered.low);
const float filteredMid = static_cast<float>(waveformData.filtered.mid);
const float filteredHigh = static_cast<float>(waveformData.filtered.high);
u8maxLow = math_max(u8maxLow, waveformData.filtered.low);
u8maxMid = math_max(u8maxMid, waveformData.filtered.mid);
u8maxHigh = math_max(u8maxHigh, waveformData.filtered.high);
u8maxAll = math_max(u8maxAll, waveformData.filtered.all);
}

maxLow = math_max(maxLow, filteredLow);
maxMid = math_max(maxMid, filteredMid);
maxHigh = math_max(maxHigh, filteredHigh);
// Cast to float
float maxLow = static_cast<float>(u8maxLow);
Copy link
Member

Choose a reason for hiding this comment

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

Not sure I understand why maxLow is a different type than u8maxLow. are we casting from uchar to float because of math max?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this way we cast only once, rather than casting for every iteration in the for loop in line 148.

float maxMid = static_cast<float>(u8maxMid);
float maxHigh = static_cast<float>(u8maxHigh);
float maxAll = static_cast<float>(u8maxAll);

const float all = math_pow2(filteredLow) * lowGain +
math_pow2(filteredMid) * midGain +
math_pow2(filteredHigh) * highGain;
maxAll = math_max(maxAll, all);
}
// 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
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);

// The maxAll value will be used to draw the amplitude. We scale them according to
// magnitude of the gained maxLow, maxMid and maxHigh values
Comment on lines +158 to +159
Copy link
Member

Choose a reason for hiding this comment

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

A bit more background info is useful here.
because ....

if (sum != 0.f) {
// magnitude = sqrt(sum) and magnitudeGained = sqrt(sumGained), and
// factor = magnitudeGained / magnitude, but we can do with a single sqrt:
const float factor = std::sqrt(sumGained / sum);
maxAll *= factor;
}

// Use the gained maxLow, maxMid and maxHigh values to calculate the color components
float red = maxLow * low_r + maxMid * mid_r + maxHigh * high_r;
float green = maxLow * low_g + maxMid * mid_g + maxHigh * high_g;
float blue = maxLow * low_b + maxMid * mid_b + maxHigh * high_b;

const float max = math_max3(red, green, blue);

// Normalize red, green, blue, using the maximum of the three

if (max == 0.f) {
// avoid division by 0
// Normalize the color components using the maximum of the three
const float maxComponent = math_max3(red, green, blue);
if (maxComponent == 0.f) {
// Avoid division by 0
red = 0.f;
green = 0.f;
blue = 0.f;
} else {
const float normFactor = 1.f / max;
const float normFactor = 1.f / maxComponent;
red *= normFactor;
green *= normFactor;
blue *= normFactor;
Expand All @@ -193,15 +189,15 @@ void WaveformRendererLRRGB::paintGL() {
m_vertices.addRectangle(fpos - 0.5f,
halfBreadth,
fpos + 0.5f,
halfBreadth + heightFactor[chn] * std::sqrt(maxAll));
halfBreadth + heightFactor[chn] * maxAll);
m_colors.addForRectangle(red, green, blue);
}

xVisualSampleIndex += visualIncrementPerPixel;
xVisualFrame += visualIncrementPerPixel;
}

DEBUG_ASSERT(linesReserved == m_vertices.size());
DEBUG_ASSERT(colorsReserved == m_colors.size());
DEBUG_ASSERT(reserved == m_vertices.size());
DEBUG_ASSERT(reserved == m_colors.size());
Copy link
Member

Choose a reason for hiding this comment

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

so I guess m_colors and m_vertices are the same size? (do we need to check both these things? This seems like an odd place to enforce this restriction rather than when reserved is calculated)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, they are the same size. we could remove these asserts, it's just to make sure i didn't make a mistake when filling these vectors. but they don't enforce anything, they merly check. and checking doesn't hurt.

Copy link
Member

Choose a reason for hiding this comment

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

Got it. Wouldn't it be better to check them back up where reserved is created, then? If there's something wrong with that size, then we will encounter problems before we get to this check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is being checked with this assert is that the amount of data that we push to the m_vertices / m_colors vectors is correct (in m_vertices.addRectangle and m_colors.addForRectangle), to make sure we reserve exactly the amount we are going to push.


const QMatrix4x4 matrix = matrixForWidgetGeometry(m_waveformRenderer, true);

Expand Down
Loading
Loading