Address review: sparse overrides, hfov clear, readout prune

Three fixes from PR review of the per-pane tuner:

- Sync no longer writes back. _syncAspectPanel dispatches synthetic input
  events to refresh slider labels; guard those with _aspectSyncing so the
  slider handler skips the write. Previously opening/switching a target
  populated a full override for every field (defeating sparse inherit) and
  spammed localStorage.

- Unchecking "Override held hFOV" on a pane target now clears the override
  key (via _aspectClearVal) so the pane re-inherits the base value, instead
  of pinning hfovDeg:null in the override. On the base target it still sets
  the explicit auto (null).

- _aspectPrunePanes now prunes the matching __h3dAspectReadout slot and drops
  a dangling __last, so the readout cache can't grow unbounded as songs and
  arrangements churn.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: topkoa <topkoa@gmail.com>
This commit is contained in:
topkoa
2026-07-01 02:27:01 -04:00
co-authored by Claude Opus 4.8
parent 5ef163e9f9
commit 9f914770c6
2 changed files with 80 additions and 17 deletions
+52 -17
View File
@@ -1773,11 +1773,23 @@
const reg = window.__h3dAspectPanes; const reg = window.__h3dAspectPanes;
if (!reg) return; if (!reg) return;
const now = _aspectNowMs(); const now = _aspectNowMs();
const ro = window.__h3dAspectReadout;
Object.keys(reg).forEach((k) => { Object.keys(reg).forEach((k) => {
if (now - (reg[k].seen || 0) > 1500) { delete reg[k]; _aspectPanesDirty = true; } if (now - (reg[k].seen || 0) > 1500) {
delete reg[k];
// Prune the matching readout slot so it can't grow unbounded as
// songs/arrangements churn, and drop a dangling __last pointer.
if (ro) { delete ro[k]; if (ro.__last === k) delete ro.__last; }
_aspectPanesDirty = true;
}
}); });
} }
// True while _syncAspectPanel is programmatically refreshing controls, so the
// synthetic 'input' events it dispatches to update labels don't write back
// into the tune (which would populate a full override for every field and
// spam localStorage). Real user input runs with this false.
let _aspectSyncing = false;
// Read/write against the current edit target ('' → base, else pane override). // Read/write against the current edit target ('' → base, else pane override).
function _aspectReadVal(k) { function _aspectReadVal(k) {
const base = _aspectTune(); const base = _aspectTune();
@@ -1794,6 +1806,18 @@
} }
_aspectPersist(); _aspectPersist();
} }
// Clear a field: for the base target set the explicit auto value (null); for a
// pane target delete the override key so the pane re-inherits the base value
// (and drop the pane's override object once it's empty).
function _aspectClearVal(k) {
const base = _aspectTune();
if (!_aspectEditTarget) { base[k] = null; }
else {
const m = base.__panels, ov = m && m[_aspectEditTarget];
if (ov) { delete ov[k]; if (!Object.keys(ov).length) delete m[_aspectEditTarget]; }
}
_aspectPersist();
}
// (Re)build the Target dropdown from the live pane registry, preserving the // (Re)build the Target dropdown from the live pane registry, preserving the
// current selection when it's still valid. // current selection when it's still valid.
@@ -1892,7 +1916,8 @@
const show = () => { val.textContent = (+sl.value).toFixed(f.step < 1 ? 2 : 0); }; const show = () => { val.textContent = (+sl.value).toFixed(f.step < 1 ? 2 : 0); };
show(); show();
sl.addEventListener('input', () => { sl.addEventListener('input', () => {
_aspectWriteVal(f.k, parseFloat(sl.value)); show(); show(); // label always refreshes
if (!_aspectSyncing) _aspectWriteVal(f.k, parseFloat(sl.value));
}); });
row.appendChild(sl); wrap.appendChild(row); row.appendChild(sl); wrap.appendChild(row);
}); });
@@ -1913,11 +1938,13 @@
sl.disabled = !cb.checked; sl.disabled = !cb.checked;
sl.style.cssText = 'width:100%;'; sl.style.cssText = 'width:100%;';
cb.addEventListener('change', () => { cb.addEventListener('change', () => {
if (_aspectSyncing) return;
sl.disabled = !cb.checked; sl.disabled = !cb.checked;
_aspectWriteVal('hfovDeg', cb.checked ? parseFloat(sl.value) : null); if (cb.checked) _aspectWriteVal('hfovDeg', parseFloat(sl.value));
else _aspectClearVal('hfovDeg'); // base → auto (null); pane → re-inherit base
}); });
sl.addEventListener('input', () => { sl.addEventListener('input', () => {
if (cb.checked) _aspectWriteVal('hfovDeg', parseFloat(sl.value)); if (!_aspectSyncing && cb.checked) _aspectWriteVal('hfovDeg', parseFloat(sl.value));
}); });
row.appendChild(sl); wrap.appendChild(row); row.appendChild(sl); wrap.appendChild(row);
_aspectHfovCb = cb; _aspectHfovSl = sl; _aspectHfovCb = cb; _aspectHfovSl = sl;
@@ -1972,19 +1999,27 @@
function _syncAspectPanel() { function _syncAspectPanel() {
if (!_aspectPanelEl) return; if (!_aspectPanelEl) return;
_aspectBuildTargets(); _aspectBuildTargets();
_aspectPanelEl.querySelectorAll('input[type=checkbox][data-k]').forEach((cb) => { // Guard so the synthetic 'input' events below only refresh labels and
cb.checked = !!_aspectReadVal(cb.dataset.k); // don't write the read-back values into the target (which would turn a
}); // sparse pane override into a full one and spam localStorage).
_aspectPanelEl.querySelectorAll('input[type=range][data-k]').forEach((sl) => { _aspectSyncing = true;
const v = _aspectReadVal(sl.dataset.k); try {
if (Number.isFinite(v)) sl.value = v; _aspectPanelEl.querySelectorAll('input[type=checkbox][data-k]').forEach((cb) => {
sl.dispatchEvent(new Event('input')); // refresh the value label cb.checked = !!_aspectReadVal(cb.dataset.k);
}); });
if (_aspectHfovCb) { _aspectPanelEl.querySelectorAll('input[type=range][data-k]').forEach((sl) => {
const hv = _aspectReadVal('hfovDeg'); const v = _aspectReadVal(sl.dataset.k);
_aspectHfovCb.checked = Number.isFinite(hv); if (Number.isFinite(v)) sl.value = v;
_aspectHfovSl.disabled = !_aspectHfovCb.checked; sl.dispatchEvent(new Event('input')); // refresh the value label only
if (Number.isFinite(hv)) _aspectHfovSl.value = hv; });
if (_aspectHfovCb) {
const hv = _aspectReadVal('hfovDeg');
_aspectHfovCb.checked = Number.isFinite(hv);
_aspectHfovSl.disabled = !_aspectHfovCb.checked;
if (Number.isFinite(hv)) _aspectHfovSl.value = hv;
}
} finally {
_aspectSyncing = false;
} }
} }
+28
View File
@@ -207,6 +207,34 @@ test('the target dropdown prunes dead panes and does not rebuild while focused',
'_aspectBuildTargets must skip rebuilding while the select is focused'); '_aspectBuildTargets must skip rebuilding while the select is focused');
}); });
test('programmatic sync does not write back into the tune', () => {
// _syncAspectPanel dispatches synthetic input events to refresh labels; the
// slider handler must skip the write while syncing, else opening/switching a
// target would populate a full override for every field.
assert.match(src, /_aspectSyncing\s*=\s*true[\s\S]*?finally[\s\S]*?_aspectSyncing\s*=\s*false/,
'_syncAspectPanel must set/reset the _aspectSyncing guard');
assert.match(src, /if\s*\(\s*!_aspectSyncing\s*\)\s*_aspectWriteVal\(\s*f\.k\s*,/,
'the slider input handler must skip the write while syncing');
});
test('unchecking hfov override clears a pane override key (re-inherits base)', () => {
assert.match(
src,
/function\s+_aspectClearVal\s*\(\s*k\s*\)[\s\S]*?delete\s+ov\[k\][\s\S]*?delete\s+m\[\s*_aspectEditTarget\s*\]/,
'_aspectClearVal must delete the pane override key (and empty object)',
);
assert.match(src, /else\s+_aspectClearVal\(\s*'hfovDeg'\s*\)/,
'unchecking the hfov override must call _aspectClearVal');
});
test('pruning drops the matching readout slot and a dangling __last', () => {
assert.match(
src,
/delete\s+reg\[k\]\s*;[\s\S]*?delete\s+ro\[k\]\s*;\s*if\s*\(\s*ro\.__last\s*===\s*k\s*\)\s*delete\s+ro\.__last/,
'_aspectPrunePanes must prune the readout cache alongside the registry',
);
});
test('the panel has a dismiss (close) control', () => { test('the panel has a dismiss (close) control', () => {
assert.match( assert.match(
src, src,