From d2cc33e9dec4ad78b6f727af2532db59398f9db9 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Mon, 27 Jul 2026 03:31:29 -0400 Subject: [PATCH] fix(card_drag): extend drop rects one trailing row so beyond-extent drops land computeSlotRectsForDrop adds cols slots past maxSlot; classifyCardDrag and drawCardDropTarget use it instead of the render rects. doReorderDrop already accepts any non-negative slot. New tests cover beyond-extent resolves, empty-bank trailing row, gap pixels still miss, empty-cell within-extent works. --- src/bank_panel.cpp | 20 +++++++++++-- src/card_drag.cpp | 10 +++++++ src/card_drag.h | 10 +++++++ tests/test_bank_book.cpp | 21 +++++++++++++ tests/test_card_drag.cpp | 65 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 124 insertions(+), 2 deletions(-) diff --git a/src/bank_panel.cpp b/src/bank_panel.cpp index 99bc4f2..6037411 100644 --- a/src/bank_panel.cpp +++ b/src/bank_panel.cpp @@ -1411,7 +1411,13 @@ void drawCardDropTarget(LICE_IBitmap* bmp, const RECT& region, bool isBanks, Reg const RECT grid = regionGridRect(region, isBanks); const RegionDisplay disp = regionDisplay(region, isBanks, reg); - for (const SlotCellRect& r : disp.slotRects) { + // Use the drop rects (includes the trailing row past maxSlot) so a beyond-extent + // target slot gets a visible highlight cue, not silence. + const int gridW = grid.right - grid.left; + const int maxSlot = disp.bank ? disp.bank->slots.maxSlot() : -1; + std::vector dropRects = computeSlotRectsForDrop(maxSlot, gridW, kGrid); + for (SlotCellRect& r : dropRects) { r.x += grid.left; r.y += grid.top; } + for (const SlotCellRect& r : dropRects) { if (r.slot != g_panel.dragTargetSlot) continue; if (r.y >= grid.bottom) return; // below the viewport (no scroll) const LICE_pixel hot = toLice(roleColor(Role::AccentHot)); @@ -2738,11 +2744,21 @@ void classifyCardDrag(int x, int y) { if (!destBank.empty() && destBank == g_panel.dragSourceBankId) { // Same-bank grid: a reorder/replace target. Resolve the slot the pointer sits over // in the SOURCE bank's own region display + whether it is occupied. + // Uses computeSlotRectsForDrop (one trailing row past maxSlot) so a drop beyond + // the last occupied card resolves to a valid trailing slot, not a -1 miss. mods.region = DropRegion::SameBankGrid; const bool isBanks = g_panel.dragSourceRegion == Region::Banks; const RECT region = isBanks ? banksRegionRect(w, h) : poolRegionRect(w, h); const RegionDisplay disp = regionDisplay(region, isBanks, g_panel.dragSourceRegion); - const int slot = hitTestSlot(x, y, disp.slotRects); + const RECT grid = regionGridRect(region, isBanks); + const int gridW = grid.right - grid.left; + const std::vector dropRects = + computeSlotRectsForDrop(disp.bank ? disp.bank->slots.maxSlot() : -1, + gridW, kGrid); + // Translate the drop rects to client space (matching regionDisplay's translation). + std::vector dropRectsClient = dropRects; + for (SlotCellRect& r : dropRectsClient) { r.x += grid.left; r.y += grid.top; } + const int slot = hitTestSlot(x, y, dropRectsClient); mods.targetSlot = slot; mods.slotOccupied = slot >= 0 && !disp.idAtSlot(slot).empty(); g_panel.dragTargetSlot = slot; diff --git a/src/card_drag.cpp b/src/card_drag.cpp index 2e52008..afe5971 100644 --- a/src/card_drag.cpp +++ b/src/card_drag.cpp @@ -73,6 +73,16 @@ std::vector computeSlotRects(int maxSlot, int panelWidth, return rects; } +std::vector computeSlotRectsForDrop(int maxSlot, int panelWidth, + const GridSpec& spec) { + const int cols = columnsForWidth(panelWidth, spec); + // One trailing row of slots past the last occupied slot — the drop-target extension. + // When maxSlot < 0 (empty bank) the trailing row begins at slot 0. + const int firstTrailing = maxSlot + 1; + const int newMax = firstTrailing + cols - 1; // fills one full trailing row + return computeSlotRects(newMax, panelWidth, spec); +} + int hitTestSlot(int px, int py, const std::vector& rects) { for (const SlotCellRect& r : rects) { // Half-open bounds so adjacent rects never both claim a pixel. diff --git a/src/card_drag.h b/src/card_drag.h index 2fa2128..ef0681b 100644 --- a/src/card_drag.h +++ b/src/card_drag.h @@ -128,6 +128,16 @@ struct SlotCellRect { std::vector computeSlotRects(int maxSlot, int panelWidth, const GridSpec& spec); +// Like computeSlotRects but extends one full trailing row of slots beyond maxSlot so a +// drop pointer past the last occupied card still resolves to a valid target slot. The +// trailing slots (maxSlot+1 .. maxSlot+cols) are empty — a drop on any of them calls +// reorderSample with that slot index, which places the card there directly (no shift, +// because the slot is empty). Used ONLY for drop hit-testing; the draw path uses +// computeSlotRects (no trailing ghost row in the visual). +// When maxSlot < 0 the trailing row starts at slot 0 (same as a fresh bank with no cards). +std::vector computeSlotRectsForDrop(int maxSlot, int panelWidth, + const GridSpec& spec); + // Hit-tests a point against slot rects (half-open bounds, matching hitTestCell). Returns // the SLOT index (rect.slot) of the first rect containing the point, or -1 on a miss (gap, // margin, below the last row). NOTE the return is the slot index, NOT the vector index — diff --git a/tests/test_bank_book.cpp b/tests/test_bank_book.cpp index d1c68eb..0e8674c 100644 --- a/tests/test_bank_book.cpp +++ b/tests/test_bank_book.cpp @@ -995,6 +995,26 @@ static void testReorderSampleSameSlotIsNoOp() { CHECK(book.serialize() == jsonBefore); } +// Reorder to a slot BEYOND the current maxSlot — the trailing-drop case. The model +// places the card at the exact target slot (no shift, slot is empty), leaving the card's +// old slot empty. This is the pure-layer gate for the DAW beyond-extent drop repro. +static void testReorderSampleBeyondMaxSlot() { + BankBook book; + CHECK(book.pool().index.add(sampleWith("a")) == AddResult::Added); + CHECK(book.pool().index.add(sampleWith("b")) == AddResult::Added); + book.reconcileSlots(); // a@0, b@1 -> maxSlot = 1 + CHECK(book.pool().slots.maxSlot() == 1); + + // Drag "b" to slot 5 (well beyond maxSlot=1). Should succeed: slot 5 is empty, + // no insert-shift needed, b just takes slot 5. + CHECK(book.reorderSample("id-b", kPoolBankId, 5)); + CHECK(book.pool().slots.slotOf("id-b") == 5); + CHECK(book.pool().slots.slotOf("id-a") == 0); // a untouched + // maxSlot is now 5; slot 1 is empty (gap). + CHECK(book.pool().slots.maxSlot() == 5); + CHECK(book.pool().slots.idAt(1).empty()); // b's old slot is now a gap +} + // --- BankBook L7: Alt-replace mutator ---------------------------------------- static void testReplaceSampleTakesSlotAndRemovesOccupant() { @@ -1113,6 +1133,7 @@ int main() { testOrderedSampleIdsReconcilesLazily(); testReorderSampleRejectsUnknown(); testReorderSampleSameSlotIsNoOp(); + testReorderSampleBeyondMaxSlot(); testReplaceSampleTakesSlotAndRemovesOccupant(); testReplaceSampleNonDestructiveFileStays(); testReplaceSampleRejectionsNoMutation(); diff --git a/tests/test_card_drag.cpp b/tests/test_card_drag.cpp index f0fe377..a7018f4 100644 --- a/tests/test_card_drag.cpp +++ b/tests/test_card_drag.cpp @@ -176,6 +176,66 @@ static void testHitTestSlotMiss() { CHECK(hitTestSlot(115, 15, r) == -1); // inter-cell gap between slot 0 (ends x=110) and slot 1 (starts x=120) } +// --- computeSlotRectsForDrop — beyond-extent trailing row --------------------- +// Spec: kSpec, 340px wide, 3 cols. maxSlot=2 -> render rects for 0..2. +// Drop rects extend one trailing row: slots 0..2 (existing) + 3..5 (trailing). + +static void testDropRectsExtendOneTrailingRow() { + // computeSlotRects(2, ...) gives 3 rects (0..2). Drop rects should give 6 (0..5). + const std::vector render = computeSlotRects(2, 340, kSpec); + const std::vector drop = computeSlotRectsForDrop(2, 340, kSpec); + CHECK(render.size() == 3); + CHECK(drop.size() == 6); + // First three match (existing slots preserved). + for (std::size_t i = 0; i < 3; ++i) { + CHECK(drop[i].slot == render[i].slot); + CHECK(drop[i].x == render[i].x && drop[i].y == render[i].y); + } + // Slot 3 starts the trailing row: row 1, col 0. y = gap + 1*(40+10) = 60. + CHECK(drop[3].slot == 3); + CHECK(drop[3].x == 10 && drop[3].y == 60); +} + +static void testDropRectsEmptyBankHasTrailingRow() { + // maxSlot=-1 (no occupied slots): drop rects start at slot 0 (one trailing row). + const std::vector drop = computeSlotRectsForDrop(-1, 340, kSpec); + CHECK(drop.size() == 3); // 3 cols = one trailing row + CHECK(drop[0].slot == 0); + CHECK(drop[1].slot == 1); + CHECK(drop[2].slot == 2); +} + +static void testBeyondExtentResolvesToTrailingSlot() { + // A pointer placed in the trailing row (below the last occupied card row) resolves + // to a valid slot via the drop rects, but returns -1 via the render rects — this is + // the exact gap the fix closes. + const std::vector render = computeSlotRects(2, 340, kSpec); + const std::vector drop = computeSlotRectsForDrop(2, 340, kSpec); + // Slot 3 rect: row 1, col 0. A point at (15, 65) is inside it (y=60..99, x=10..109). + CHECK(hitTestSlot(15, 65, render) == -1); // render rects: miss — bug confirmed + CHECK(hitTestSlot(15, 65, drop) == 3); // drop rects: hits trailing slot 3 — fix +} + +static void testGapPixelStillMissesInDropRects() { + // Inter-cell pixel gaps should still be misses even in the drop rects. + const std::vector drop = computeSlotRectsForDrop(4, 340, kSpec); + // x=115 is in the gap between slot 0 (x=10..109) and slot 1 (x=120..219). + CHECK(hitTestSlot(115, 15, drop) == -1); + // Top-left margin (before any cell) is still a miss. + CHECK(hitTestSlot(0, 0, drop) == -1); +} + +static void testEmptySlotWithinExtentHitsViaRenderRects() { + // Slot 1 is empty (gap), slot 0 and slot 2 are "occupied" (just position — the + // render rects cover ALL slots 0..maxSlot including empties). A pointer at slot 1's + // cell center returns slot 1 from both render AND drop rects. + const std::vector render = computeSlotRects(2, 340, kSpec); + const std::vector drop = computeSlotRectsForDrop(2, 340, kSpec); + // Slot 1: x = gap + 1*(100+10) = 120, y = gap = 10. Center: (170, 30). + CHECK(hitTestSlot(170, 30, render) == 1); // works via render rects (existing) + CHECK(hitTestSlot(170, 30, drop) == 1); // also works via drop rects +} + int main() { testNoDragIsNone(); testEmptyPayloadIsNone(); @@ -194,6 +254,11 @@ int main() { testSlotRectsDenseMatchesGrid(); testHitTestSlotReturnsSlotIndex(); testHitTestSlotMiss(); + testDropRectsExtendOneTrailingRow(); + testDropRectsEmptyBankHasTrailingRow(); + testBeyondExtentResolvesToTrailingSlot(); + testGapPixelStillMissesInDropRects(); + testEmptySlotWithinExtentHitsViaRenderRects(); if (g_fail == 0) std::printf("card_drag: all tests passed\n"); else std::printf("card_drag: %d CHECK(s) FAILED\n", g_fail);