Repository navigation
Move code copied between products into libs/testing and libs/ui - #22
Merged
Merged
Conversation
- alloc_counter and the JUCE run listener (juce_init.cpp) were copied into all six products' tests. They now live once in libs/testing (target altvane_testing), which altvane_add_tests links into every test binary. - The copies behaved the same: the De-Esser's and Dialogue's only restyled the Windows aligned-allocation branch. The shared one is the EQ's and Compressor's (the latest), unchanged. - altvane_testing is an INTERFACE library, as altvane_ui is, so the listener's registration and the operator new replacement compile into each test binary; a static library would drop them, as nothing refers to them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The Leveler, De-Esser and Dialogue each had the same storePeak (a relaxed compare-exchange loop raising an atomic peak on the audio thread) and takePeaks (exchange with 0 and convert to dB, floored at -120, on the message thread). Both are now ui::PeakExchange, header-only in libs/ui/util beside TriBuffer and SpscQueue, with the same memory orders. It stays lock-free (static_assert) and allocation-free. - takeInputPeaks and takeOutputPeaks keep their signatures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Their OutputRails were the same widget: an "OUT" caption over a ui::LevelMeter, the same layout and shared colour tokens, a click that resets the peak hold. It is now ui::OutputRail (libs/ui/widgets), given the processor and a function that takes its output peaks. - A mono main output shows its one channel in both bars. Dialogue checked the main bus and the Leveler all output channels; the Leveler has one output bus, so the shared rail checks the main bus for both. - The EQ's rail stays its own: it has the display-range selector and its menu instead of the caption, a taller top row, peaks from its spectrum worker and repaints on global changes. The De-Esser's, Limiter's and Compressor's rails show gain reduction and are different widgets. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- ui::format::decibels: one decimal, the typographic minus, an optional plus, no sign within 0.04 dB of zero. It replaces the Leveler's and De-Esser's formatDb and Dialogue's formatLevel formula (the Leveler and Dialogue sign gains, the De-Esser doesn't). ui::format::gain stays the EQ's: two decimals below 10 dB. - ui::format::parseLenient: keeps the digits, signs and points of what was typed after mapping the typographic minus, so "−3.5 dB" is -3.5. It replaces the parsers in the Leveler's and De-Esser's params, Dialogue's level parameters and the Leveler's rider fader. ui::format::parseNumber differs (a comma is a decimal point, letters stop the number), so they stay apart. - The text and values are unchanged: checked against the old code for 23M floats (every float around the sign thresholds and the rounding steps up to 200 dB, and a log sweep) and 200k typed strings. - Not merged: the Compressor's formats::db and the Limiter's level (they take the sign from the rounded value, so -0.045 shows "0.0", not "−0.0"; the Compressor's also drops the decimal from 100 dB), and Dialogue's sensitivity and the Leveler's percent knob parsers (they don't map the typographic minus). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Moves code that was copied between products into the shared libraries, with no change to behaviour, sound or pixels.
Moved
libs/testing(new INTERFACE targetaltvane_testing).alloc_counter.{hpp,cpp}andjuce_init.cppwere copied into all six products'tests/(not only Dialogue and the Leveler).altvane_add_testsnow linksaltvane_testinginto every test binary, and tests include<altvane_testing/alloc_counter.hpp>.alloc_counter.cpponly restyled the Windows aligned-allocation branch (if (void* p = …),#ifdef). The shared copy is the EQ's and Compressor's (the newest), unchanged. Nothing got stricter or looser.altvane_ui. The listener's static registration and theoperator newreplacement must be compiled into each test binary, and a static library would drop them because nothing refers to them. JUCE comes from the plugin the tests link (altvane_link_plugin_code), so it isn't linked again.ui::PeakExchange(libs/ui/util/peak_exchange.hpp, header-only, next toTriBufferandSpscQueue). It replacesstorePeak/takePeaksin the Leveler, De-Esser and Dialogue processors. The memory orders are the same: a relaxed CAS loop to raise the peak, then a relaxed exchange with 0 converted to dB with a −120 floor. It is lock-free (static_assert) and doesn't allocate.takeInputPeaks/takeOutputPeakskeep their signatures.ui::OutputRail(libs/ui/widgets/output_rail) for Dialogue and the Leveler, whose rails were the same widget: the same layout, "OUT" caption, shared colour tokens and caption font, and a click that resets the peak hold. It takes the processor and a function that takes the output peaks. To show a mono output in both bars, Dialogue checked the main bus and the Leveler checked all output channels. The Leveler has a single output bus, so both checks give the same answer, and the shared rail checks the main bus. No colour parameters were needed.ui::format:decibels(db, force_sign)shows one decimal with the typographic minus, an optional plus, and no sign within 0.04 dB of zero. It replaces the Leveler's and De-Esser'sformatDband the formula inside Dialogue'sformatLevel.parseLenient(text)maps the typographic minus to a hyphen, then keeps only digits, signs and points. It replaces the parsers in the Leveler's and De-Esser's params, Dialogue's level parameters and the Leveler's rider fader.params::formatDb, the localparseNumber), so the product still decides whether gains are signed.Left alone, and why
EQ's OutputRail. It isn't the same widget: it has a display-range button and menu in place of the caption, a 22 px top row instead of 16, peaks from the
SpectrumWorker, and a repaint on global changes. Sharing it would mean hooks for the header, the hit-testing and the repaint condition, which is more code than the copy. The De-Esser's, Limiter's and Compressor's rails show gain reduction and are different widgets.History views. The parts they could share are already shared. The Leveler draws through
ui::HistoryRing,TimeAxis,LevelAxisandtraceHistoryfromlibs/ui/widgets/history. The De-Esser keeps its own:Their hover readouts also differ in geometry, rows and styling. A shared base would hold almost nothing, so I didn't add one.
Formatters that round differently. The Compressor's
formats::dband the Limiter'sleveltake the sign from the rounded value: −0.045 shows "0.0", not "−0.0". The Compressor's also drops the decimal from 100 dB.ui::format::gain(the EQ's) shows two decimals below 10 dB. Merging any of them would change displayed text.ui::format::parseNumber(used by the EQ, Compressor and Limiter) also differs fromparseLenient: it reads a comma as a decimal point, stops at letters, and returns nothing without digits. So it stays as it is.Dialogue's sensitivity and the Leveler's percent-knob parsers. They don't map the typographic minus, so "−50%" reads as 50 there. Switching them to
parseLenientwould change that, so they are untouched. That is probably an oversight and worth a separate fix.The EQ's
SpectrumWorkeroutput peaks. These use a similar CAS loop, but with sequentially consistent ordering and one channel at a time. That is a different contract, so I didn't fold it in.K-weighting (Limiter only), per the brief.
captionFont()is still defined in a few places (the Leveler's tokens, Dialogue's chrome, the Limiter's common header, and an anonymous one in libs/ui). The new rail uses its own anonymous copy, aswidgets.cppdoes. Making it public API was outside this change.No state save/load code,
.github/workflowsor vendored engine directories (dsp,zlp,chore) are touched.Testing
Local, on the Linux dev box (clang-21, Release,
-DALTVANE_BUILD_TESTS=ON -DALTVANE_BUILD_TOOLS=ON -DALTVANE_LTO=OFF -DALTVANE_FORMATS=VST3 -DALTVANE_SIMD_TARGET=AVX2 -DALTVANE_EQ_BAND_NUM=8). Dialogue was built with a local, uncommittedLinux-x86_64line in itsPLATFORMSfile.xvfb-run), all six products including Dialogue: 229/230 pass. The exception isthe models load under a locale with a decimal comma, which skips itself because the box has no decimal-comma locale (only C, POSIX and en_US), and ctest counts an all-skipped run as failed. Neither the test nor the model loader changes here, and it passes on macOS and Windows CI.nmshows all six test binaries define the replacementoperator new,alloc_counter::beginandJuceRunListenerfromlibs/testing.tools/snapshoton unmodifiedmainand on this branch, then rendered the Leveler (7), Dialogue (8) and De-Esser (9) snapshots at scale 2. All 24 PNGs are byte-identical tomain's. Two baseline runs were also byte-identical to each other, so the renders are deterministic and a byte comparison is meaningful. The EQ, Compressor and Limiter editors aren't touched; their only change is in their tests.ui::format::decibelsandparseLenientwith the code they replace (the Leveler's, De-Esser's and Dialogue's formatters and parsers). It found 0 mismatches over 23M floats and 200k typed strings. The floats were every value within ±0.035–0.045 and 64 ulps either side of each 0.05 rounding step up to ±200 dB, plus a log sweep from 1e-30 to 1e6. The strings were random mixes of digits, signs, points, commas, spaces, units and the typographic minus, plus edge cases. The only possible difference is text without any digits: it now always parses as +0 where it could have been −0 before, and the two compare equal.CI, run 41495002417, all green:
Not verified: nothing was run in a DAW. Real-time timing isn't affected (the audio-thread code is the same CAS loop and exchange, moved into a header) and wasn't measured, since this box's numbers aren't meaningful.
Review
codex exec review --base origin/main, round 1: "No actionable regressions were identified." There have been no code changes since, so no further rounds.🤖 Generated with Claude Code