Ω-W1-T3 review fixes: skip meter invalidate when covered; push fast-path predicates into core
Fixes the critical finding — Browse/curve-popup no longer trigger 60 Hz whole-client repaints when the meter is invisible. Pushes the fast-path and self-containment geometry into master_meter with tests; batches the four minor findings.
This commit is contained in:
@@ -362,7 +362,7 @@ anything for a trigger shape.
|
||||
drag the bank model and the WAV codec in behind it. The shell keeps only the controls the
|
||||
parameter set does not carry (key-track, voice count, master gain, preview velocity) and the
|
||||
labels for them.
|
||||
- `master_meter` — the MASTER column's interior, split from `knob_deck` on the axis `sample_chrome` has to `sample_bands`: that says where the column is, this lays out inside it (22 px numeral gutter · 4 · 36 px bar field) and holds the per-instance UI state the bars draw from. `kMeterColumnW` is the SUM of those three, exported so `deck_groups`' MASTER descriptor reserves exactly what the interior consumes — the column is banked to grow, and a reserve that did not track it would underfill or overrun silently. **Bar count takes a RESOLVED `LaneSplit`, the same value `waveform_view`'s `resolveLaneSplit` answers** — a mono source under stereo mode is dual-mono, and two identical bars would be a lie. Also owns `meterTickNumeralled` (the spec-pinned 0/−12/−24/−36/−48/−60 numeral set, beside the tick step it derives from), `meterNumeralRect` (bottom-clamped, so the floor tick's numeral cannot hang out of the gutter), and `meterSingleLaneState` — the one bar folds both channels PER FIELD, never picking a whole channel by level. Composes `engine/meter_ballistics` per channel and gives the gain-reduction lamp the peak tick's own hold-then-release, without which a catch smaller than 20 dB × the UI period is dark again before it has been drawn twice; the audio thread's clip flag is ORed in because it is the only latch that sees every block. `meterDrawEqual` is what lets the UI tick repaint on change alone.
|
||||
- `master_meter` — the MASTER column's interior, split from `knob_deck` on the axis `sample_chrome` has to `sample_bands`: that says where the column is, this lays out inside it (22 px numeral gutter · 4 · 36 px bar field) and holds the per-instance UI state the bars draw from. `kMeterColumnW` is the SUM of those three, exported so `deck_groups`' MASTER descriptor reserves exactly what the interior consumes — the column is banked to grow, and a reserve that did not track it would underfill or overrun silently. **Bar count takes a RESOLVED `LaneSplit`, the same value `waveform_view`'s `resolveLaneSplit` answers** — a mono source under stereo mode is dual-mono, and two identical bars would be a lie. Also owns `meterTickNumeralled` (the spec-pinned 0/−12/−24/−36/−48/−60 numeral set, beside the tick step it derives from), `meterNumeralRect` (bottom-clamped, so the floor tick's numeral cannot hang out of the gutter), and `meterSingleLaneState` — the one bar folds both channels PER FIELD, never picking a whole channel by level. Composes `engine/meter_ballistics` per channel and gives the gain-reduction lamp the peak tick's own hold-then-release, without which a catch smaller than 20 dB × the UI period is dark again before it has been drawn twice; the audio thread's clip flag is ORed in because it is the only latch that sees every block. `meterDrawEqual` is what lets the UI tick repaint on change alone. Also owns the editor's two paint-dispatch predicates, so neither lives as a bare comment in the shell: `meterFastPathEligible` (is a WM_PAINT dirty rect wholly inside the field?) and `meterBarsWithinField` (the self-containment invariant that fast path rests on), both asserted in `test_master_meter.cpp`.
|
||||
- `deck_groups` — also home to `deckParamCommit` and `liveCommitFor`, the editor's whole commit-tier routing decision (see "Live parameter delivery" above), and to `OverlayEnv` + `nextOverlaySelection`/`overlayEnvEnabled`/`overlayEnvInert`, the whole overlay-selection state machine (exclusivity, the none resting state, and which selections a disabled or DRAWN group makes inert); WHICH groups the Sample face's deck carries, split from `knob_deck`'s HOW they lay out: the `DeckParam` control-id space (the editor's `ParamControl` is an alias of it), the `DeckGroupId` list, `sampleDeckGroups` in signal-flow order (**pitch → filter → amp**, then velocity/voice/master), and the deck's bipolar-knob law. Reads `PlayMode` for the AMP group's Gate/Trigger face, which is why this and not `knob_deck` is the module that touches the engine's value layer. Also home to `CurveTarget` + `curveTargetFor` — the VELOCITY group's three cells are popup openers, not dials, and that predicate is the ONE place they are named, so paint, hit-test routing and the popup's title all agree. MASTER is reserved for post-voice-mixer concerns, which is why the curves sit in their own group immediately left of VOICE rather than there; it now discharges that reservation as the double-height bus deck — gain, the limiter enable, one reserved slot, the meter column and the GR lamp. FILTER's `Band|Notch` rides its caption slack rather than the knob row: that is the −92 px that makes the SOUND row fit its block, and putting it back breaks the fit. VOICE's `Retrig|Legato` deliberately stays in the knob row — VOICE's caption row is the binding side, so moving it there makes the group 226 rather than 164.
|
||||
- `spline_edit` — THE point-editing grammar, and the one place it is written down: left-click grabs a node and adds one in empty space, right-click deletes, control-click toggles hard/smooth. Both spline consumers — the velocity-curve popup and the spline EG overlay — route their mouse-down through `resolveSplineEdit`, so the two cannot drift into two grammars. The endpoint and point-count rules are NOT restated here: `deletePoint` and `addPoint` own them, and the caller applies the resolved action to the curve. Also home to `splineOverlayBox`, the contour's mapping box inside the waveform overlay — the FULL area, no inset, so the drawn contour stays 1:1 with the sample's time axis. Spline points are excluded from `param_taper`'s Shift/Ctrl modifier law like waveform markers are: a point is a normalized position with no displayed unit, and control-click there is already claimed by the hard/smooth toggle above.
|
||||
- `curve_popup` — pure curve-popup geometry + dismissal test (FB1): centered sheet over the Sample face — width/height clamps, title row, Close button rect, curve-box rect, outside-sheet dismissal test. Mirror of `overflow_menu`; no LICE or REAPER types.
|
||||
|
||||
@@ -26,6 +26,24 @@ MeterRects meterRects(const Rect& column, LaneSplit split) {
|
||||
return r;
|
||||
}
|
||||
|
||||
bool meterFastPathEligible(const MeterRects& m, const Rect& dirty) {
|
||||
return !m.field.empty() && dirty.x >= m.field.x && dirty.y >= m.field.y &&
|
||||
dirty.right() <= m.field.right() && dirty.bottom() <= m.field.bottom();
|
||||
}
|
||||
|
||||
namespace {
|
||||
bool rectWithin(const Rect& outer, const Rect& inner) {
|
||||
return inner.empty() ||
|
||||
(inner.x >= outer.x && inner.right() <= outer.right() && inner.y >= outer.y &&
|
||||
inner.bottom() <= outer.bottom());
|
||||
}
|
||||
} // namespace
|
||||
|
||||
bool meterBarsWithinField(const MeterRects& m) {
|
||||
if (m.field.empty()) return m.barA.empty() && m.barB.empty();
|
||||
return rectWithin(m.field, m.barA) && rectWithin(m.field, m.barB);
|
||||
}
|
||||
|
||||
Rect meterNumeralRect(const Rect& labels, int y) {
|
||||
if (labels.empty()) return {};
|
||||
int top = y - 5;
|
||||
|
||||
@@ -42,6 +42,19 @@ struct MeterRects {
|
||||
// A column narrower than kMeterColumnW yields nothing rather than an overrunning field.
|
||||
MeterRects meterRects(const Rect& column, LaneSplit split);
|
||||
|
||||
// Whether `dirty` (a WM_PAINT update rect, already client-clipped) lies wholly inside the bar
|
||||
// field — the editor's meter-frame fast-path test. Geometric, not a flag: the caller decides
|
||||
// per paint from the rect Windows actually handed it, never from a remembered "was this a
|
||||
// meter tick" bit, because Windows unions a meter invalidate with any other pending one into a
|
||||
// single rcPaint and a flag would then paint a meter frame over a face that had really changed.
|
||||
bool meterFastPathEligible(const MeterRects& m, const Rect& dirty);
|
||||
|
||||
// The structural invariant drawMeterField's self-containment rests on: every bar it paints
|
||||
// stays inside the field it fills first. The ticks, the held-peak tick and the clip cap are all
|
||||
// drawn directly off `field`'s own coordinates and so are bounded by construction; the bars are
|
||||
// the one independently-computed rect that isn't.
|
||||
bool meterBarsWithinField(const MeterRects& m);
|
||||
|
||||
// y of `db` inside the bar field — kMeterTopDb at the top edge, kMeterFloorDb at the bottom,
|
||||
// linear in dB between, clamped outside.
|
||||
int meterDbToY(const Rect& field, double db);
|
||||
|
||||
@@ -284,7 +284,11 @@ against a performance budget — they are there because `VoiceEngine::applyLiveT
|
||||
a flag would paint a meter frame over a face that had really changed. What stays outside the
|
||||
field stays on the full path: the static numeral gutter (AA text re-blended onto itself every
|
||||
frame thickens) and the GR lamp (drawn straight onto the deck group's gradient, which a
|
||||
sub-rect fill cannot reproduce), so a lamp transition takes a whole-client repaint.
|
||||
sub-rect fill cannot reproduce), so a lamp transition takes a whole-client repaint. **When the
|
||||
meter is covered or absent** (Browse, the curve popup, the empty state) `invalidateMeter`
|
||||
skips invalidating anything at all rather than falling back to a whole-client repaint — the
|
||||
ballistics still advance on `onMeterTimer`'s own clock, but nothing visible changed, so a
|
||||
60 FPS whole-client repaint under a modal sheet would be pure cost for zero pixels shown.
|
||||
- The bake's availability probe runs on the SAME tick that paints the button, so the
|
||||
control can never be enabled on one tick and refuse on the next. The bake Hold control's
|
||||
applicability (`resolveBakeHoldNeeded`) rides the same tick for the same reason, and
|
||||
|
||||
@@ -51,11 +51,9 @@ void ReaSamplerEditor::paint(HDC hdc, const RECT& dirty) {
|
||||
|
||||
// A meter frame's dirty rect lies wholly inside the bar field, and nothing else is drawn
|
||||
// there — so the field is all that needs redrawing, and the rest of the face is still in the
|
||||
// back buffer from the last full compose. Decided GEOMETRICALLY rather than by a flag,
|
||||
// because Windows coalesces a meter invalidate with any other pending one into a single
|
||||
// union rect: a flag would then paint a meter frame over a face that had really changed.
|
||||
const Rect& mf = meterRects_.field;
|
||||
if (!mf.empty() && dx >= mf.x && dy >= mf.y && dr <= mf.right() && db <= mf.bottom()) {
|
||||
// back buffer from the last full compose. Decided GEOMETRICALLY rather than by a flag — see
|
||||
// meterFastPathEligible (master_meter) for why.
|
||||
if (meterFastPathEligible(meterRects_, Rect::ltrb(dx, dy, dr, db))) {
|
||||
paintMeterField(bmp);
|
||||
} else {
|
||||
// Repopulated by paintDeck when the meter is on screen; left empty by the empty state.
|
||||
@@ -83,7 +81,8 @@ void ReaSamplerEditor::paint(HDC hdc, const RECT& dirty) {
|
||||
}
|
||||
|
||||
// A sheet layered over the face covers the meter, so the fast path must not paint
|
||||
// through it — dropping the rect routes the next meter tick down this path instead.
|
||||
// through it even if something else invalidates a rect that happens to match the
|
||||
// stale field bounds — invalidateMeter itself already skips while covered (see there).
|
||||
if (view_ == View::kBrowse || curvePopup_ != CurveTarget::kNone) meterRects_ = {};
|
||||
}
|
||||
|
||||
|
||||
@@ -54,6 +54,10 @@ void drawMeterNumerals(LICE_IBitmap* bmp, const MeterRects& m) {
|
||||
// covers every pixel the rest of it then draws, so a meter frame can redraw this rect alone.
|
||||
// Two bars or one is read off the rects (barB is empty exactly when the split is Single), so
|
||||
// the lane decision has one representation here rather than two.
|
||||
// This self-containment depends on Role::BgCell being fully OPAQUE at InteractionState::Rest
|
||||
// (theme.cpp: alpha 255) — fillGradient blends rather than overwrites at alpha < 255, so a
|
||||
// translucent BgCell would make every meter frame re-blend over whatever the last frame left,
|
||||
// the exact AA-thickening the numeral gutter comment below is guarding against.
|
||||
void drawMeterField(LICE_IBitmap* bmp, const MeterRects& m, const MasterMeterUi& state) {
|
||||
// A column narrower than the interior needs yields all-empty rects, which under rect.h's
|
||||
// contract means suppressed — not a zero-height field to fill, tick twelve times and cap.
|
||||
|
||||
@@ -77,8 +77,20 @@ void ReaSamplerEditor::invalidate() {
|
||||
|
||||
void ReaSamplerEditor::invalidateMeter() {
|
||||
if (!childHwnd_) return;
|
||||
// The meter is covered (Browse, the curve popup) or has nothing to draw (empty state) —
|
||||
// ballistics still advance in onMeterTimer, but nothing on screen changed, so invalidating
|
||||
// anything here would only buy a whole-client repaint of chrome/waveform/deck the meter
|
||||
// never touches. Distinct from "bounds not resolved yet" below: this is a fact about what
|
||||
// the face is showing, not about whether meterRects_ happens to be populated.
|
||||
const bool meterOnScreen =
|
||||
view_ == View::kSample && curvePopup_ == CurveTarget::kNone && !selectedId_.empty();
|
||||
if (!meterOnScreen) return;
|
||||
|
||||
const Rect& f = meterRects_.field;
|
||||
if (f.empty()) { // nothing cached (see meterRects_) — the whole face has to answer
|
||||
if (f.empty()) {
|
||||
// On screen, but no full paint has resolved its bounds yet (first paint, or a resize
|
||||
// just dropped the cache) — the whole-client fallback is cheap here because the window
|
||||
// is already fully invalid from the resize/creation that caused this.
|
||||
invalidate();
|
||||
return;
|
||||
}
|
||||
@@ -155,6 +167,10 @@ void ReaSamplerEditor::removedFromParent() {
|
||||
childHwnd_ = nullptr;
|
||||
}
|
||||
releaseBackBuffer(); // a client-area bitmap outlives nothing here
|
||||
// Unreachable today (WM_PAINT outranks WM_TIMER, so a reopen's first paint resolves this
|
||||
// before any meter tick can read it stale) — cleared anyway so the invariant is structural,
|
||||
// not a timing accident, the same reason onSize clears it on resize.
|
||||
meterRects_ = {};
|
||||
}
|
||||
|
||||
tresult PLUGIN_API ReaSamplerEditor::onSize(ViewRect* newSize) {
|
||||
|
||||
@@ -1,9 +1,8 @@
|
||||
// reasampler_editor.h — VST3 IPlugView LICE editor for the ReaSampler 9000 UI. Thin shell:
|
||||
// hosts a LICE child window, routing host paint/mouse into the pure geometry modules
|
||||
// (sample_bands, sample_chrome, capture_browser, keyboard_strip, sample_map). The Sample
|
||||
// face is a three-band stack — chrome, waveform, decks — and the shell TUs split on that
|
||||
// same axis; Browse is a modal picker over it. All layout/hit-test/drag math lives in the
|
||||
// pure modules; every edit commits off the audio thread via reloadInstrument.
|
||||
// hosts a LICE child window, routing host paint/mouse into the pure geometry modules. The
|
||||
// Sample face is a three-band stack — chrome, waveform, decks — and the shell TUs split on
|
||||
// that same axis; Browse is a modal picker over it. All layout/hit-test/drag math lives in
|
||||
// the pure modules; every edit commits off the audio thread via reloadInstrument.
|
||||
|
||||
#pragma once
|
||||
|
||||
@@ -65,6 +64,9 @@ public:
|
||||
// always supplies one).
|
||||
explicit ReaSamplerEditor(ReaSamplerProcessor* processor);
|
||||
~ReaSamplerEditor() override;
|
||||
// backBuffer_ owns a raw LICE bitmap with no refcount behind it — a copy would double-free.
|
||||
ReaSamplerEditor(const ReaSamplerEditor&) = delete;
|
||||
ReaSamplerEditor& operator=(const ReaSamplerEditor&) = delete;
|
||||
|
||||
Steinberg::tresult PLUGIN_API isPlatformTypeSupported(
|
||||
Steinberg::FIDString type) override;
|
||||
@@ -302,8 +304,8 @@ private:
|
||||
struct ChannelPcm {
|
||||
std::vector<AudioSample> interleaved; // frame-interleaved source frames
|
||||
int channelCount = 0; // 0 = nothing decoded
|
||||
Envelope heroEnv; // binned from `interleaved`, so every path that drops the
|
||||
std::size_t heroBins = 0; // decode drops it too. 0 = not computed
|
||||
Envelope heroEnv; // binned from `interleaved`; cleared whenever it is
|
||||
std::size_t heroBins = 0; // 0 = not computed
|
||||
std::int64_t frameCount() const {
|
||||
return channelCount > 0
|
||||
? static_cast<std::int64_t>(interleaved.size()) / channelCount
|
||||
@@ -467,9 +469,7 @@ private:
|
||||
// up. One note at a time — a fresh press releases the prior.
|
||||
int previewingNote_ = -1;
|
||||
|
||||
// Resample bake. The click only ARMS it; the sync tick runs it. Running it inline
|
||||
// would nest a synchronous REAPER action — which re-points this very instance — inside
|
||||
// a mouse handler with the capture held.
|
||||
// Resample bake — the click only ARMS it (see this directory's CLAUDE.md Gotchas for why).
|
||||
bool bakePending_ = false;
|
||||
// Whether the extension's bake action is registered, resolved on the same tick that
|
||||
// governs the button's paint, so the control is never enabled and then refusing.
|
||||
@@ -494,8 +494,8 @@ private:
|
||||
// The MASTER deck's meter, advanced from the published block magnitudes on its own timer
|
||||
// (see onMeterTimer for why it runs mid-drag too, and why the first read is discarded).
|
||||
// A default-constructed meterTick_ means never ticked. meterRects_ is the column interior
|
||||
// resolved at the last FULL paint; an empty field means "not on screen" (no layout yet, the
|
||||
// empty state, a modal sheet over it) and sends the tick down the whole-client path.
|
||||
// resolved at the last FULL paint; an empty field is ambiguous alone (no layout yet, or
|
||||
// genuinely off screen) — invalidateMeter asks view_/curvePopup_/selectedId_ to tell which.
|
||||
instrument::ui::MasterMeterUi masterMeter_;
|
||||
std::chrono::steady_clock::time_point meterTick_{};
|
||||
instrument::ui::MeterRects meterRects_;
|
||||
|
||||
@@ -5,6 +5,8 @@
|
||||
// the mono bar taking the whole field, the two stereo bars, all inside the column.
|
||||
// * bar count — the SAME LaneSplit resolveLaneSplit folds, over channel mode x source
|
||||
// channel count, so it can never become a second rule.
|
||||
// * the editor's dirty-rect fast path — meterFastPathEligible's wholly-inside test, and
|
||||
// meterBarsWithinField, the self-containment invariant the fast path rests on.
|
||||
// * the dB axis — top/floor on the field's edges, an interior value, and the clamps.
|
||||
// * ballistics — instantaneous rise, 20 dB/s fall, the 1.5 s hold and its release AT RATE;
|
||||
// the audio thread's clip latch surviving a UI frame; the per-field single-lane fold.
|
||||
@@ -287,6 +289,29 @@ static void testDrawEqualityCoversTheDrawnQuantities() {
|
||||
CHECK(meterDrawEqual(a, graze));
|
||||
}
|
||||
|
||||
// The editor's WM_PAINT fast-path test: a dirty rect wholly inside the field takes the
|
||||
// meter-only redraw; anything that pokes outside it, or a field that hasn't been laid out yet,
|
||||
// falls back to the full compose.
|
||||
static void testFastPathEligibleOnlyForADirtyRectInsideTheField() {
|
||||
const MeterRects m = meterRects(kColumn, LaneSplit::Single);
|
||||
CHECK(meterFastPathEligible(m, m.field)); // the whole field
|
||||
CHECK(meterFastPathEligible(
|
||||
m, Rect::ltrb(m.field.x + 2, m.field.y + 2, m.field.right() - 2, m.field.bottom() - 2)));
|
||||
CHECK(!meterFastPathEligible(m, Rect::ltrb(m.field.x - 1, m.field.y, m.field.right(),
|
||||
m.field.bottom()))); // pokes left of the field
|
||||
CHECK(!meterFastPathEligible(
|
||||
m, Rect::ltrb(kColumn.x, kColumn.y, kColumn.right(), kColumn.bottom()))); // whole column
|
||||
CHECK(!meterFastPathEligible(MeterRects{}, m.field)); // no cached layout at all
|
||||
}
|
||||
|
||||
// The self-containment invariant drawMeterField's redraw-the-field-alone shortcut rests on:
|
||||
// every bar meterRects() hands back stays inside the field, in both lane splits.
|
||||
static void testFieldBoundsEveryBarItPaints() {
|
||||
CHECK(meterBarsWithinField(meterRects(kColumn, LaneSplit::Single)));
|
||||
CHECK(meterBarsWithinField(meterRects(kColumn, LaneSplit::Stereo)));
|
||||
CHECK(meterBarsWithinField(MeterRects{})); // degenerate: nothing to bound, nothing drawn
|
||||
}
|
||||
|
||||
static void testDegenerateColumnYieldsNothing() {
|
||||
const MeterRects m = meterRects(Rect::ltrb(0, 0, 0, 0), LaneSplit::Stereo);
|
||||
CHECK(m.field.empty() && m.barA.empty() && m.barB.empty());
|
||||
@@ -300,6 +325,8 @@ int main() {
|
||||
testEveryOtherTickCarriesANumeral();
|
||||
testTheFloorNumeralStaysInsideTheGutter();
|
||||
testAColumnTooNarrowForTheInteriorDrawsNothing();
|
||||
testFastPathEligibleOnlyForADirtyRectInsideTheField();
|
||||
testFieldBoundsEveryBarItPaints();
|
||||
testPeakRisesAtOnceAndFallsAtTwentyDbPerSecond();
|
||||
testPeakHoldSitsForItsFullWindowThenReleases();
|
||||
testClipLatchesFromThePublishedFlagAndClearsOnDemand();
|
||||
|
||||
Reference in New Issue
Block a user