From 497c56a962512949d606c26ae5159621b58a5e7b Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 10 Aug 2026 08:32:36 +0200 Subject: feat(view): compute a card's geometry with no painting Split from the delegate deliberately. A delegate needs a live painter and an exposed view, which is what makes delegate tests fragile: viewport()->render() returns a blank image in several ordinary situations, and a probe reporting no ink is likelier broken than the code it tests. Every geometric claim about a card is made here, where a test is a function call. Three lines at a uniform height, so setUniformRowHeights(true) survives. Indent caps at depth 4 with qMin rather than a branch, so depth 5 and depth 50 land in the same place. The date is measured before the sender, so a long sender elides instead of painting over it. Two traps handled that a first pass gets wrong. QRect::right() is inclusive, so the right edge is carried as an exclusive one and everything sized from it lands where the padding constant says rather than a pixel short. And QFont::pointSizeF returns -1 for a font set in pixels, which qt6ct does, so smallFont branches on which unit the font actually carries instead of silently returning the card's own size. --- src/CMakeLists.txt | 1 + src/cardlayout.cpp | 119 +++++++++++++++++++++++ src/cardlayout.h | 119 +++++++++++++++++++++++ tests/CMakeLists.txt | 1 + tests/test_cardlayout.cpp | 235 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 475 insertions(+) create mode 100644 src/cardlayout.cpp create mode 100644 src/cardlayout.h create mode 100644 tests/test_cardlayout.cpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index cae3bd4..5478fab 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -5,6 +5,7 @@ add_library(qtmaildir_lib STATIC requestinterceptor.cpp htmlbuilder.cpp cidschemehandler.cpp + cardlayout.cpp notmuchworker.cpp tagchip.cpp tagcolors.cpp diff --git a/src/cardlayout.cpp b/src/cardlayout.cpp new file mode 100644 index 0000000..8d952b2 --- /dev/null +++ b/src/cardlayout.cpp @@ -0,0 +1,119 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "cardlayout.h" + +#include + +QFont CardLayout::smallFont(const QFont &cardFont) +{ + QFont small = cardFont; + // Derived from the card's font rather than fixed, so it follows the + // desktop's font size instead of shrinking to nothing on a large one. + // + // pointSizeF() returns -1 for a font set in PIXELS, which qt6ct and some + // styles do. Subtracting from -1 would ask for an invalid size and Qt + // would silently keep the original, making the small font the same size as + // the card's; the pixel branch avoids that. + if (small.pointSizeF() > 0.0) + small.setPointSizeF(qMax(6.0, cardFont.pointSizeF() - 1.0)); + else if (small.pixelSize() > 0) + small.setPixelSize(qMax(8, cardFont.pixelSize() - 1)); + return small; +} + +int CardLayout::heightFor(const QFont &font) +{ + const QFontMetrics metrics(font); + const QFontMetrics smallMetrics(smallFont(font)); + // Two lines at the card's font, one at the small one, plus the padding + // above the first and below the last. + return kPaddingY * 2 + metrics.height() * 2 + smallMetrics.height(); +} + +CardLayout CardLayout::compute(const Input &input, const QRect &rect, + const QFont &font) +{ + CardLayout out; + const QFontMetrics metrics(font); + const QFontMetrics smallMetrics(smallFont(font)); + + out.totalHeight = rect.height(); + + // The accent bar sits flush against the card's left edge, on thread cards + // only, and everything else starts after it so no text sits on the colour. + if (!input.isMessage) { + out.accentRect = + QRect(rect.left(), rect.top(), kAccentWidth, rect.height()); + } + const int textLeft = rect.left() + kAccentWidth; + + // Indent, capped. qMin rather than a branch so depth 5 and depth 50 land + // in exactly the same place. + const int depth = qMin(input.depth, kMaxDepth); + const int indent = depth * kIndentStep; + out.contentLeft = textLeft + kPaddingX + indent; + + // One spine per level actually indented, each running the card's full + // height so an expansion reads as one continuous block. + for (int level = 0; level < depth; ++level) { + const int x = textLeft + kPaddingX + level * kIndentStep + + kIndentStep / 2; + out.spines.append(QRect(x, rect.top(), 2, rect.height())); + } + + // The EXCLUSIVE right edge: one past the last pixel a card may draw on. + // QRect::right() is inclusive (left + width - 1), so building widths from + // it directly lands everything one pixel short of the intended padding. + const int right = rect.right() + 1 - kPaddingX; + const int lineOneTop = rect.top() + kPaddingY; + const int lineTwoTop = lineOneTop + metrics.height(); + const int lineThreeTop = lineTwoTop + metrics.height(); + + // The date is measured first and the sender gets what is left, so a long + // sender is elided rather than painting over the date. + const int dateWidth = metrics.horizontalAdvance( + QStringLiteral("8888-88-88 88:88")); + out.dateRect = QRect(right - dateWidth, lineOneTop, dateWidth, + metrics.height()); + out.senderRect = QRect(out.contentLeft, lineOneTop, + qMax(0, out.dateRect.left() - out.contentLeft + - kPaddingX), + metrics.height()); + + // The expander is the reply count, on line two and on the right. + if (input.replyCount > 0) { + const int countWidth = smallMetrics.horizontalAdvance( + QStringLiteral("▾ 8888 replies")); + out.expanderRect = QRect(right - countWidth, lineTwoTop, countWidth, + metrics.height()); + } + + const int subjectRight = out.expanderRect.isEmpty() + ? right + : out.expanderRect.left() - kPaddingX; + out.subjectRect = QRect(out.contentLeft, lineTwoTop, + qMax(0, subjectRight - out.contentLeft), + metrics.height()); + + out.tagRect = QRect(out.contentLeft, lineThreeTop, + qMax(0, right - out.contentLeft), + smallMetrics.height()); + + return out; +} diff --git a/src/cardlayout.h b/src/cardlayout.h new file mode 100644 index 0000000..d06ed92 --- /dev/null +++ b/src/cardlayout.h @@ -0,0 +1,119 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include +#include +#include + +/// Where everything on a card goes, with no painting and no widget. +/// +/// Split out from CardDelegate on purpose. A delegate needs a live QPainter and +/// an exposed view before it draws anything, which is what makes delegate tests +/// fragile: CLAUDE.md records that viewport()->render() returns a blank image in +/// several ordinary situations, and that a probe reporting "no ink anywhere" is +/// far more likely broken than the code it is testing. Every geometric claim +/// about a card is therefore made here, where a test is a function call. +/// +/// The card is three lines, always: +/// +/// sender ................................ date <- senderRect/dateRect +/// * subject @ v 3 replies <- subjectRect/expanderRect +/// [tag] [tag] <- tagRect +struct CardLayout +{ + /// What the model says about the row. Deliberately plain data: the layout + /// must be computable in a test without a model or a view. + struct Input + { + bool isMessage = false; + int depth = 0; ///< 0 for a thread root, 1 for a direct reply. + int replyCount = 0; ///< 0 means no expander. + }; + + /// Width of the account accent bar down a thread card's left edge. + /// + /// A starting value, not a settled one. Five accounts is enough that two + /// colours distinct as chips can read alike as thin stripes, and that can + /// only be judged against real cards on the user's own screen and theme + /// (Task 10). Widen it there if the accounts are not tellable apart. + static constexpr int kAccentWidth = 3; + + /// Horizontal breathing room at the card's edges, measured from the accent + /// bar rather than from the card, so text does not sit on the colour. + static constexpr int kPaddingX = 8; + + /// Vertical breathing room above the first line and below the last. + static constexpr int kPaddingY = 4; + + /// How far one level of reply nesting indents. + static constexpr int kIndentStep = 18; + + /// The depth past which nothing indents further. + /// + /// A mailing-list chain can nest a dozen deep, and without a cap the + /// sender is eventually pushed off the right edge. Item 20 accepted that + /// deep chains must be capped in the VIEW rather than flattened in the + /// model, and this is that cap. Rows past it draw at this depth's indent + /// with no marker saying so. + static constexpr int kMaxDepth = 4; + + QRect senderRect; + QRect dateRect; + QRect subjectRect; + QRect tagRect; + + /// The reply count's rect, and the click target that toggles the thread. + /// Empty when the row has no replies. + QRect expanderRect; + + /// The account accent bar down the card's left edge. + /// + /// Thread cards only. A reply's account is its thread's, stated once at the + /// head of the conversation, and a second vertical line in a reply's gutter + /// would sit a few pixels from the spine and compete with it. The spine + /// carries the accent instead, so an expansion is bounded by one colour + /// without ever drawing two lines. Empty on a reply. + QRect accentRect; + + /// One full-height vertical line per depth level, outermost first. + QVector spines; + + /// Where the card's text starts, after any indent. + int contentLeft = 0; + + int totalHeight = 0; + + /// The height EVERY row gets, thread and reply alike. + /// + /// Uniform by design: it keeps setUniformRowHeights(true), which is the + /// single cheapest property of this layout, since no scrolling or + /// hit-testing arithmetic has to account for rows of differing size. The + /// cost is a blank third line on a card with no tags, which was accepted + /// explicitly. + static int heightFor(const QFont &font); + + /// The font the tag chips and the reply count are drawn in: a size down + /// from the card's own, so they read as annotation rather than as a third + /// column of content. + static QFont smallFont(const QFont &cardFont); + + static CardLayout compute(const Input &input, const QRect &rect, + const QFont &font); +}; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a7bb670..4ecc04d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -41,6 +41,7 @@ add_qtmaildir_test(interceptor) add_qtmaildir_test(htmlbuilder) add_qtmaildir_test(notmuchworker) add_qtmaildir_test(tagcolors) +add_qtmaildir_test(cardlayout) add_qtmaildir_test(threadlistmodel) add_qtmaildir_test(mailsync) add_qtmaildir_test(syncmonitor) diff --git a/tests/test_cardlayout.cpp b/tests/test_cardlayout.cpp new file mode 100644 index 0000000..1082f4c --- /dev/null +++ b/tests/test_cardlayout.cpp @@ -0,0 +1,235 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "cardlayout.h" + +#include +#include + +class TestCardLayout : public QObject +{ + Q_OBJECT + +private slots: + void everyCardIsTheSameHeight(); + void threeLinesStackWithoutOverlapping(); + void replyIndentsByDepth(); + void indentStopsAtTheCap(); + void expanderSitsOnTheSecondLine(); + void expanderIsEmptyWithoutReplies(); + void dateIsFlushRight(); + void threadCardCarriesAnAccentBar(); + void replyCardCarriesNoAccentBar(); +}; + +namespace { + +CardLayout::Input threadInput() +{ + CardLayout::Input in; + in.isMessage = false; + in.depth = 0; + in.replyCount = 3; + return in; +} + +CardLayout::Input replyInput(int depth) +{ + CardLayout::Input in; + in.isMessage = true; + in.depth = depth; + in.replyCount = 0; + return in; +} + +} // namespace + +void TestCardLayout::everyCardIsTheSameHeight() +{ + const QFont font; + const int thread = CardLayout::heightFor(font); + + // The uniform height is the whole reason setUniformRowHeights(true) + // survives this design, so it is asserted directly rather than inferred + // from two cards happening to look alike. + // + // Note what is NOT varied here: the tag list. CardLayout reserves line 3 + // unconditionally and never sees the tags, which is exactly the property + // being asserted. A version of this test that passed a tag list in would + // be testing a parameter that does not exist. + const CardLayout threadCard = + CardLayout::compute(threadInput(), QRect(0, 0, 400, thread), font); + const CardLayout deepReply = + CardLayout::compute(replyInput(3), QRect(0, 0, 400, thread), font); + CardLayout::Input noRepliesIn = threadInput(); + noRepliesIn.replyCount = 0; + const CardLayout noReplies = + CardLayout::compute(noRepliesIn, QRect(0, 0, 400, thread), font); + + QCOMPARE(threadCard.totalHeight, thread); + QCOMPARE(deepReply.totalHeight, thread); + QCOMPARE(noReplies.totalHeight, thread); + + // The third line exists on every card, including one with nothing to put + // there. That blank band is the cost the uniform height was bought with. + QCOMPARE(noReplies.tagRect.height(), threadCard.tagRect.height()); +} + +void TestCardLayout::threeLinesStackWithoutOverlapping() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + const CardLayout card = + CardLayout::compute(threadInput(), QRect(0, 0, 400, h), font); + + QVERIFY(card.senderRect.height() > 0); + QVERIFY(card.subjectRect.height() > 0); + QVERIFY(card.tagRect.height() > 0); + + // Guard: these must actually be three stacked bands. A layout that + // collapsed them all to the same rect would satisfy any assertion that + // only checked they exist. + QVERIFY(card.senderRect.bottom() <= card.subjectRect.top()); + QVERIFY(card.subjectRect.bottom() <= card.tagRect.top()); + QVERIFY(card.tagRect.bottom() <= h); +} + +void TestCardLayout::replyIndentsByDepth() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + const QRect rect(0, 0, 400, h); + + const CardLayout root = CardLayout::compute(threadInput(), rect, font); + const CardLayout d1 = CardLayout::compute(replyInput(1), rect, font); + const CardLayout d2 = CardLayout::compute(replyInput(2), rect, font); + + QVERIFY(d1.contentLeft > root.contentLeft); + QVERIFY(d2.contentLeft > d1.contentLeft); + + // One spine per depth level, so the count is the depth itself. + QCOMPARE(root.spines.size(), 0); + QCOMPARE(d1.spines.size(), 1); + QCOMPARE(d2.spines.size(), 2); + + // Each spine runs the full height of the card, which is what makes an + // expansion read as one continuous block rather than as dashes. + for (const QRect &spine : d2.spines) { + QCOMPARE(spine.top(), rect.top()); + QCOMPARE(spine.bottom(), rect.bottom()); + } +} + +void TestCardLayout::indentStopsAtTheCap() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + const QRect rect(0, 0, 400, h); + + const CardLayout d4 = CardLayout::compute(replyInput(4), rect, font); + const CardLayout d5 = CardLayout::compute(replyInput(5), rect, font); + const CardLayout d9 = CardLayout::compute(replyInput(9), rect, font); + + QCOMPARE(d5.contentLeft, d4.contentLeft); + QCOMPARE(d9.contentLeft, d4.contentLeft); + QCOMPARE(d5.spines.size(), d4.spines.size()); + QCOMPARE(d9.spines.size(), d4.spines.size()); + + // Guard: the cap must not be so low that it has already bitten at depth 3, + // which would make the three assertions above true for the wrong reason. + const CardLayout d3 = CardLayout::compute(replyInput(3), rect, font); + QVERIFY(d3.contentLeft < d4.contentLeft); +} + +void TestCardLayout::expanderSitsOnTheSecondLine() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + const CardLayout card = + CardLayout::compute(threadInput(), QRect(0, 0, 400, h), font); + + QVERIFY(!card.expanderRect.isEmpty()); + // It is the reply count, so it belongs on the line the reply count is on. + QVERIFY(card.expanderRect.top() >= card.subjectRect.top()); + QVERIFY(card.expanderRect.bottom() <= card.subjectRect.bottom()); + // And it is on the right, where the count is drawn, not in a left gutter. + QVERIFY(card.expanderRect.left() > 400 / 2); +} + +void TestCardLayout::expanderIsEmptyWithoutReplies() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + CardLayout::Input in = threadInput(); + in.replyCount = 0; + + const CardLayout card = CardLayout::compute(in, QRect(0, 0, 400, h), font); + QVERIFY(card.expanderRect.isEmpty()); +} + +void TestCardLayout::dateIsFlushRight() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + const QRect rect(0, 0, 400, h); + const CardLayout card = CardLayout::compute(threadInput(), rect, font); + + // Compared as exclusive edges. QRect::right() is inclusive (left + width - + // 1), so asserting card.dateRect.right() == rect.right() - kPaddingX + // demands a gap of kPaddingX - 1 pixels and is off by one against the + // padding the constant names. + QCOMPARE(card.dateRect.right() + 1, rect.right() + 1 - CardLayout::kPaddingX); + // The sender must stop before the date starts, or a long sender overwrites + // it. This is the assertion that fails if the two are laid out + // independently. + QVERIFY(card.senderRect.right() <= card.dateRect.left()); +} + +void TestCardLayout::threadCardCarriesAnAccentBar() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + const QRect rect(0, 0, 400, h); + const CardLayout card = CardLayout::compute(threadInput(), rect, font); + + QCOMPARE(card.accentRect.left(), rect.left()); + QCOMPARE(card.accentRect.width(), CardLayout::kAccentWidth); + // Full height, so a run of cards from one account reads as a continuous + // edge rather than as dashes. + QCOMPARE(card.accentRect.top(), rect.top()); + QCOMPARE(card.accentRect.bottom(), rect.bottom()); + + // Nothing may be drawn on top of the colour. + QVERIFY(card.contentLeft >= card.accentRect.right()); +} + +void TestCardLayout::replyCardCarriesNoAccentBar() +{ + const QFont font; + const int h = CardLayout::heightFor(font); + const CardLayout reply = + CardLayout::compute(replyInput(1), QRect(0, 0, 400, h), font); + + // A reply's account is its thread's, stated once at the head. The spine + // carries the accent instead, so the gutter never holds two lines. + QVERIFY(reply.accentRect.isEmpty()); + QCOMPARE(reply.spines.size(), 1); +} + +QTEST_MAIN(TestCardLayout) +#include "test_cardlayout.moc" -- cgit v1.2.3