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 2 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
74 changes: 43 additions & 31 deletions src/waveform/renderers/allshader/waveformrendererlrrgb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,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 @@ -92,13 +92,12 @@ void WaveformRendererLRRGB::paintGL() {

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 Down Expand Up @@ -140,48 +139,61 @@ void WaveformRendererLRRGB::paintGL() {
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 magnitude of the maxLow, maxMid and maxHigh values
const float magnitude = std::sqrt(
math_pow2(maxLow) + math_pow2(maxMid) + math_pow2(maxHigh));

// Apply the gains
maxLow *= lowGain;
maxMid *= midGain;
maxHigh *= highGain;

// Calculate the magnitude of the gained maxLow, maxMid and maxHigh values
const float magnitudeGained = std::sqrt(
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 (magnitude != 0.f) {
const float factor = magnitudeGained / magnitude;
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 +205,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;
}

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
75 changes: 44 additions & 31 deletions src/waveform/renderers/allshader/waveformrendererrgb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ void WaveformRendererRGB::paintGL() {
const float breadth = static_cast<float>(m_waveformRenderer->getBreadth()) * devicePixelRatio;
const float halfBreadth = breadth / 2.0f;

const float heightFactor = allGain * halfBreadth / std::sqrt(3.f * 256.f * 256.f);
const float heightFactor = allGain * halfBreadth / 255.f;

const float low_r = static_cast<float>(m_rgbLowColor_r);
const float mid_r = static_cast<float>(m_rgbMidColor_r);
Expand Down Expand Up @@ -137,63 +137,76 @@ void WaveformRendererRGB::paintGL() {

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

// combined left+right
float maxLow{};
float maxMid{};
float maxHigh{};
// per channel
float maxAll[2]{};

// Find the max values for low, mid, high and all in the waveform data.
// - Max of left and right
uchar u8maxLow{};
uchar u8maxMid{};
uchar u8maxHigh{};
// - Per channel
uchar u8maxAllChn[2]{};
for (int chn = 0; chn < 2; chn++) {
// 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);

maxLow = math_max(maxLow, filteredLow);
maxMid = math_max(maxMid, filteredMid);
maxHigh = math_max(maxHigh, filteredHigh);

const float all = math_pow2(filteredLow) * lowGain +
math_pow2(filteredMid) * midGain +
math_pow2(filteredHigh) * highGain;
maxAll[chn] = math_max(maxAll[chn], all);
u8maxLow = math_max(u8maxLow, waveformData.filtered.low);
u8maxMid = math_max(u8maxMid, waveformData.filtered.mid);
u8maxHigh = math_max(u8maxHigh, waveformData.filtered.high);
u8maxAllChn[chn] = math_max(u8maxAllChn[chn], waveformData.filtered.all);
}
}

// Cast to float
float maxLow = static_cast<float>(u8maxLow);
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])};

// Calculate the magnitude of the maxLow, maxMid and maxHigh values
const float magnitude = std::sqrt(
math_pow2(maxLow) + math_pow2(maxMid) + math_pow2(maxHigh));

// Apply the gains
maxLow *= lowGain;
maxMid *= midGain;
maxHigh *= highGain;

// Calculate the magnitude of the gained maxLow, maxMid and maxHigh values
const float magnitudeGained = std::sqrt(
Copy link
Member

Choose a reason for hiding this comment

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

You can use only one std::sqrt() if you build the quotient (factor below) before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I understand what you mean here

Copy link
Member

Choose a reason for hiding this comment

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

You can make use of the rule:
factor = Sqrt(A)/Sqrt(B) = Sqrt(A/B)
To get rid of one expensive Sqrt call.

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, well spotted! i'll make the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

math_pow2(maxLow) + math_pow2(maxMid) + math_pow2(maxHigh));

// The maxAll values will be used to draw the amplitude. We scale them according to
// magnitude of the gained maxLow, maxMid and maxHigh values
if (magnitude != 0.f) {
const float factor = magnitudeGained / magnitude;
maxAllChn[0] *= factor;
maxAllChn[1] *= 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) {
ywwg marked this conversation as resolved.
Show resolved Hide resolved
// 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;
}

// lines are thin rectangles
// maxAll[0] is for left channel, maxAll[1] is for right channel
// Lines are thin rectangles
m_vertices.addRectangle(fpos - 0.5f,
halfBreadth - heightFactor * std::sqrt(maxAll[0]),
halfBreadth - heightFactor * maxAllChn[0],
fpos + 0.5f,
halfBreadth + heightFactor * std::sqrt(maxAll[1]));
halfBreadth + heightFactor * maxAllChn[1]);
m_colors.addForRectangle(red, green, blue);

xVisualSampleIndex += visualIncrementPerPixel;
Expand Down