Skip to content

Bit Invader cleanup#8298

Open
rubiefawn wants to merge 10 commits intoLMMS:masterfrom
rubiefawn:refac/bitinvader
Open

Bit Invader cleanup#8298
rubiefawn wants to merge 10 commits intoLMMS:masterfrom
rubiefawn:refac/bitinvader

Conversation

@rubiefawn
Copy link
Contributor

@rubiefawn rubiefawn commented Mar 13, 2026

Makes several changes to the Bit Invader plugin for increased maintainability and readability, which is especially important since Bit Invader has been used as a starting point for many (including myself) when learning about how LMMS instrument plugins work.

User interface is modified as little as possible in this PR as UI upgrades will be taken care of in #8122.

image

Performance

  • Remove unnecessary duplicated state and logic
  • Perform non-sample-exact calculations once per buffer, rather than once per sample
  • Remove unnecessary heap allocations
  • Optimize struct/class member layout for better locality
  • Run actual benchmarks to make sure all this has the intended effect on performance

Functionality

  • Improve normalization behavior, such that the wavetable always has an output range of [-1, 1], including DC offset prevention
  • Change normalization toggle to a combo box with four options: "Off", "Full", "Length only", and "Legacy".
    • Off: No normalization is applied
    • Full: The improved normalization behavior described above
    • Length only: Same as "Full", but accounts for the "Length" knob, such that samples that do not contribute to the sound also do not contribute to normalization calculations.
    • Legacy: Previous behavior. Old presets that had normalization toggled on load with this setting applied.
  • Update layout enough to make room for the normalization combo box

Miscellaneous

  • Use non-static data member initializers where possible
  • Change uses of SIGNAL and SLOT macros to whatever they should be post-Qt6 upgrade
  • Completely conform to current code conventions
  • Update presets (?)

@rubiefawn rubiefawn self-assigned this Mar 13, 2026
@rubiefawn rubiefawn marked this pull request as ready for review March 17, 2026 05:28
@rubiefawn rubiefawn added needs style review A style review is currently required for this PR needs code review A functional code review is currently required for this PR needs testing This pull request needs more testing labels Mar 17, 2026
compiling takes 30 minutes on this laptop yeeowch
Previously, BitInvader would normalize using the following steps:

1. Whenever the wavetable was changed, it would find the sample with
   greatest magnitude and record its reciprocal.
2. Whenever a new note was constructed, it would allocate a copy of the
   wavetable on the heap **during playback!!!**, multiply each sample by
   the above reciprocal, and then clip it to [-1, 1] for good measure.

The new implementation is as follows:

1. Whenever the wavetable is changed, it records double the reciprocal
   of the difference between the maximum sample and minimum samples.
   Multiplying the output by this factor ensures that the magnitude of
   the output is 2 (expected from a wave normalized to [-1, 1]).
2. However, multiplying by that factor magnifies any existing DC offset
   in the wavetable, so the negative quotient of the sum and difference
   between the maximum sample and minimum sample is also recorded.
   Adding this to the output after it is multiplied by the above factor
   removes any DC offset, ensuring the normalized wave has the exact
   range of [-1, 1].

This could forseeably break some user projects by removing any
intentionally-created DC offset. To that I say, "xkcd 1172".
It was mostly unnecessarily duplicated state. Moving those calculations
into BitInvader::playNote() allows them to happen once per buffer rather
than once per sample.
Moved it into the constructor, like Nescaline (and sort of
Vestige and Organic, though that's in their paint events)
Instead of storing and updating two indicies, now only the real index is
stored, since the integer index was dependent on the real index every
sample anyways. Now instead of a struct, the note plugin data is just a
float through a using type alias.
Changes the normalization toggle to a combo box. The options are:

- Off: No normalization.
- Full: Normalization is applied such that the resulting waveform has a
  range of [-1, 1].
- Length only: Same as "Full", but takes into account the current value
  of the "Length" knob. This is the new default value.
- Legacy: Normalization is applied such that the resulting waveform has
  one or more samples with magnitude 1. This was the previous
  normalization behavior, and the value with which old presets load with
  if they had normalization enabled.

The combo box is haphazardly placed and lacks a proper label, so the UI
looks like trash as a result. This is temporary.
This extends the background art vertically by 30 pixels and does a whole
lot of naughty things that I would never do... except I'm about to fix
them in another PR lol
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement needs code review A functional code review is currently required for this PR needs style review A style review is currently required for this PR needs testing This pull request needs more testing performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant