diff --git a/App/Document/DocumentCommands.swift b/App/Document/DocumentCommands.swift index 694a5a2..548aab2 100644 --- a/App/Document/DocumentCommands.swift +++ b/App/Document/DocumentCommands.swift @@ -120,9 +120,9 @@ extension DrawingDocument { set { UserDefaults.standard.set(newValue, forKey: snapGridDefaultsKey) } } - /// ⇧⌘C opens the swatch popover. In canvas-first there is no permanent + /// ⇧⌘C opens the system colour panel. In canvas-first there is no permanent /// colour surface, so the keystroke is the guaranteed way in. - @IBAction func showColours(_ sender: Any?) { model.isColourPopoverRequested = true } + @IBAction func showColours(_ sender: Any?) { model.presentSystemColourPicker(for: .foreground) } // MARK: - Image diff --git a/App/Document/DrawingDocument.swift b/App/Document/DrawingDocument.swift index 5868c9f..667ec4f 100644 --- a/App/Document/DrawingDocument.swift +++ b/App/Document/DrawingDocument.swift @@ -143,7 +143,7 @@ final class DrawingDocument: NSDocument { let window = NSWindow(contentViewController: hosting) window.setContentSize(sized.contentSize) - window.minSize = NSSize(width: 560, height: 420) + window.minSize = Self.minimumContentSize window.title = displayName // Canvas-first: the artwork runs to all four edges, so the titlebar is @@ -188,6 +188,15 @@ final class DrawingDocument: NSDocument { addWindowController(controller) } + /// The narrowest window this app will make. + /// + /// Stated once, so the header's shed-ladder can be checked against the number + /// the window is actually built from rather than against a copy of it. It was + /// a literal at the call site, and the header needed 647pt to draw itself with + /// *no filename in it* — the app shipped a window its own chrome could not fit + /// and nothing noticed, because nothing had ever compared the two. + static let minimumContentSize = NSSize(width: 560, height: 420) + /// The window size and zoom a canvas of `size` wants. /// /// Shared by the open path and the grow path so a window opened at 1000×640 diff --git a/App/Model/EditorModel.swift b/App/Model/EditorModel.swift index 689e261..ecaba07 100644 --- a/App/Model/EditorModel.swift +++ b/App/Model/EditorModel.swift @@ -141,32 +141,19 @@ final class EditorModel { isOptionsExpanded = true } - var foreground: PaintColour { - didSet { - engine.colours.foreground = foreground - rememberColour(foreground) - } - } + var foreground: PaintColour { didSet { engine.colours.foreground = foreground } } var background: PaintColour { didSet { engine.colours.background = background } } var palette: Palette = .standard - /// Colours used recently that are not already in the fixed palette. - /// - /// The 28 swatches are muscle memory and must not move, so custom colours - /// get their own short row in the popover instead of displacing them. Kept - /// out of the rail deliberately: they arrive unpredictably, and a toolbar - /// that changes size while you work moves the button you were reaching for. - /// Most recent first. - private(set) var recentColours: [PaintColour] = [] - - private func rememberColour(_ colour: PaintColour) { - guard !palette.swatches.contains(colour) else { return } - recentColours.removeAll { $0 == colour } - recentColours.insert(colour, at: 0) - if recentColours.count > 8 { recentColours.removeLast() } - } + // **Recent colours are the system's, not ours.** This model used to keep its + // own list of eight, rendered in one place: the bespoke colour popover. That + // popover is gone — the system panel it half-copied is better at every part of + // the job — and `NSColorPanel` already carries a recently-used row that + // persists across quitting and across every app on the Mac. Ours was + // per-document and died with the window. Two lists of the same thing, and the + // one that survived is the one that survives. /// A stable identity for this model, so the shared colour panel knows which /// document currently owns it. @@ -284,8 +271,6 @@ final class EditorModel { /// Turning it on draws the grid too. Snapping you cannot see is a drag that /// disobeys you for reasons you have to guess. var snapGrid: Int = 0 { didSet { engine.settings.snapGrid = snapGrid } } - /// Set by ⇧⌘C; the cluster's colour well observes it and opens. - var isColourPopoverRequested: Bool = false var isSizeSheetPresented: Bool = false var presentedError: PresentableError? diff --git a/App/UI/CanvasOverlays.swift b/App/UI/CanvasOverlays.swift index d73c1b0..a9c353d 100644 --- a/App/UI/CanvasOverlays.swift +++ b/App/UI/CanvasOverlays.swift @@ -520,8 +520,78 @@ struct TitlebarScrim: View { /// not tools. The rail is a list of things that make marks; the clipboard acts on /// the document, like undo and zoom, and grouping it with them is what keeps the /// rail a short list you can scan. +/// **What a window of a given width can carry, and what it drops first.** +/// +/// The rail sheds palette columns before it scrolls, for the reason it states: a +/// control below the fold of an indicator-less scroll view is a control nobody +/// can tell is missing. The header had no ladder at all — it simply ran off the +/// right edge — and `DrawingDocument.minimumContentSize` is 560pt against a row +/// that needs 647pt with *no filename in it*. The app shipped a window its own +/// header could not fit. +/// +/// Every rung sheds a run whose commands have another door: a chord, a menu-bar +/// item, or a bar this app already puts on screen. Nothing that is the *only* way +/// to do something is ever shed — which inverts the obvious answer. Cut, copy and +/// paste go before the drag-out handle, because ⌘X/⌘C/⌘V are three chords every +/// Mac already has and `SelectionActions` restates two of them on screen the +/// moment there is a selection, while the drag-out handle has no chord and no menu +/// item anywhere. Undo and redo, the zoom read-out, Share and the filename are +/// never shed. +/// +/// Declaration order is the ladder, widest first, so a window that has grown +/// climbs back to the rung it fell from. +enum HeaderFit: CaseIterable { + /// `noMenus` is the rung *below* the window floor, and that is its whole job: + /// `dragOutOnly` fits 560pt with one point to spare, which is not a margin. It + /// is what the ladder falls to if the window floor ever drops or a group ever + /// gains a cell, so the failure mode is a shed control rather than a clipped + /// one. + case full, trailing, zoomReadoutOnly, shareOnly, dragOutOnly, noMenus + + var showsZoomSteppers: Bool { self == .full || self == .trailing } + var showsLastActions: Bool { showsZoomSteppers || self == .zoomReadoutOnly } + var showsClipboardRun: Bool { self != .dragOutOnly && self != .noMenus } + var showsMenus: Bool { self != .noMenus } + + var workingWidth: CGFloat { + let groups = [ + showsClipboardRun ? Tokens.Header.clipboard : Tokens.Header.dragOnly, + Tokens.Header.history, + showsMenus ? Tokens.Header.menus : nil, + showsZoomSteppers ? Tokens.Header.zoom : Tokens.Header.zoomOnly, + ].compactMap { $0 } + return groups.reduce(0, +) + CGFloat(groups.count - 1) * Tokens.Space.tight + } + + var documentWidth: CGFloat { + showsLastActions ? Tokens.Header.document : Tokens.Header.shareOnly + } + + /// The narrowest window this arrangement fits, with the filename still + /// getting `Tokens.Header.titleRoom` of it. + var minimumWindow: CGFloat { + guard self == .full else { + return Tokens.Header.surround + workingWidth + documentWidth + Tokens.Header.titleRoom + } + // Measured about the window's midline, because the centred cluster is + // centred on the *window* rather than between its neighbours: everything + // the title needs on its side of the midline is doubled. + return (Tokens.Space.comfortable * 3 + Tokens.Chrome.trafficLightClearance + + Tokens.Header.titleRoom + Tokens.Space.base + workingWidth / 2) * 2 + } + + /// The widest arrangement a window of `width` can carry. + /// + /// The last rung is returned whether it fits or not: there is nothing below + /// it, and clipping is the one outcome this ladder exists to prevent. + static func fitting(_ width: CGFloat) -> HeaderFit { + allCases.first { width >= $0.minimumWindow } ?? .noMenus + } +} + struct WorkingActions: View { @Bindable var model: EditorModel + var fit: HeaderFit = .full var body: some View { // Undo lives in the UI-free engine, so its computed flags are not @@ -531,6 +601,7 @@ struct WorkingActions: View { HStack(spacing: Tokens.Space.tight) { HeaderGroup { + if fit.showsClipboardRun { HeaderButton( symbol: "scissors", title: "Cut", shortcut: "⌘X", isEnabled: model.hasSelection @@ -561,6 +632,9 @@ struct WorkingActions: View { : "Copy an image somewhere first", isEnabled: model.canPaste ) { model.paste() } + } + // Never shed: the one control in this row with no chord and no + // menu item. Everything above it is ⌘X/⌘C/⌘V. DragOutHandle(model: model) } @@ -578,16 +652,20 @@ struct WorkingActions: View { // What the picture is, and how you are looking at it. Between the // clipboard and the history because that is the order of the work: // get something in, change it, look at it, undo it. - HeaderGroup { - ImageMenu(model: model) - ViewMenu(model: model) + if fit.showsMenus { + HeaderGroup { + ImageMenu(model: model) + ViewMenu(model: model) + } } HeaderGroup { + if fit.showsZoomSteppers { HeaderButton( symbol: "minus", title: "Zoom out", shortcut: "⌘−", isEnabled: model.zoom > (EditorModel.zoomSteps.first ?? 1) ) { model.zoomOut() } + } // **The percentage is the button.** There used to be a separate // `arrow.up.left.and.arrow.down.right` cell for Actual Size, @@ -597,15 +675,20 @@ struct WorkingActions: View { // *full screen*, so the pair managed to be both redundant and // misleading. What is left is a real button whose label is the // current zoom and whose action is 100%. + // Never shed: the only place the window states its zoom, and the + // Actual Size button. The two steppers either side of it have + // ⌘+, ⌘−, pinch, ⌘-scroll and the View menu. ZoomReadout(label: zoomLabel, isActualSize: model.zoom == 1) { model.hasUserZoomed = true model.zoom = 1 } + if fit.showsZoomSteppers { HeaderButton( symbol: "plus", title: "Zoom in", shortcut: "⌘+", isEnabled: model.zoom < (EditorModel.zoomSteps.last ?? 1) ) { model.zoomIn() } + } } } } @@ -624,12 +707,14 @@ struct WorkingActions: View { /// press forty times an hour. struct DocumentActions: View { @Bindable var model: EditorModel + var fit: HeaderFit = .full @Environment(TooltipController.self) private var tooltips @State private var shareFrame: CGRect = .zero var body: some View { HeaderGroup { + if fit.showsLastActions { // **Signing has a button now.** It lived only in the Tools menu // under ⌃⌘S — a chord nothing else in the app uses — which made the // one feature nobody would guess at the one feature the window never @@ -645,6 +730,7 @@ struct DocumentActions: View { } HeaderDivider() + } // Share is in the File menu, but a markup app's whole purpose is // getting the result to someone else — burying its most common last @@ -666,6 +752,11 @@ struct DocumentActions: View { ) : tooltips.endHover(key: "header-share") } + + // Everything after Share is the last two minutes of a session, and + // every one of them has a chord and a menu-bar item. Share does not + // go: getting the result to someone else is what this app is for. + if fit.showsLastActions { HeaderButton( symbol: "doc.on.doc", title: "Duplicate", shortcut: "⇧⌘S", detail: "Opens a copy in a new window" @@ -689,6 +780,7 @@ struct DocumentActions: View { ) { AppCommandsBridge.openGuide() } + } } } } @@ -947,7 +1039,7 @@ struct ViewMenu: View { model.chromeEdge = model.chromeEdge.toggled } Divider() - Button("Colours…") { model.isColourPopoverRequested = true } + Button("Colours…") { model.presentSystemColourPicker(for: .foreground) } } } diff --git a/App/UI/ColourPopover.swift b/App/UI/ColourPopover.swift deleted file mode 100644 index cb358a7..0000000 --- a/App/UI/ColourPopover.swift +++ /dev/null @@ -1,159 +0,0 @@ -import PaintKit -import SwiftUI - -/// The 28-swatch grid, on demand from the colour well. -/// -/// The pair is restated at full size *above* the grid so the mapping from -/// "front / back" to the two chips is unambiguous — the compact overlapping -/// pair in the cluster is legible but not self-explanatory, and this is where -/// it gets explained. -/// -/// Swatch selection uses a ring **inside** the cell, drawn in the material's -/// own label colour rather than the accent, because the accent already means -/// "selected tool" everywhere else in this chrome. An outer ring would also -/// shift every neighbour by a pixel as selection moved. -struct ColourPopover: View { - @Bindable var model: EditorModel - - private static let columns = 14 - - var body: some View { - VStack(alignment: .leading, spacing: Tokens.Space.snug) { - header - - grid(model.palette.swatches, cell: { swatch($0) }) - .padding(Tokens.Space.tight) - .background { - RoundedRectangle(cornerRadius: Tokens.Radius.well, style: .continuous) - .fill(.primary.opacity(Tokens.Fill.cell)) - } - - if !model.recentColours.isEmpty { - Text("Recent") - .font(Tokens.Text.popoverHint) - .foregroundStyle(.primary.opacity(Tokens.Ink.faint)) - HStack(spacing: Tokens.Space.hair) { - ForEach(Array(model.recentColours.enumerated()), id: \.offset) { _, colour in - swatch(colour) - } - } - } - - Button { - model.presentSystemColourPicker(for: .foreground) - } label: { - HStack(spacing: Tokens.Space.tight) { - Image(systemName: "eyedropper.halffull").font(.system(size: 10)) - Text("Other colour…") - } - .font(Tokens.Text.popoverHint) - .foregroundStyle(.primary.opacity(Tokens.Ink.regular)) - .contentShape(Rectangle()) - } - .buttonStyle(.plain) - } - .padding(Tokens.Space.comfortable - 2) - .frame(width: 340) - } - - private var header: some View { - HStack(spacing: Tokens.Space.snug) { - ZStack(alignment: .topLeading) { - largeChip(model.background) - .offset(x: 11, y: 9) - largeChip(model.foreground) - } - .frame( - width: Tokens.Size.colourPairLarge + 11, - height: Tokens.Size.colourPairLarge + 9 - ) - - VStack(alignment: .leading, spacing: 1) { - Text("Front \(model.foreground.hexString) · Back \(model.background.hexString)") - .font(Tokens.Text.popoverTitle) - Text("X swaps · ⇧⌘C opens this") - .font(Tokens.Text.popoverHint) - .foregroundStyle(.primary.opacity(Tokens.Ink.muted)) - } - - Spacer(minLength: 0) - - Button { - model.swapColours() - } label: { - Image(systemName: "arrow.triangle.2.circlepath") - .font(.system(size: 11, weight: .medium)) - .frame(width: 22, height: 22) - .contentShape(Rectangle()) - } - .buttonStyle(.plain) - .help("Swap front and back (X)") - .accessibilityLabel("Swap front and back colours") - } - } - - private func grid( - _ colours: [PaintColour], @ViewBuilder cell: @escaping (PaintColour) -> Cell - ) -> some View { - let rows = stride(from: 0, to: colours.count, by: Self.columns).map { - Array(colours[$0.. some View { - let isFront = colour == model.foreground - let isBack = colour == model.background - - return Button { - model.applySwatch(colour, to: .foreground) - } label: { - RoundedRectangle(cornerRadius: Tokens.Radius.swatch, style: .continuous) - .fill(Color(cgColor: colour.cgColor)) - .frame(width: Tokens.Size.swatch, height: Tokens.Size.swatch) - .overlay { - // Inset hairline: on glass, a white or near-white swatch - // would otherwise have no edge at all. - RoundedRectangle(cornerRadius: Tokens.Radius.swatch, style: .continuous) - .strokeBorder(.black.opacity(0.35), lineWidth: 0.5) - } - .overlay { - if isFront || isBack { - RoundedRectangle(cornerRadius: Tokens.Radius.swatch - 1.5, style: .continuous) - .strokeBorder( - colour.prefersDarkContrast ? Color.black : Color.white, - lineWidth: isFront ? 2 : 1 - ) - .padding(2) - } - } - .contentShape(Rectangle()) - } - .buttonStyle(.plain) - .help(colour.hexString) - .accessibilityLabel("Colour \(colour.hexString)") - .accessibilityAddTraits(isFront ? [.isSelected, .isButton] : .isButton) - .contextMenu { - Button("Set as front") { model.applySwatch(colour, to: .foreground) } - Button("Set as back") { model.applySwatch(colour, to: .background) } - } - } - - private func largeChip(_ colour: PaintColour) -> some View { - RoundedRectangle(cornerRadius: Tokens.Radius.segmentInner, style: .continuous) - .fill(Color(cgColor: colour.cgColor)) - .frame(width: Tokens.Size.colourPairLarge, height: Tokens.Size.colourPairLarge) - .overlay { - RoundedRectangle(cornerRadius: Tokens.Radius.segmentInner, style: .continuous) - .strokeBorder(.black.opacity(0.35), lineWidth: 0.5) - } - } -} diff --git a/App/UI/DesignTokens.swift b/App/UI/DesignTokens.swift index f9d7355..28020a2 100644 --- a/App/UI/DesignTokens.swift +++ b/App/UI/DesignTokens.swift @@ -5,8 +5,8 @@ import SwiftUI /// /// With nothing selected the artwork owns ~96% of the window. All chrome is a /// single glass cluster at the bottom, an options pill that trails it, a title -/// chip, and a colour popover on demand. Nothing is permanently docked to an -/// edge, and nothing is ever placed on the artwork itself. +/// chip, and the system colour panel on demand. Nothing is permanently docked +/// to an edge, and nothing is ever placed on the artwork itself. /// /// Three rules govern everything below. /// @@ -19,7 +19,8 @@ import SwiftUI /// selected one and nothing else paints an edge. /// /// **Elevation is the one thing this direction spends.** Exactly two levels: -/// the cluster and the popover sit at the same height, and nothing else lifts. +/// the cluster and the options panel sit at the same height, and nothing else +/// lifts. enum Tokens { // MARK: - Spacing @@ -44,10 +45,14 @@ enum Tokens { static let segment: CGFloat = 21 /// The options panel's label column. /// - /// Fixed, so "Size", "Stroke", "Flow" and "Corner" all end at the same - /// x and every control in the panel starts at the same x. This one - /// number is the difference between a panel and a stack of unrelated - /// rows that happen to share a background. + /// Fixed, so "Tip", "Fill", "Line" and "Align" all end at the same x and + /// every control beside them starts at the same x. This one number is the + /// difference between a panel and a stack of unrelated rows that happen to + /// share a background. + /// + /// Continuous values do not use it: a `Mark` carries its own name on a + /// caption line and takes the panel's whole width for its track. See + /// `OptionRow`, which states the rule and why it is two rules. static let optionLabel: CGFloat = 42 /// `GeometryReader` offers only 10pt intrinsically, which is not enough /// travel for a size mark when the bottom panel measures its own row. @@ -63,14 +68,19 @@ enum Tokens { static let colourSwap: CGFloat = 18 /// Glyph inside a tool cell — optically sized, not mathematically. static let toolGlyph: CGFloat = 19 - /// The same pair restated at full size inside the popover. - static let colourPairLarge: CGFloat = 28 /// A swatch in the always-visible palette. static let swatch: CGFloat = 16 /// Every capsule in the top header shares one optical height. static let headerControl: CGFloat = 32 /// One square in the transparency grid behind the artwork. static let transparencyTile: CGFloat = 8 + /// The same grid, at chip scale. + /// + /// Half, stated as a relationship rather than as a second magic 4: a chip + /// has to read as the *same kind of nothing* the artwork does. At the + /// canvas's own 8pt an entire 24pt colour well is three squares across, + /// which reads as three grey blocks and not as pattern at all. + static let transparencyTileChip: CGFloat = transparencyTile / 2 } // MARK: - Radii @@ -84,7 +94,7 @@ enum Tokens { /// The options panel: tighter than the rail, so it reads as attached to /// it rather than as a second window floating nearby. static let panel: CGFloat = 12 - /// The well a grid sits in, inside the popover. + /// The well a grid sits in. static let well: CGFloat = 10 static let segmentTrack: CGFloat = 7 static let segmentInner: CGFloat = 5 @@ -230,7 +240,7 @@ enum Tokens { /// /// A thin rail spends its length on tools, and the full fourteen pairs /// would run past the bottom of the window on a laptop screen. The rest - /// are one click away in the colour popover, and the pairs kept are the + /// are one press away in the system colour panel, and the pairs kept are the /// leading ones, so a swatch is where it always was. /// /// Six rather than seven since Clone joined the Draw run. A thirteenth @@ -281,6 +291,57 @@ enum Tokens { static let optionsContentWidth: CGFloat = optionsWidth - Space.snug * 2 } + // MARK: - The header row + + /// **What the header costs, derived from the cells it is built from.** + /// + /// Derived, not written down. `WorkingActions` has gained three buttons since + /// the row was designed, and a literal here would go stale on the commit that + /// adds the fourth — silently, and in the direction that clips. + enum Header { + /// A `HeaderButton`, a `ShareButton`, a `DragOutHandle`. + static let cell: CGFloat = 26 + /// A `HeaderMenu`: glyph plus chevron. + static let menu: CGFloat = 34 + /// `ZoomReadout`'s floor. + static let readout: CGFloat = 40 + /// A `HeaderDivider`: a 1pt rule with 2pt either side. + static let divider: CGFloat = 5 + static let cellGap: CGFloat = 1 + /// `HeaderGroup`'s horizontal padding, both sides. + static let groupPad: CGFloat = 6 + + /// One capsule holding these cells. + static func group(_ cells: CGFloat...) -> CGFloat { + cells.reduce(0, +) + CGFloat(cells.count - 1) * cellGap + groupPad + } + + static let clipboard = group(cell, cell, cell, cell) + static let dragOnly = group(cell) + static let history = group(cell, cell) + static let menus = group(menu, menu) + static let zoom = group(cell, readout, cell) + static let zoomOnly = group(readout) + static let document = group(cell, divider, cell, cell, divider, cell) + static let shareOnly = group(cell) + + /// Everything in the row that is not a control: its own padding, the + /// traffic lights, the spacer holding the title off the cluster, and the + /// four gaps between the five things in it. + static let surround = Space.comfortable * 2 + Chrome.trafficLightClearance + + Space.base + Space.comfortable * 4 + + /// Room kept for the filename before the next control is shed. + /// + /// **The number with its evidence.** The name this app sees most is not + /// `Untitled.png`, it is `Screenshot 2026-08-26 at 14.02.11.png` — about + /// 250pt at 13pt semibold, which no narrow window can show whole. 140pt is + /// what middle-truncation needs before it stops carrying information: + /// `Screenshot 20…14.02.11.png`, not `S…png`. Below it, truncating says + /// nothing and the row should be shedding a control instead. + static let titleRoom: CGFloat = 140 + } + // MARK: - Chrome geometry /// Fixed dimensions of the floating chrome, so a canvas fit can be derived @@ -362,7 +423,7 @@ struct SelectedSegmentFill: View { // MARK: - The chrome material /// The one surface treatment shared by the cluster, the pill, the title chip -/// and the popover. +/// and the options panel. /// /// **The glass never relies on the artwork behind it.** A fixed tint floor sits /// over the blur, so label contrast is constant whether the chrome straddles diff --git a/App/UI/EditorView.swift b/App/UI/EditorView.swift index dad64d6..95f5b03 100644 --- a/App/UI/EditorView.swift +++ b/App/UI/EditorView.swift @@ -25,8 +25,9 @@ struct EditorView: View { /// The panel's measured length along the rail's axis, so it can be kept /// inside the window without guessing how tall its content is. @State private var panelSpan: CGFloat = 0 - /// Width of the header's tooltip chip, so it can be kept inside the window. - @State private var headerTooltipWidth: CGFloat = 0 + /// The tooltip chip's measured size, so it can be kept inside the window on + /// both axes. + @State private var tooltipSize: CGSize = .zero var body: some View { ZStack { @@ -52,7 +53,7 @@ struct EditorView: View { titleRow .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) - headerTooltip + tooltipLayer // **Above the tool chrome, which is drawn after it.** Share is // the one control in this row that is an NSViewRepresentable // rather than a SwiftUI Button — it hosts a real NSButton so its @@ -248,19 +249,6 @@ struct EditorView: View { .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .bottomLeading) } } - .overlay(alignment: isVertical ? .topLeading : .bottomLeading) { - // Only for controls that have no anchor of their own — the rail's, - // whose geometry this offset already knows. A header button carries - // its position with it and is drawn by `headerTooltip` instead. - if tooltips.isVisible, tooltips.anchor == nil { - Tooltip(title: tooltips.title, shortcut: tooltips.shortcut, detail: tooltips.detail) - .offset( - x: isVertical ? Tokens.Rail.thickness + Tokens.Space.snug : 0, - y: isVertical ? Tokens.Chrome.titleReserve : -Tokens.Rail.thickness - ) - .allowsHitTesting(false) - } - } .animation(Tokens.Motion.pillResize, value: anchoredIndex) .animation(Tokens.Motion.pillResize, value: model.isOptionsExpanded) .animation(Tokens.Motion.micro, value: tooltips.visibleKey) @@ -298,50 +286,41 @@ struct EditorView: View { max(0, min(panelOffset, available - panelSpan)) } - /// The rail, shedding swatches before it ever scrolls. + /// The rail, shedding swatches before it ever scrolls — and scrolling only + /// the part that can afford to be scrolled. /// - /// A short window used to clip the rail: the colour block was cut in half - /// and the edge toggle was simply gone, below the fold of a scroll view - /// with hidden indicators — so there was no way to tell that anything was - /// missing, let alone reach it. **The tools and the loaded colours are the - /// part that must never be cut**, and the palette is the part that can give - /// ground, because every swatch it drops is still in the popover. + /// A short window used to clip the rail: the colour block was cut in half and + /// the edge toggle was simply gone, below the fold of a scroll view with + /// hidden indicators — so there was no way to tell anything was missing, let + /// alone reach it. **The tools and the loaded colours are the part that must + /// never be cut**, and the palette is the part that can give ground. /// - /// Only once the palette is gone entirely and the window is still too short - /// does it fall back to scrolling. + /// So there are two answers here, in order. The palette drops columns first, + /// because every swatch it drops is one press away in the system picker. Then, + /// if the tools *still* do not fit, the tool run alone scrolls — inside the + /// rail, with a real scroller — while the colour block and the edge toggle + /// keep their place at the end of it. Wrapping the whole rail was what put the + /// colours below the fold in the first place. + /// **Both edges, one rule.** The bottom bar used to be told it always had all + /// fourteen palette columns and never needed to scroll, which is true of a + /// 1400pt window and nonsense at 560 — the colour block ran straight off the + /// right-hand end. The side rail's shedding was already written; the bottom + /// bar simply never called it. @ViewBuilder private func scrollingRail(maxLength: CGFloat, isVertical: Bool) -> some View { - ScrollView(isVertical ? .vertical : .horizontal, showsIndicators: false) { - ToolRail( - model: model, - swatchPairs: isVertical ? Self.swatchPairs(fitting: maxLength) : Palette.columns - ) { model.selectTool($0) } - .fixedSize() - } - .frame( - maxWidth: isVertical ? Tokens.Rail.thickness : maxLength, - maxHeight: isVertical ? maxLength : Tokens.Rail.thickness + 2 - ) - .fixedSize(horizontal: isVertical, vertical: !isVertical) - } - - /// How many columns of the palette the side rail can show in `height`. - /// - /// Everything above the palette is fixed, so this is one subtraction rather - /// than a layout pass — which matters, because the answer feeds a view that - /// is being laid out. - private static func swatchPairs(fitting height: CGFloat) -> Int { - let cell = Tokens.Size.toolCell + Tokens.Space.hair - let tools = ToolKind.groups.reduce(CGFloat.zero) { $0 + CGFloat($1.count) * cell } - let separators = CGFloat(ToolKind.groups.count) * (1 + Tokens.Rail.sectionSpacing * 2) - let pair = Tokens.Size.colourWell * 1.42 + Tokens.Space.hair + Tokens.Size.colourSwap - let toggle = Tokens.Rail.sectionSpacing + Tokens.Size.toolCell - let fixed = Tokens.Rail.padding * 2 + tools + separators - + pair + Tokens.Space.tight + Tokens.Rail.colourInset * 2 + toggle - - let forSwatches = height - fixed - let row = Tokens.Size.swatch + Tokens.Rail.swatchGap - return max(0, min(Tokens.Rail.swatchPairs, Int(forSwatches / row))) + let pairs = RailFit.swatchPairs(fitting: maxLength, isVertical: isVertical) + let room = RailFit.toolRoom(in: maxLength, pairs: pairs, isVertical: isVertical) + + ToolRail( + model: model, + swatchPairs: pairs, + toolsLength: room + ) { model.selectTool($0) } + .frame( + maxWidth: isVertical ? Tokens.Rail.thickness : maxLength, + maxHeight: isVertical ? maxLength : Tokens.Rail.thickness + 2, + alignment: isVertical ? .top : .leading + ) } /// Distance from the rail's leading edge to the selected cell, so the panel @@ -402,11 +381,11 @@ struct EditorView: View { /// the same trick the rail uses when it drops palette columns. private var titleRow: some View { GeometryReader { proxy in - let hasRoomToCentre = proxy.size.width >= Self.centredClusterMinimum + let fit = HeaderFit.fitting(proxy.size.width) ZStack { - if hasRoomToCentre { - WorkingActions(model: model) + if fit == .full { + WorkingActions(model: model, fit: fit) .fixedSize() } @@ -422,13 +401,21 @@ struct EditorView: View { // The title yields before the controls do. A long filename // should truncate in the middle, not push Undo off the window. .layoutPriority(-1) + // **A ceiling, not just a priority.** Centred, the cluster is + // not in this stack at all — so nothing stopped a long + // filename being handed the whole row and drawn straight over + // the zoom controls, since the `HStack` is painted after the + // cluster. Priority decides who yields when the row is short; + // this is what stops the title crossing the midline when it is + // long. + .frame(maxWidth: Self.titleCeiling(fit, in: proxy.size.width), alignment: .leading) Spacer(minLength: Tokens.Space.base) - if !hasRoomToCentre { - WorkingActions(model: model).fixedSize() + if fit != .full { + WorkingActions(model: model, fit: fit).fixedSize() } - DocumentActions(model: model).fixedSize() + DocumentActions(model: model, fit: fit).fixedSize() } } .padding(.horizontal, Tokens.Space.comfortable) @@ -437,29 +424,40 @@ struct EditorView: View { .frame(height: Tokens.Chrome.titleReserve) } - /// Traffic lights, a title worth reading, the cluster, and the document - /// actions, with the gaps between them. Below this the cluster overlaps. - private static let centredClusterMinimum: CGFloat = 940 + /// How wide the filename may be before it reaches the centred cluster. + static func titleCeiling(_ fit: HeaderFit, in width: CGFloat) -> CGFloat { + guard fit == .full else { return .infinity } + return max( + Tokens.Header.titleRoom, + width / 2 - fit.workingWidth / 2 + - Tokens.Chrome.trafficLightClearance + - Tokens.Space.comfortable * 3 + - Tokens.Space.base + ) + } - /// The chip for a header control, on one fixed line under the header. + /// **One chip, for every control in the window.** /// - /// The header's buttons cannot draw it themselves: their group clips to its - /// own capsule, so a chip inside one is a chip with its bottom half cut off. - /// They report where they are instead and it is drawn out here, still one - /// chip at a time — the rail's chip is suppressed while a header button owns - /// the tooltip, because two of them on screen is two answers to one question. + /// Nothing draws its own. A header button's group clips to its own capsule, + /// so a chip inside one is a chip with its bottom half cut off; a rail cell + /// sits inside a `ScrollView`, which is worse. Every control reports where it + /// is and the single chip is drawn out here, over everything, clipped by + /// nothing. /// - /// **The chip's top edge does not move.** It used to be positioned from each - /// control's own frame, so a chip with a second line grew *downwards from a - /// different y* than a one-line chip beside it, and reading along the header - /// meant re-finding the text on every cell. Every header control is on one - /// row and the answer belongs on one line under it. Only the horizontal - /// centre follows the glyph, so the chip still points at what it names. + /// It used to be two layers with two rules, and the rail's was not really a + /// rule: it offset the chip by a constant, so hovering the eyedropper — nine + /// cells down — put the answer up beside the pencil. A tooltip that does not + /// point at what it names is a tooltip you have to work out. @ViewBuilder - private var headerTooltip: some View { + private var tooltipLayer: some View { GeometryReader { proxy in - if tooltips.isVisible, let anchor = tooltips.anchor { + if tooltips.isVisible { + let anchor = tooltips.anchor let container = proxy.frame(in: .global) + let origin = Self.tooltipOrigin( + beside: anchor, in: container, + chip: tooltipSize, rail: model.chromeEdge + ) Tooltip( title: tooltips.title, shortcut: tooltips.shortcut, @@ -469,23 +467,24 @@ struct EditorView: View { .background { GeometryReader { chip in Color.clear - .onAppear { headerTooltipWidth = chip.size.width } - .onChange(of: chip.size.width) { _, new in - headerTooltipWidth = new - } + .onAppear { tooltipSize = chip.size } + .onChange(of: chip.size) { _, new in tooltipSize = new } } } // `.topLeading`, not `.position`: `position` centres on a point, // so a two-line chip and a one-line chip pinned to the same y - // still start at different heights. Pinning the top edge is what + // still start at different heights. Pinning a corner is what // "one fixed line" actually means. .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) - .offset( - x: Self.tooltipLeading( - under: anchor, in: container, chipWidth: headerTooltipWidth - ), - y: Self.tooltipTop - ) + .offset(x: origin.x, y: origin.y) + // **Measured before it is seen.** `tooltipSize` starts at zero and + // now feeds the *y* axis too: above a bottom bar the origin is + // `height − chip.height`, so an unmeasured chip pinned its top-left + // to the top edge of the bar and then jumped its own height — about + // 60pt — the instant the measurement landed. It has to be laid out + // to be measured, so it is laid out invisibly for that one frame + // rather than skipped, which would measure nothing forever. + .opacity(tooltipSize == .zero ? 0 : 1) .allowsHitTesting(false) } } @@ -495,6 +494,49 @@ struct EditorView: View { /// the same gap the header itself keeps from the top of the window. static let tooltipTop: CGFloat = Tokens.Chrome.titleReserve - Tokens.Space.tight + /// The chip's top-left corner, given where the control is. + /// + /// **Two placements, one rule.** The chip's near edge sits on a fixed line + /// beside the chrome it explains — one line under the header, one column + /// clear of the rail — and it slides *along* that line to follow the control. + /// Reading across the header moves the answer sideways only; reading down the + /// rail moves it up and down only. The text is never somewhere new. + /// + /// Which line it hangs from is read off the anchor rather than passed in by + /// every call site: the header band is the only chrome above the artwork, so + /// a control inside it is a header control and everything else is the rail's. + static func tooltipOrigin( + beside anchor: CGRect, in container: CGRect, + chip: CGSize, rail edge: EditorModel.ChromeEdge + ) -> CGPoint { + let alongTop = tooltipLeading(under: anchor, in: container, chipWidth: chip.width) + + guard anchor.midY - container.minY >= Tokens.Chrome.titleReserve else { + return CGPoint(x: alongTop, y: tooltipTop) + } + + guard edge.isVertical else { + return CGPoint( + x: alongTop, + y: container.height - Tokens.Chrome.railInset + - Tokens.Rail.thickness - Tokens.Space.snug - chip.height + ) + } + + return CGPoint( + x: Tokens.Chrome.railInset + Tokens.Rail.thickness + Tokens.Space.snug, + y: slid( + toward: anchor.midY - container.minY, + extent: chip.height, + within: container.height, + // Clear of the header at one end and of the window at the other: + // the two things a chip beside a tall rail can run into. + leading: Tokens.Chrome.titleReserve, + trailing: Tokens.Space.safeInset + ) + ) + } + /// Where the header's chip starts: centred under the control, unless that /// would hang it off an edge of the window. /// @@ -507,15 +549,32 @@ struct EditorView: View { /// its top-left corner, so that its top edge can stay on one line whatever /// height it happens to be. static func tooltipLeading(under anchor: CGRect, in container: CGRect, chipWidth: CGFloat) -> CGFloat { - let half = chipWidth / 2 - let inset = half + Tokens.Space.base - let wanted = anchor.midX - container.minX - // A chip wider than the window cannot be kept inside it; centre it and - // let both ends run out rather than pinning it to one edge. - let centre = container.width > inset * 2 - ? min(max(wanted, inset), container.width - inset) - : container.width / 2 - return centre - half + slid( + toward: anchor.midX - container.minX, + extent: chipWidth, + within: container.width, + leading: Tokens.Space.base, + trailing: Tokens.Space.base + ) + } + + /// Centre `extent` on `wanted`, then pull it back inside the window. + /// + /// One function for both axes, because "point at the control, but stay on + /// screen" is one idea and it had drifted into being two — the rail's version + /// simply did not clamp, which is how a chip beside the last tool in a short + /// window ended up below the bottom of it. + private static func slid( + toward wanted: CGFloat, extent: CGFloat, within: CGFloat, + leading: CGFloat, trailing: CGFloat + ) -> CGFloat { + let low = leading + let high = within - trailing - extent + // Something larger than the room it has cannot be kept inside it; centre + // it and let both ends run out rather than pinning one edge and cutting + // the whole sentence off the other. + guard high > low else { return (within - extent) / 2 } + return min(max(wanted - extent / 2, low), high) } private var readOutRow: some View { diff --git a/App/UI/Mark.swift b/App/UI/Mark.swift index 16b6a7a..b2a0b92 100644 --- a/App/UI/Mark.swift +++ b/App/UI/Mark.swift @@ -2,18 +2,24 @@ import SwiftUI /// A continuous value, shown as the thing it controls. /// -/// **Replaces `OptionRow` + `OptionSlider` + the size-stop capsules for tool options.** -/// That stack was four pieces saying one word: a 42pt right-aligned label, a stock -/// `Slider` at `.mini`, a trailing number, and a second row of capsules for the same -/// integer. Once one property is a label plus a grey pill, every property is a label -/// plus a grey pill, and after three rows the panel is one grey surface with no way to -/// tell which tool you are holding. +/// **Every continuous value in the tool options panel is one of these.** +/// +/// What it replaced was four pieces saying one word: a 42pt right-aligned label, a +/// stock `Slider` at `.mini`, a trailing number, and a second row of capsules for the +/// same integer. Once one property is a label plus a grey pill, every property is a +/// label plus a grey pill, and after three rows the panel is one grey surface with no +/// way to tell which tool you are holding. /// /// The rule this is built to: cover the panel's title and you should still know the /// tool. So the size control carries a wedge that gets thicker to the right, the /// highlighter's opacity control *is* a wash of the current ink, and a meter that only -/// reports a number recedes rather than competing. The property names itself, and the -/// word moves to VoiceOver where a name is worth having. +/// reports a number recedes rather than competing. +/// +/// The name **is** drawn, on the caption line beside the value. It was not, on the +/// argument that the property names itself — which holds for a yellow wash and does +/// not hold for the five plain meters this control also draws, where a grey trough +/// over the number `16` says nothing about whether that is a tolerance, a block size +/// or a corner radius. /// /// One `DragGesture(minimumDistance: 0)` over a `GeometryReader` rather than `Slider`, /// because the AppKit knob is the giveaway that nobody looked at this panel. @@ -31,7 +37,7 @@ struct Mark: View { @Binding var value: Double let range: ClosedRange let style: Style - /// The label VoiceOver reads. Deliberately not drawn. + /// Drawn at the head of the caption line, and read by VoiceOver. let name: String var gamma: Double = 1 var readout: String @@ -41,48 +47,100 @@ struct Mark: View { @State private var isHovering = false @State private var isDragging = false @Environment(\.colorSchemeContrast) private var contrast + @Environment(\.optionsAxisIsVertical) private var isVertical private let trackHeight: CGFloat = 8 private let hitHeight: CGFloat = 16 - /// Position on the track, 0...1, through the response curve. - private var t: Double { + // MARK: - The curve + + /// Where a value sits on the track, 0...1, through the response curve. + /// + /// Static and public to the module so the mapping can be checked without a + /// view: it is arithmetic, it is what the control is *for*, and it was + /// previously three private copies of two lines that had already drifted — + /// `fraction(of:)` clamped and `t` did not. + static func fraction(of value: Double, in range: ClosedRange, gamma: Double) -> Double { let span = range.upperBound - range.lowerBound guard span > 0 else { return 0 } - let linear = (value - range.lowerBound) / span - return gamma == 1 ? linear : pow(min(max(linear, 0), 1), 1 / gamma) + let linear = min(max((value - range.lowerBound) / span, 0), 1) + return gamma == 1 ? linear : pow(linear, 1 / gamma) } - private func value(atFraction f: Double) -> Double { + static func value(atFraction f: Double, in range: ClosedRange, gamma: Double) -> Double { let clamped = min(max(f, 0), 1) let curved = gamma == 1 ? clamped : pow(clamped, gamma) return range.lowerBound + curved * (range.upperBound - range.lowerBound) } + /// Position on the track, 0...1. + /// + /// **Clamped, which it was not.** A model value can legitimately sit outside a + /// mark's own range — `ToolSettings` lets the spray's density reach 1 while + /// the Flow mark stops at 0.6 — and the meter's fill is an unclipped overlay, + /// so an out-of-range value painted straight past the end of its own trough. + /// The gamma path clamped; the linear path did not, and every mark that does + /// not bend its travel takes the linear path. + private var t: Double { Self.fraction(of: value, in: range, gamma: gamma) } + + private func value(atFraction f: Double) -> Double { + Self.value(atFraction: f, in: range, gamma: gamma) + } + private func fraction(of v: Double) -> Double { - let span = range.upperBound - range.lowerBound - guard span > 0 else { return 0 } - let linear = (v - range.lowerBound) / span - return gamma == 1 ? linear : pow(min(max(linear, 0), 1), 1 / gamma) + Self.fraction(of: v, in: range, gamma: gamma) } var body: some View { VStack(alignment: .trailing, spacing: Tokens.Space.hair) { - Text(readout) - .font(.system(size: 11, weight: .medium).monospacedDigit()) - .foregroundStyle(.primary.opacity(isDragging ? Tokens.Ink.strong : Tokens.Ink.regular)) - .frame(height: 14) - .animation(Tokens.Motion.micro, value: isDragging) + // **The name, on the line the readout was already using.** + // + // `Mark`'s original claim was that the property names itself and the + // word belongs in VoiceOver. That holds for the wash — a yellow track + // is a highlighter and nothing else — and it does not hold for the + // five plain meters this control now also draws: a grey trough over + // the number `16` says nothing about whether that is a tolerance, a + // block size or a corner radius. The panel has already had the "which + // Size is this one" problem badly enough to rename a row to "Area". + // + // It costs the left half of a line that was empty, and it lets these + // marks leave `OptionRow`'s 42pt label column entirely — so every + // track in the panel now starts and ends at the same x, which is the + // alignment this panel exists to have. + HStack(spacing: Tokens.Space.tight) { + Text(name) + // The same size as the value beside it. The panel's label font + // is 11.5 and the readout is 11, and two sizes on one line is + // exactly the accidental look this control was built to stop. + .font(.system(size: 11)) + .foregroundStyle(.primary.opacity(Tokens.Ink.muted)) + Spacer(minLength: Tokens.Space.tight) + Text(readout) + .font(.system(size: 11, weight: .medium).monospacedDigit()) + .foregroundStyle(.primary.opacity(isDragging ? Tokens.Ink.strong : Tokens.Ink.regular)) + } + .frame(height: 14) + .animation(Tokens.Motion.micro, value: isDragging) track } + // A `GeometryReader` offers only 10pt intrinsically, so a mark measuring + // its own row along the bottom bar collapses to a stub. Here rather than + // at each call site: it was on `widthMark` alone, and the six controls + // that used to be stock sliders need it just as much. + .frame(minWidth: isVertical ? nil : Tokens.Size.horizontalMarkMinimum) .accessibilityElement() .accessibilityLabel(name) .accessibilityValue(readout) .accessibilityAdjustableAction { direction in - let step = (range.upperBound - range.lowerBound) / 100 + // **One step for integers, a twentieth for fractions.** `max(step, 1)` + // was correct for a 1...96 size and catastrophic for a 0.1...0.8 ink: + // a single VoiceOver increment jumped the whole range, so every + // fractional mark in the panel had exactly two reachable values. + let span = range.upperBound - range.lowerBound + let step = span >= 8 ? 1 : span / 20 switch direction { - case .increment: value = min(range.upperBound, value + max(step, 1)) - case .decrement: value = max(range.lowerBound, value - max(step, 1)) + case .increment: value = min(range.upperBound, value + step) + case .decrement: value = max(range.lowerBound, value - step) @unknown default: break } } @@ -167,12 +225,25 @@ struct Mark: View { } } + /// The caret **grows** under the pointer rather than merely brightening. + /// + /// Landing a value was never the problem — the whole 16pt track is the hit + /// area and a click anywhere jumps the value, so nobody has to aim at a + /// hairline. Knowing it is draggable *at all* was the problem, and the answer + /// to that was 0.82 → 0.92 opacity on a 1.5pt line, which is no answer. + /// + /// A hairline at rest, an unmistakable grab handle the moment the pointer + /// arrives — which is the only moment discoverability is being asked for. + /// Still not an AppKit knob: no bezel, no fill, no shadow, no accent. private func caret(width: CGFloat) -> some View { - Capsule(style: .continuous) - .fill(.primary.opacity(isHovering || isDragging ? Tokens.Ink.strong : Tokens.Ink.regular)) - .frame(width: 1.5, height: 14) - .offset(x: max(0, min(width - 1.5, width * t - 0.75))) + let active = isHovering || isDragging + let thickness: CGFloat = active ? 3 : 1.5 + return Capsule(style: .continuous) + .fill(.primary.opacity(active ? Tokens.Ink.strong : Tokens.Ink.regular)) + .frame(width: thickness, height: active ? 16 : 14) + .offset(x: max(0, min(width - thickness, width * t - thickness / 2))) .animation(Tokens.Motion.micro, value: isHovering) + .animation(Tokens.Motion.micro, value: isDragging) } /// A stop, drawn on the trough rather than as a second row of capsules below it. diff --git a/App/UI/ToolOptions.swift b/App/UI/ToolOptions.swift index 1e77622..759514b 100644 --- a/App/UI/ToolOptions.swift +++ b/App/UI/ToolOptions.swift @@ -139,6 +139,13 @@ struct ToolOptions: View { private var hint: String? { switch model.tool { case .pencil: "1 px, fixed" + // **The eraser paints Colour 2**, so what it leaves behind is whatever is + // loaded there — and that is invisible until you have already used it. + // It matters more now than it did: a transparent Colour 2 rubs a real hole + // through the picture, where until this release it silently did nothing at + // all. A hint is exactly the right size for a fact that changes under you. + case .eraser: + model.background.alpha == 0 ? "Rubs through to nothing" : "Rubs back to Colour 2" case .text: "⌘↩ places it" case .eyedropper: "⌥ from any tool" case .select where model.selectionKind == .lasso: "Closes itself" @@ -180,13 +187,10 @@ struct ToolOptions: View { // tip that has it rather than sitting greyed out under three that // do not. if model.brushShape.isSpray { - OptionRow("Flow") { - OptionSlider( - value: $model.sprayDensity, - range: 0.02...0.6, - readout: "\(Int(model.sprayDensity * 100))%" - ) - } + Mark( + value: $model.sprayDensity, range: 0.02...0.6, style: .meter, name: "Flow", + readout: "\(Int((model.sprayDensity * 100).rounded()))%", normal: 0.12 + ) } case .highlighter: @@ -261,27 +265,24 @@ struct ToolOptions: View { } } if model.shapeKind == .roundedRectangle || model.shapeKind == .callout { - OptionRow("Corner") { - OptionSlider( - value: Binding( - get: { Double(model.cornerRadius) }, - set: { model.cornerRadius = Int($0.rounded()) } - ), - range: 0...48, - readout: "\(model.cornerRadius)" - ) - } + Mark( + value: Binding( + get: { Double(model.cornerRadius) }, + set: { model.cornerRadius = Int($0.rounded()) } + ), + range: 0...48, style: .meter, name: "Corner", + readout: "\(model.cornerRadius)", normal: 12 + ) } case .text: - OptionRow("Size") { - OptionSlider( - value: $model.textSize, - range: 8...200, - readout: "\(Int(model.textSize))", - gamma: 2 - ) - } + // A meter, not a wedge. The wedge is this app's word for *stroke + // weight*, and a taper in the Text panel would say you were holding a + // brush. + Mark( + value: $model.textSize, range: 8...200, style: .meter, name: "Size", + gamma: 2, readout: "\(Int(model.textSize))", normal: 36 + ) OptionRow("Font") { OptionMenu( selection: $model.textFont, @@ -316,16 +317,14 @@ struct ToolOptions: View { widthMark(name: "Size") case .fill: - OptionRow("Match") { - OptionSlider( - value: Binding( - get: { Double(model.fillTolerance) }, - set: { model.fillTolerance = Int($0.rounded()) } - ), - range: 0...Double(ToolSettings.usefulTolerance), - readout: "\(model.fillTolerance)" - ) - } + Mark( + value: Binding( + get: { Double(model.fillTolerance) }, + set: { model.fillTolerance = Int($0.rounded()) } + ), + range: 0...Double(ToolSettings.usefulTolerance), style: .meter, name: "Match", + readout: "\(model.fillTolerance)", normal: 16 + ) case .spotlight: // One property, so the panel is that property: a plate whose darkness is @@ -334,16 +333,14 @@ struct ToolOptions: View { SpotlightDimPlate(dim: $model.spotlightDim) case .pixelate: - OptionRow("Block") { - OptionSlider( - value: Binding( - get: { Double(model.pixelateBlockSize) }, - set: { model.pixelateBlockSize = Int($0.rounded()) } - ), - range: 4...48, - readout: "\(model.pixelateBlockSize)" - ) - } + Mark( + value: Binding( + get: { Double(model.pixelateBlockSize) }, + set: { model.pixelateBlockSize = Int($0.rounded()) } + ), + range: 4...48, style: .meter, name: "Block", + readout: "\(model.pixelateBlockSize)", normal: 12 + ) case .select: OptionRow("Mode") { @@ -352,16 +349,14 @@ struct ToolOptions: View { }) } if model.selectionKind == .instantAlpha { - OptionRow("Match") { - OptionSlider( - value: Binding( - get: { Double(model.selectionTolerance) }, - set: { model.selectionTolerance = Int($0.rounded()) } - ), - range: 0...Double(ToolSettings.usefulTolerance), - readout: "\(model.selectionTolerance)" - ) - } + Mark( + value: Binding( + get: { Double(model.selectionTolerance) }, + set: { model.selectionTolerance = Int($0.rounded()) } + ), + range: 0...Double(ToolSettings.usefulTolerance), style: .meter, name: "Match", + readout: "\(model.selectionTolerance)", normal: 12 + ) } case .eyedropper: @@ -401,7 +396,6 @@ struct ToolOptions: View { readout: "\(model.brushSize)", normal: Double(max(allowed.lowerBound, 2)) ) - .frame(minWidth: isVertical ? nil : Tokens.Size.horizontalMarkMinimum) } // MARK: - Selection @@ -733,6 +727,21 @@ private struct BadgeNumber: View { } } +/// One row of the panel: a label in the shared column, then its control. +/// +/// **Two anatomies, and which one you get is decided by what kind of control it +/// is.** A *value* — anything continuous — is a `Mark`, which carries its own +/// name and its live number on one caption line with a full-bleed track beneath. +/// A *choice* stays here: a label in the 42pt column and a track of cells beside +/// it, on one line. +/// +/// That is a rule rather than an accident, and the alternative was measured +/// rather than assumed. Giving the choices a caption line too would align every +/// control in the panel at one x — and cost the Shape panel three extra lines, +/// on the tool that is already the tallest thing the rail has to make room for. +/// Giving the values a label column instead would hand the readout back to the +/// end of the track, which is what made every track a different length depending +/// on how many digits its value happened to have: "8" against "200". struct OptionRow: View { let title: String? @ViewBuilder let content: Content @@ -848,52 +857,6 @@ struct OptionMenu: View { } } -/// A slider with a live read-out, filling whatever column it is given. -/// -/// `gamma` bends the travel: 2 squares the input, which is what turns a 1–96 -/// range from "everything useful in the first centimetre" into a control you -/// can actually land 6px with. -struct OptionSlider: View { - @Binding var value: Double - let range: ClosedRange - let readout: String - var gamma: Double = 1 - - @Environment(\.optionsAxisIsVertical) private var isVertical - - var body: some View { - HStack(spacing: Tokens.Space.tight) { - Slider(value: curved, in: 0...1) - .controlSize(.mini) - .frame(maxWidth: isVertical ? .infinity : 68) - .labelsHidden() - Text(readout) - .font(Tokens.Text.pillValue) - .foregroundStyle(.primary.opacity(Tokens.Ink.regular)) - .frame(minWidth: 26, alignment: .trailing) - } - .accessibilityElement(children: .combine) - .accessibilityValue(readout) - } - - /// The slider works in 0...1 and the curve maps it onto the real range, so - /// the control keeps a linear feel while the values do not. - private var curved: Binding { - Binding( - get: { - let span = range.upperBound - range.lowerBound - guard span > 0 else { return 0 } - let t = (value - range.lowerBound) / span - return pow(max(0, min(1, t)), 1 / gamma) - }, - set: { t in - let span = range.upperBound - range.lowerBound - value = range.lowerBound + pow(max(0, min(1, t)), gamma) * span - } - ) - } -} - /// A segmented control on the chrome's own scale. /// /// Built rather than `.pickerStyle(.segmented)` because the system control diff --git a/App/UI/ToolRail.swift b/App/UI/ToolRail.swift index 6c937db..7670125 100644 --- a/App/UI/ToolRail.swift +++ b/App/UI/ToolRail.swift @@ -1,6 +1,100 @@ import PaintKit import SwiftUI +/// **What a rail of a given length can carry, and what it gives up first.** +/// +/// One ladder for both edges. The palette drops columns first, because every +/// swatch it drops is one press away in the system picker. Then, if the tool run +/// *still* does not fit, the tools alone scroll — while the loaded pair, the two +/// colour actions and the edge toggle keep their place. Nothing else is ever cut. +/// +/// The bottom bar had none of this. It was handed all fourteen palette columns +/// whatever the window was and never bounded its tools, so at the 560pt window +/// floor the colour block simply ran off the right-hand end. +/// +/// Plain arithmetic rather than methods on a `View`, for two reasons: the answer +/// feeds a layout pass that is already running, so it cannot wait for one — and +/// `View` is main-actor isolated, which took the whole test process down the first +/// time a geometry check asked it a question. +enum RailFit { + + /// A rule between two runs, with the stack's own spacing either side of it. + private static var rule: CGFloat { 1 + Tokens.Rail.sectionSpacing * 2 } + + /// Everything in the rail that is not a tool cell, along the rail's own axis. + /// + /// Mirrors the layout term by term rather than approximating it. The first + /// version counted **three** rules when only one is outside the tool run — the + /// other two are between the tool groups, and so belong to `toolRunLength` — + /// and it charged a trailing gap for a palette row that has none. The two + /// errors pulled in opposite directions and left the ladder shedding about + /// 6pt early, which is invisible until it is a whole tool. + static func tail(pairs: Int, isVertical: Bool) -> CGFloat { + let chips = Tokens.Size.colourWell * 1.42 + // Swap and More colours sit *under* the chips in a side rail and *beside* + // them along a bottom bar, so they cost one cell of length on one edge and + // two on the other. + let actions = isVertical + ? Tokens.Space.hair + Tokens.Size.colourSwap + : Tokens.Space.tight + Tokens.Size.colourSwap * 2 + // No palette, no gap before it: the grid is not rendered at all at zero. + let swatches = pairs > 0 + ? Tokens.Space.tight + + CGFloat(pairs) * Tokens.Size.swatch + + CGFloat(pairs - 1) * Tokens.Rail.swatchGap + : 0 + let colourBlock = Tokens.Rail.colourInset * 2 + chips + actions + swatches + let toggle = Tokens.Rail.sectionSpacing + Tokens.Size.toolCell + return Tokens.Rail.padding * 2 + rule + colourBlock + toggle + } + + /// Where each tool cell **ends** along the run. + /// + /// Walked rather than divided, because the pitch is not constant: cells inside + /// a group are `hair` apart and the groups themselves are a `rule` apart. An + /// average pitch is right until the fold crosses a group boundary, and then it + /// is 9pt wrong — which is most of a glyph. + static var toolCellEnds: [CGFloat] { + var ends: [CGFloat] = [] + var offset: CGFloat = 0 + for (index, group) in ToolKind.groups.enumerated() { + if index > 0 { offset += rule } + for cell in 0.. 0 { offset += Tokens.Space.hair } + offset += Tokens.Size.toolCell + ends.append(offset) + } + } + return ends + } + + /// The whole tool run, laid out end to end. + static var toolRunLength: CGFloat { toolCellEnds.last ?? 0 } + + /// How much room the tools get, or `nil` when they need no bound at all. + /// + /// Cut at a **real cell boundary**. A scroller cut to whatever happened to be + /// left over slices the last glyph through the middle, and a half-drawn button + /// reads as a rendering fault rather than as "there is more below" — which is + /// the one thing the scroller is there to say. + static func toolRoom(in length: CGFloat, pairs: Int, isVertical: Bool) -> CGFloat? { + let room = length - tail(pairs: pairs, isVertical: isVertical) + guard room < toolRunLength else { return nil } + return toolCellEnds.last { $0 <= room } ?? Tokens.Size.toolCell + } + + /// How many columns of the palette the rail can show in `length`. + /// + /// A side rail spends its length on tools and keeps the leading six columns; + /// the bottom bar has a whole window width and can carry all fourteen. + static func swatchPairs(fitting length: CGFloat, isVertical: Bool) -> Int { + let most = isVertical ? Tokens.Rail.swatchPairs : Palette.columns + let spare = length - tail(pairs: 0, isVertical: isVertical) - toolRunLength + let step = Tokens.Size.swatch + Tokens.Rail.swatchGap + return max(0, min(most, Int(spare / step))) + } +} + /// The permanent chrome: every tool, both colours, the palette. /// /// **Nothing here is hidden.** The classic app put its tools, its two loaded @@ -20,18 +114,31 @@ import SwiftUI /// neither can drift into being the second-class one. /// /// The one place they differ is palette breadth: the bottom bar has the window's -/// width and carries all fourteen columns, and the side rail has the window's -/// height and carries the leading seven. Both are two swatches across. The rest -/// is one click away in the colour popover. +/// width to spend and the side rail has its height, so a wide window carries all +/// fourteen columns along the bottom and a tall one carries the leading six down +/// the side. Both are two swatches across, and both give columns up before +/// anything else is cut — see `RailFit`. The rest is one press away in the system +/// colour panel, behind the More colours button that sits in the block itself. struct ToolRail: View { @Bindable var model: EditorModel - /// Columns of the palette this rail has room for. The side rail hands down - /// what the window's height allows; the bottom bar always has all fourteen. + /// Columns of the palette this rail has room for, from `RailFit`. Both edges + /// shed them: fourteen is the most a bottom bar carries and six the most a + /// side rail does, and a small window gets fewer of either. var swatchPairs: Int = Tokens.Rail.swatchPairs + /// Room the *tools* have along the rail, once the colour block and the edge + /// toggle have taken theirs. `nil` when the window has room for all of it. + /// + /// **Only the tools scroll.** The whole rail used to sit in one scroll view, + /// so a short window put the colour block and the edge toggle below the fold + /// of an indicator-less scroller — gone, with nothing to say they were gone. + /// This file has claimed since it was written that the loaded colours are the + /// part that must never be cut; that was a comment, not a behaviour. + var toolsLength: CGFloat? /// Fires when a tool cell is clicked, so the options panel can point at it. var onSelect: (ToolKind) -> Void = { _ in } @Environment(TooltipController.self) private var tooltips + @State private var edgeFrame: CGRect = .zero private var isVertical: Bool { model.chromeEdge.isVertical } @@ -39,7 +146,7 @@ struct ToolRail: View { Group { if isVertical { VStack(alignment: .center, spacing: Tokens.Rail.sectionSpacing) { - toolGroups + scrollingTools separator colourBlock edgeToggle @@ -47,7 +154,7 @@ struct ToolRail: View { .padding(Tokens.Rail.padding) } else { HStack(alignment: .center, spacing: Tokens.Rail.sectionSpacing) { - toolGroups + scrollingTools separator colourBlock edgeToggle @@ -55,13 +162,53 @@ struct ToolRail: View { .padding(Tokens.Rail.padding) } } - .fixedSize() + .fixedSize(horizontal: isVertical, vertical: !isVertical) .chromeSurface(cornerRadius: Tokens.Radius.rail) .animation(Tokens.Motion.micro, value: model.chromeEdge) } // MARK: - Tools + /// The tool run, scrolling only when the window genuinely cannot hold it — + /// and with a real scroller when it does, because a rail that is quietly + /// shorter than its contents is a rail that has hidden a tool from you. + @ViewBuilder + private var scrollingTools: some View { + if let toolsLength { + // **No scroll-to-selection here, deliberately.** The obvious polish is + // a `ScrollViewReader` bringing the armed tool into view, and it does + // not work: the cells sit inside `FixedGrid`, a custom `Layout`, and + // `scrollTo` cannot resolve an id through one — it landed on the right + // cell once and one cell from the top every other time. A control that + // works by luck is worse than one that does not exist, because the time + // it fails is the time somebody is depending on it. + // + // What covers it instead: the options panel beside the rail is titled + // with the tool you are holding, so the answer to "which one is armed" + // is on screen either way. Worth revisiting if `FixedGrid` ever becomes + // a plain stack. + ScrollView(isVertical ? .vertical : .horizontal, showsIndicators: true) { + toolGroups.fixedSize() + } + // **The cross axis is pinned, not left to the scroll view.** + // + // `Rail.thickness` is not decoration: the canvas inset and the + // tooltip column are both computed from it before any layout pass + // runs. With *Show scroll bars: Always* macOS uses legacy scrollers + // that take real space, and a `fixedSize` rail would have taken + // its width from them — quietly making the rail wider than the + // number the rest of the app is working from. The scroller overlaps + // the cells in that setting instead, which is the right trade for + // a fallback that only appears on a window shorter than any laptop. + .frame( + width: isVertical ? Tokens.Rail.cross : toolsLength, + height: isVertical ? toolsLength : Tokens.Rail.cross + ) + } else { + toolGroups + } + } + /// Tool cells in their three runs, one abreast — a single file of buttons /// along the rail, whichever edge it is on. @ViewBuilder @@ -172,12 +319,14 @@ struct ToolRail: View { .contentShape(Rectangle()) } .buttonStyle(.plain) + .trackedForTooltip($edgeFrame) .onHover { hovering in hovering ? tooltips.hover( key: "edge", title: "Move toolbar \(model.chromeEdge.toggled.displayName.lowercased())", - shortcut: "⌥⌘T" + shortcut: "⌥⌘T", + anchor: edgeFrame ) : tooltips.endHover(key: "edge") } @@ -210,6 +359,9 @@ struct ToolCell: View { @State private var isHovering = false @State private var isPressed = false + /// Where this cell is, so the one shared chip can sit beside *it* rather + /// than beside the top of the rail. + @State private var frame: CGRect = .zero @Environment(TooltipController.self) private var tooltips @Environment(\.colorSchemeContrast) private var contrast @@ -260,10 +412,14 @@ struct ToolCell: View { .contentShape(Rectangle()) } .buttonStyle(.plain) + .trackedForTooltip($frame) .onHover { hovering in isHovering = hovering hovering - ? tooltips.hover(key: key, title: title, shortcut: shortcut, detail: detail) + ? tooltips.hover( + key: key, title: title, shortcut: shortcut, + detail: detail, anchor: frame + ) : tooltips.endHover(key: key) } // A pressed state the finger can feel. `.plain` gives none, and a @@ -311,7 +467,10 @@ private struct ColourControls: View { let isVertical: Bool @Environment(TooltipController.self) private var tooltips - @State private var isPopoverPresented = false + @Environment(\.colorScheme) private var colorScheme + @State private var swapFrame: CGRect = .zero + @State private var moreFrame: CGRect = .zero + @State private var chipFrames: [EditorModel.ColourRole: CGRect] = [:] var body: some View { // Swap sits beside the pair along the bottom bar and beneath it in the @@ -335,35 +494,80 @@ private struct ColourControls: View { height: Tokens.Size.colourWell * 1.42, alignment: .topLeading ) + colourActions + } + .frame(width: isVertical ? Tokens.Rail.cross : nil, alignment: .center) + } + + /// Swap, and the way to every colour there is — two cells in the run the swap + /// button used to hold alone. + /// + /// **The second cell is the door the app never had.** The full palette, custom + /// colours and *opacity* were behind ⇧⌘C and a menu item three levels down, so + /// the app's most interesting capability — a colour that is partly or entirely + /// see-through, which a transparent one now erases with — was a thing you had + /// to already know about. `PHILOSOPHY.md` has the sentence for this: a popover + /// you have to know about is a worse control than a swatch you can already + /// see. That argument does not end in "build a nicer popover"; it ends in a + /// button in the permanent chrome. The panel it opens is Apple's, which has an + /// opacity slider, an eyedropper, saved swatches and a recents row that + /// survives quitting — all things a bespoke popover was a weaker copy of. + /// + /// Two cells of `Rail.cross / 2` rather than a second run, so `Rail.cross` + /// does not move — and therefore neither do `Rail.thickness`, + /// `ToolbarGeometryTests` or `RailFit.swatchPairs(fitting:)`. A second 18pt + /// run would have cost exactly one palette column on a laptop-height window. + @ViewBuilder + private var colourActions: some View { + let cellWidth = isVertical ? Tokens.Rail.cross / 2 : Tokens.Size.colourSwap + let cellHeight = isVertical ? Tokens.Size.colourSwap : Tokens.Size.colourWell + + HStack(spacing: 0) { Button { model.swapColours() } label: { Image(systemName: "arrow.triangle.2.circlepath") .font(.system(size: 10, weight: .medium)) - .frame( - width: isVertical ? Tokens.Rail.run : Tokens.Size.colourSwap, - height: isVertical ? Tokens.Size.colourSwap : Tokens.Size.colourWell - ) + .frame(width: cellWidth, height: cellHeight) .contentShape(Rectangle()) } .buttonStyle(.plain) - .foregroundStyle(.primary.opacity(Tokens.Ink.muted)) + .trackedForTooltip($swapFrame) .onHover { hovering in hovering - ? tooltips.hover(key: "swap", title: "Swap colours", shortcut: "X") + ? tooltips.hover( + key: "swap", title: "Swap colours", shortcut: "X", anchor: swapFrame + ) : tooltips.endHover(key: "swap") } .accessibilityLabel("Swap the front and back colours") + + Button { + model.presentSystemColourPicker(for: .foreground) + } label: { + DrawnGlyph.colourWell.shape(in: .primary.opacity(Tokens.Ink.muted)) + .frame(width: 13, height: 13) + .frame(width: cellWidth, height: cellHeight) + .contentShape(Rectangle()) + } + .buttonStyle(.plain) + .trackedForTooltip($moreFrame) + .onHover { hovering in + hovering + ? tooltips.hover( + key: "morecolours", + title: "More colours", + shortcut: "⇧⌘C", + // The second clause is the whole reason this button + // exists. Transparency was reachable and unmentioned. + detail: "Any colour, at any opacity. A fully clear one rubs paint out.", + anchor: moreFrame + ) + : tooltips.endHover(key: "morecolours") + } + .accessibilityLabel("More colours, including transparency") } - .frame(width: isVertical ? Tokens.Rail.cross : nil, alignment: .center) - .popover(isPresented: $isPopoverPresented, arrowEdge: isVertical ? .trailing : .top) { - ColourPopover(model: model) - } - .onChange(of: model.isColourPopoverRequested) { _, requested in - guard requested else { return } - isPopoverPresented = true - model.isColourPopoverRequested = false - } + .foregroundStyle(.primary.opacity(Tokens.Ink.muted)) } private func chip(_ colour: PaintColour, role: EditorModel.ColourRole, label: String) -> some View { @@ -378,17 +582,19 @@ private struct ColourControls: View { // Left-clicking the back chip now promotes its colour to the front, // which is the common move: you had it a moment ago and you want it // again. Double-click still opens the system picker for that slot. - RoundedRectangle(cornerRadius: Tokens.Radius.swatch, style: .continuous) + let shape = RoundedRectangle(cornerRadius: Tokens.Radius.swatch, style: .continuous) + + return shape .fill(Color(cgColor: colour.cgColor)) .frame(width: Tokens.Size.colourWell, height: Tokens.Size.colourWell) + .background { TransparencyChecker(under: colour).clipShape(shape) } .overlay { - RoundedRectangle(cornerRadius: Tokens.Radius.swatch, style: .continuous) - .strokeBorder(.black.opacity(0.35), lineWidth: 0.5) + shape.strokeBorder(.black.opacity(0.35), lineWidth: 0.5) } .overlay(alignment: .bottomTrailing) { Text(label) .font(.system(size: 7.5, weight: .bold)) - .foregroundStyle(colour.prefersDarkContrast ? .black.opacity(0.55) : .white.opacity(0.75)) + .foregroundStyle(prefersDarkInk(colour) ? .black.opacity(0.55) : .white.opacity(0.75)) .padding(1.5) } .contentShape(Rectangle()) @@ -398,31 +604,127 @@ private struct ColourControls: View { Button("Set as Colour 1") { model.applySwatch(colour, to: .foreground) } Button("Set as Colour 2") { model.applySwatch(colour, to: .background) } Divider() + // The one-click way to the thing the More colours button teaches. + // Loaded as Colour 2 it also gives the eraser a real hole to rub, + // which is the move people were reaching for when they found the + // eraser doing nothing at all. + Button("Transparent") { model.applySwatch(.clear, to: role) } Button("Other colour…") { model.presentSystemColourPicker(for: role) } } + .trackedForTooltip(Binding( + get: { chipFrames[role] ?? .zero }, + set: { chipFrames[role] = $0 } + )) .onHover { hovering in hovering + // **The name goes in the title, everything else in the line + // under it.** All three clauses used to be one heading, which + // has no width cap — and once the opacity was added to it, a + // translucent colour produced a heading about 380pt wide. + // A title is a name; what the thing is set to and what you can + // do with it are a sentence, and the chip already has a place + // for a sentence. ? tooltips.hover( key: "colour\(label)", - // One separator, not two. These read left to right as - // "what it is, what it is set to, what you can do", - // which a dash and a middle dot in the same nine words - // actively worked against. title: role == .foreground - ? "Colour 1 · \(colour.hexString) · double-click to change" - : "Colour 2 · \(colour.hexString) · click to use as Colour 1", - shortcut: nil + ? "Colour 1 · \(colour.hexString)" + : "Colour 2 · \(colour.hexString)", + shortcut: nil, + detail: chipDetail(colour, role: role), + anchor: chipFrames[role] ?? .zero ) : tooltips.endHover(key: "colour\(label)") } .accessibilityElement() .accessibilityLabel( role == .foreground - ? "Colour 1, front, \(colour.hexString)" - : "Colour 2, back, \(colour.hexString)" + ? "Colour 1, front, \(describe(colour))" + : "Colour 2, back, \(describe(colour))" ) .accessibilityHint("Click to load as Colour 1. Double-click to choose another colour.") } + + /// What this chip is set to, and what pressing it will do. + private func chipDetail(_ colour: PaintColour, role: EditorModel.ColourRole) -> String { + let gesture = role == .foreground + ? "Double-click for another colour" + : "Click to use it as Colour 1" + guard colour.alpha < 1 else { return gesture } + let state = colour.alpha == 0 + ? "Fully transparent, so it rubs paint out" + : "\(Int((colour.alpha * 100).rounded()))% opaque" + return "\(state). \(gesture)" + } + + /// A colour in words, with its opacity when it has one worth saying. + /// + /// `hexString` already appends the alpha byte, and `FF000080` is not "50%" to + /// anybody. The checkerboard behind the chip says *that* it is see-through and + /// roughly how much; this is where the exact number goes, at no cost in + /// pixels, and it is what VoiceOver reads. + private func describe(_ colour: PaintColour) -> String { + guard colour.alpha < 1 else { return colour.hexString } + guard colour.alpha > 0 else { return "transparent" } + return "\(colour.hexString) · \(Int((colour.alpha * 100).rounded()))% opaque" + } + + /// Which ink reads on this chip, allowing for what is *behind* it. + /// + /// `prefersDarkContrast` is computed from relative luminance and knows nothing + /// about alpha, so a 10%-opacity black chip — visually almost entirely the + /// checkerboard — asked for white text and vanished. Below half opacity the + /// chip is mostly the system-coloured checkerboard, which is light in light + /// appearance and dark in dark. + private func prefersDarkInk(_ colour: PaintColour) -> Bool { + colour.alpha < 0.5 ? colorScheme == .light : colour.prefersDarkContrast + } +} + +/// The same checkerboard the canvas draws under transparent pixels, at chip scale. +/// +/// **Only under a colour that is actually see-through.** A checkerboard behind +/// every chip is a texture people learn to skip, and they skip it on the day it +/// matters. Its *arrival* is the signal that this colour is not solid, and how +/// strongly it shows through is the reading of how much alpha there is — a legend +/// nobody has to be given, which is the only kind this app allows. +/// +/// The two colours are `CanvasNSView`'s, deliberately: a chip has to read as the +/// same kind of nothing the artwork does, and two greys for one meaning is exactly +/// the drift `Tokens.Ink` exists to stop. +struct TransparencyChecker: View { + var tile: CGFloat = Tokens.Size.transparencyTileChip + /// Draws nothing at all for an opaque colour. + var opacity: Double = 1 + + init(under colour: PaintColour, tile: CGFloat = Tokens.Size.transparencyTileChip) { + self.tile = tile + self.opacity = colour.alpha < 1 ? 1 : 0 + } + + var body: some View { + Canvas(opaque: false, rendersAsynchronously: false) { context, size in + context.fill( + Path(CGRect(origin: .zero, size: size)), + with: .color(Color(nsColor: .controlBackgroundColor)) + ) + let dark = Color(nsColor: .separatorColor).opacity(0.22) + for row in 0.. some View { - RoundedRectangle(cornerRadius: Tokens.Radius.swatch, style: .continuous) + let shape = RoundedRectangle(cornerRadius: Tokens.Radius.swatch, style: .continuous) + + return shape .fill(Color(cgColor: colour.cgColor)) .frame(width: Tokens.Size.swatch, height: Tokens.Size.swatch) + // Same rule as the loaded pair, so the two grids cannot disagree + // about what transparency looks like. The fixed 28 are all opaque + // today; a document that arrives carrying a custom palette need not be. + .background { TransparencyChecker(under: colour).clipShape(shape) } .overlay { - RoundedRectangle(cornerRadius: Tokens.Radius.swatch, style: .continuous) - .strokeBorder(.black.opacity(0.28), lineWidth: 0.5) + shape.strokeBorder(.black.opacity(0.28), lineWidth: 0.5) } .overlay { // A ring on whichever swatches are loaded, so the palette says @@ -553,16 +860,62 @@ struct MouseButtons: NSViewRepresentable { /// Glyphs SF Symbols does not ship. /// -/// Exactly one so far. A tipped bucket with a drip is the icon everyone already -/// knows means "flood fill" — the droplet the system offers reads as the -/// eyedropper's cousin, which is the one tool it must not be confused with. +/// A tipped bucket with a drip is the icon everyone already knows means "flood +/// fill" — the droplet the system offers reads as the eyedropper's cousin, which +/// is the one tool it must not be confused with. +/// +/// The colour well is the same kind of gap. `paintpalette` and +/// `circle.hexagongrid.fill` both say "more colours" and neither says the second +/// half — that a colour here can be see-through, which is the half the rail has +/// never said anywhere. A chip sitting on a checkerboard says both in one mark, +/// and it is the mark every image editor already uses for alpha. enum DrawnGlyph { case bucket + case colourWell @ViewBuilder func shape(in colour: Color) -> some View { switch self { case .bucket: BucketGlyph(colour: colour) + case .colourWell: ColourWellGlyph(colour: colour) + } + } +} + +/// A swatch cut in half on the diagonal: solid on one side, nothing on the other. +/// +/// **The mark macOS itself uses for alpha** — it is the well at the foot of the +/// system Colors panel, so anybody who has met a colour picker on this platform +/// has already been taught it. That matters more than inventing something here. +/// +/// A checkerboard was the first attempt and it does not survive the size. Four +/// tiles across a 13pt glyph is 3.25pt a square, which resolves as grey mush and +/// says nothing at all; the diagonal reads down to about 10pt because it is one +/// edge rather than sixteen. +private struct ColourWellGlyph: View { + let colour: Color + + var body: some View { + GeometryReader { proxy in + let side = min(proxy.size.width, proxy.size.height) + let field = RoundedRectangle(cornerRadius: side * 0.24, style: .continuous) + + field + .strokeBorder(colour.opacity(0.75), lineWidth: max(1, side * 0.09)) + .background { + // The solid half. Clipped by the same rounded shape, so the + // corner it fills is the swatch's corner and not a square one + // poking out of it. + Path { path in + path.move(to: .zero) + path.addLine(to: CGPoint(x: side, y: 0)) + path.addLine(to: CGPoint(x: 0, y: side)) + path.closeSubpath() + } + .fill(colour) + .clipShape(field) + } + .frame(width: side, height: side) } } } diff --git a/App/UI/Tooltip.swift b/App/UI/Tooltip.swift index 90416b7..a4ac4db 100644 --- a/App/UI/Tooltip.swift +++ b/App/UI/Tooltip.swift @@ -1,3 +1,4 @@ +import AppKit import Observation import SwiftUI @@ -26,13 +27,13 @@ struct Tooltip: View { heading if let detail { Text(detail) - .font(.system(size: 11)) + .font(.system(size: Self.detailPointSize)) // A tooltip is read once, quickly, at a glance. `muted` is // the value-beside-a-label step and it is too quiet for a // sentence somebody has stopped to read. .foregroundStyle(.primary.opacity(Tokens.Ink.regular)) .fixedSize(horizontal: false, vertical: true) - .frame(maxWidth: 210, alignment: .leading) + .frame(width: Self.detailWidth(of: detail), alignment: .leading) } } .padding(.horizontal, Tokens.Space.base + 1) @@ -56,11 +57,65 @@ struct Tooltip: View { .transition(.opacity.combined(with: .offset(y: 3))) } + /// The size the explanation is set at, in one place, because the layout has + /// to measure the same face it draws. + static let detailPointSize: CGFloat = 11 + + /// The widest a chip's explanation may get before it wraps. + static let maxDetailWidth: CGFloat = 210 + + /// A **concrete** width for the sentence, and that is the whole fix for a bug + /// worth naming. + /// + /// `.fixedSize(horizontal: false, vertical: true)` keeps the *proposed* width + /// and asks for the height that fits it. But the chip is `.fixedSize()` + /// overall, so the width proposed down to that text was `nil` — and under a + /// nil proposal `.frame(maxWidth: 210)` let the text report the height of one + /// unbroken line while drawing three. The chip then clipped the other two + /// away with its own rounded-rectangle mask. + /// + /// **The bug hid itself.** Every assertion in `TooltipRenderTests` bounded the + /// chip from *above* — "not taller than 66pt" — so each line that went missing + /// made the check pass more comfortably. What replaced them is a check that a + /// longer sentence makes a taller chip, which is the property the eye is + /// actually using. See `docs/CHECKS_THAT_MISS.md`. + /// + /// Measured rather than fixed at the cap so a three-word chip stays a + /// three-word chip: "Already at 100%" in a 210pt box is a chip mostly made of + /// nothing. + static func detailWidth(of detail: String) -> CGFloat { + // AppKit's metrics for the face SwiftUI resolves `.system(size: 11)` to, + // plus a point of slack so a sentence that measures exactly at the cap is + // not wrapped by a rounding difference between the two frameworks. + let ideal = (detail as NSString) + .size(withAttributes: [.font: NSFont.systemFont(ofSize: detailPointSize)]) + .width + return min(ceil(ideal) + 1, maxDetailWidth) + } + + /// The widest a chip's *title* may get. + /// + /// A backstop rather than a working limit. The detail line has been capped + /// since the clipping fix, and the heading had nothing at all — so a caller + /// that put three clauses and a live hex value in the title, which the colour + /// chips briefly did, produced a chip about 380pt wide with no test able to + /// see it. Titles are names; a name that needs 260pt is the bug. + static let maxTitleWidth: CGFloat = 260 + private var heading: some View { HStack(spacing: Tokens.Space.tight + 1) { Text(title) .font(.system(size: 12, weight: .semibold)) .foregroundStyle(.primary) + .lineLimit(1) + .frame( + width: min( + ceil((title as NSString) + .size(withAttributes: [.font: NSFont.boldSystemFont(ofSize: 12)]).width) + 2, + Self.maxTitleWidth + ), + alignment: .leading + ) if let shortcut { Text(shortcut) @@ -98,14 +153,15 @@ final class TooltipController { private(set) var shortcut: String? private(set) var detail: String? - /// The hovered control's frame in screen coordinates, for the controls whose - /// chip cannot be positioned from the rail's own geometry. + /// The hovered control's frame in screen coordinates. /// - /// The rail knows where its cells are, so it leaves this nil and offsets the - /// chip itself. The header does not — its cluster slides with the window - /// width — so header buttons report where they actually are and the chip - /// follows the glyph rather than guessing at the middle of the row. - private(set) var anchor: CGRect? + /// **Required, not optional.** It used to be optional because the rail drew + /// its own chip from a constant offset and only the header reported a + /// position. There is one chip now and it is placed from this, so a missing + /// anchor cannot mean "the other layer will handle it" — it can only mean the + /// chip goes somewhere wrong or nowhere at all. A defaulted `nil` on `hover` + /// was a trap set for whoever adds the eleventh call site. + private(set) var anchor: CGRect = .zero @ObservationIgnored private var pendingKey: String? @ObservationIgnored private var work: DispatchWorkItem? @@ -130,7 +186,7 @@ final class TooltipController { title: String, shortcut: String?, detail: String? = nil, - anchor: CGRect? = nil + anchor: CGRect ) { guard pendingKey != key else { return } pendingKey = key @@ -171,7 +227,7 @@ final class TooltipController { MainActor.assumeIsolated { guard let self, self.pendingKey == nil else { return } self.visibleKey = nil - self.anchor = nil + self.anchor = .zero } } work = item @@ -182,6 +238,6 @@ final class TooltipController { work?.cancel() pendingKey = nil visibleKey = nil - anchor = nil + anchor = .zero } } diff --git a/AppTests/HeaderGeometryTests.swift b/AppTests/HeaderGeometryTests.swift new file mode 100644 index 0000000..530c1f1 --- /dev/null +++ b/AppTests/HeaderGeometryTests.swift @@ -0,0 +1,93 @@ +import PaintKit +import SwiftUI +import Testing +@testable import ItsPaint + +/// **The header must fit the narrowest window this app will make.** +/// +/// It did not. `DrawingDocument.minimumContentSize` is 560pt wide and the row +/// needed 647pt to draw itself with no filename in it at all, so the trailing +/// controls simply ran off the right edge — no truncation, no overflow, no way to +/// reach Share or Duplicate or the zoom. Two numbers that had never been compared. +/// +/// `HeaderFit` decides what to shed from arithmetic over `Tokens.Header`, so +/// checking the ladder against those tokens would be `docs/CHECKS_THAT_MISS.md` +/// §1 — the constants and everything derived from them agreeing with each other +/// and with nothing on screen. This **renders the real row** and measures the +/// picture instead, then compares it against the width the *window* declares. +@Suite("Header geometry") +@MainActor +struct HeaderGeometryTests { + + /// The real controls, at a given rung, measured as drawn. + private func drawnWidth(_ fit: HeaderFit) throws -> CGFloat { + let model = EditorModel(canvas: Bitmap(width: 1000, height: 640, fill: .white)) + let row = HStack(spacing: Tokens.Space.comfortable) { + WorkingActions(model: model, fit: fit) + DocumentActions(model: model, fit: fit) + } + .fixedSize() + .environment(TooltipController()) + + let renderer = ImageRenderer(content: row) + renderer.scale = 1 + return try #require(renderer.nsImage, "\(fit) did not render").size.width + } + + @Test("Every rung draws into the width it claims to need") + func rungsFitTheirOwnClaim() throws { + for fit in HeaderFit.allCases where fit != .full { + let drawn = try drawnWidth(fit) + let budget = fit.minimumWindow + - Tokens.Space.comfortable * 2 // the row's own padding + - Tokens.Chrome.trafficLightClearance + - Tokens.Space.comfortable * 3 // the gaps around title and spacer + - Tokens.Space.base // Spacer(minLength:) + - Tokens.Header.titleRoom + // No slack. At the 560pt window floor the chosen rung leaves exactly + // 141pt for a 140pt `titleRoom`, so a point of tolerance here is the + // whole margin — a row that drew 1pt wide would pass this *and* eat it. + #expect( + drawn <= budget, + "\(fit) draws \(drawn)pt of controls into the \(budget)pt it claims to need" + ) + } + } + + /// A ladder that is not monotonic is not a ladder: a window that *shrank* + /// could pick a wider arrangement than the one it just had. + @Test("The rungs get narrower, in the order they are declared") + func theLadderDescends() { + let rungs = HeaderFit.allCases.map(\.minimumWindow) + #expect(rungs == rungs.sorted(by: >), "\(rungs) is not a ladder") + } + + /// The assertion that failed before any of this existed. + @Test("The last resort fits the smallest window the app can be dragged to") + func theBottomOfTheLadderClearsTheWindowFloor() throws { + let last = try #require(HeaderFit.allCases.last) + #expect( + last.minimumWindow <= DrawingDocument.minimumContentSize.width, + "the header's last resort needs \(last.minimumWindow)pt and the window can be dragged to \(DrawingDocument.minimumContentSize.width)pt" + ) + } + + /// The second failure in the same row, and the one the screenshot showed: the + /// filename had a layout *priority* but no *ceiling*. Centred, the cluster is + /// not in the title's stack at all, so a long name was handed the whole row + /// and drawn over the zoom controls — the stack paints the title last. + @Test("A long filename never reaches the centred cluster") + func theTitleStopsShortOfTheCluster() { + let width: CGFloat = 1600 + let ceiling = EditorView.titleCeiling(.full, in: width) + // Where the cluster's own leading edge is, on a cluster centred on the + // window rather than between its neighbours. + let clusterLeading = width / 2 - HeaderFit.full.workingWidth / 2 + let titleLeading = Tokens.Space.comfortable * 2 + Tokens.Chrome.trafficLightClearance + + #expect( + titleLeading + ceiling + Tokens.Space.base <= clusterLeading, + "a full-width title reaches \(titleLeading + ceiling)pt and the cluster starts at \(clusterLeading)pt" + ) + } +} diff --git a/AppTests/OptionMarkTests.swift b/AppTests/OptionMarkTests.swift new file mode 100644 index 0000000..2085395 --- /dev/null +++ b/AppTests/OptionMarkTests.swift @@ -0,0 +1,96 @@ +import Foundation +import PaintKit +import Testing +@testable import ItsPaint + +/// The options panel's continuous values. +/// +/// Every one of them is a `Mark` now. Six were a stock `Slider` at `.mini` — +/// blue track, chrome knob — sitting in a panel whose every other control was +/// drawn by hand, which is the giveaway `Mark`'s own doc comment was written to +/// complain about and then did not cover. +@Suite("The options panel's continuous values") +@MainActor +struct OptionMarkTests { + + /// One row per mark the panel draws. `probe` is a value the app can genuinely + /// hold that sits *outside* that mark's own range — `ToolSettings` clamps the + /// spray's density to 1 while the Flow mark stops at 0.6. + struct Row: Sendable { + let name: String + let range: ClosedRange + let gamma: Double + let probe: Double + } + + // `nonisolated`: the `arguments:` list is evaluated by the test macro outside + // the suite's own actor, and a table of numbers has nothing to protect. + nonisolated static let marks: [Row] = [ + .init(name: "Size", range: 1...96, gamma: 2, probe: 200), + .init(name: "Size (highlighter's chisel floor)", range: 4...96, gamma: 2, probe: 2), + .init(name: "Ink", range: 0.1...0.8, gamma: 1, probe: 1), + .init(name: "Opacity", range: 0.1...1, gamma: 1, probe: 0), + .init(name: "Strength", range: 0.15...0.85, gamma: 1, probe: 1), + .init(name: "Flow", range: 0.02...0.6, gamma: 1, probe: 1), + .init(name: "Corner", range: 0...48, gamma: 1, probe: 96), + .init(name: "Text size", range: 8...200, gamma: 2, probe: 4), + .init(name: "Match", range: 0...Double(ToolSettings.usefulTolerance), gamma: 1, probe: 255), + .init(name: "Block", range: 4...48, gamma: 1, probe: 96), + ] + + @Test("A mark's ends are its range's ends, and nothing draws past them", arguments: marks) + func endsMapToEnds(mark: Row) { + let low = mark.range.lowerBound + let high = mark.range.upperBound + + // Dragging to either end lands on the bound. The failure this catches is a + // size control that cannot quite reach 96, or a tolerance that cannot + // reach 0 — the two values people go looking for. + #expect(abs(Mark.value(atFraction: 0, in: mark.range, gamma: mark.gamma) - low) < 1e-9) + #expect(abs(Mark.value(atFraction: 1, in: mark.range, gamma: mark.gamma) - high) < 1e-9) + + // And the caret comes back to where the drag put it. + for f in [0.0, 0.25, 0.5, 1.0] { + let value = Mark.value(atFraction: f, in: mark.range, gamma: mark.gamma) + let back = Mark.fraction(of: value, in: mark.range, gamma: mark.gamma) + #expect(abs(back - f) < 1e-9, "\(mark.name) round-tripped \(f) as \(back)") + } + + // A stored value outside the range still has to draw *inside* the trough. + // `t` skipped its clamp on the `gamma == 1` path — which is every mark + // that does not bend its travel — and the meter's fill is an unclipped + // overlay, so it painted past the end of its own bed. + let out = Mark.fraction(of: mark.probe, in: mark.range, gamma: mark.gamma) + #expect((0...1).contains(out), "\(mark.name) drew at \(out) of its track") + } + + /// **The step VoiceOver moves by has to fit the range it is moving in.** + /// + /// It was `max(range / 100, 1)`, which is one pixel on a 1–96 size and *the + /// entire range* on any 0-to-1 fraction: Ink, Opacity, Strength and Flow each + /// had exactly two reachable values. A control a keyboard user can only put at + /// its two ends is not an adjustable control. + @Test("Every mark is adjustable in more than two steps", arguments: marks) + func everyMarkHasUsableSteps(mark: Row) { + let span = mark.range.upperBound - mark.range.lowerBound + let step = span >= 8 ? 1 : span / 20 + #expect(step > 0) + #expect(span / step >= 10, "\(mark.name) has only \(span / step) steps between its ends") + } + + /// A tripwire, and honest about being one: `SwiftUI.Slider(` walks straight + /// past it. It catches the accident — somebody reaching for the familiar + /// control while adding a row — not somebody who has decided to. + @Test("No stock slider in the tool options panel") + func thePanelDrawsItsOwnControls() throws { + let ui = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() // AppTests/ + .deletingLastPathComponent() // repository root + .appendingPathComponent("App/UI") + + for file in ["ToolOptions.swift", "Mark.swift"] { + let source = try String(contentsOf: ui.appendingPathComponent(file), encoding: .utf8) + #expect(!source.contains("Slider("), "\(file) is back to an AppKit knob") + } + } +} diff --git a/AppTests/ToolbarGeometryTests.swift b/AppTests/ToolbarGeometryTests.swift index e042311..18df664 100644 --- a/AppTests/ToolbarGeometryTests.swift +++ b/AppTests/ToolbarGeometryTests.swift @@ -1,4 +1,5 @@ import Foundation +import SwiftUI import PaintKit import Testing @testable import ItsPaint @@ -138,20 +139,99 @@ struct ToolbarGeometryTests { @Test("The rail fits the height of a small laptop window") func railFitsAShortWindow() { - // The side rail runs the length of the window, so its content has to - // fit a 13-inch display's usable height or the rail starts scrolling — - // and a toolbar you scroll to reach a tool is a toolbar that hid it. - let cell = Tokens.Size.toolCell + Tokens.Space.hair - let tools = ToolKind.groups.reduce(CGFloat.zero) { $0 + CGFloat($1.count) * cell } - let separators = CGFloat(ToolKind.groups.count) * (1 + Tokens.Rail.sectionSpacing * 2) - let pair = Tokens.Size.colourWell * 1.42 + Tokens.Space.hair + Tokens.Size.colourSwap - let swatches = CGFloat(Tokens.Rail.swatchPairs) * (Tokens.Size.swatch + Tokens.Rail.swatchGap) - let colours = pair + Tokens.Space.tight + swatches + Tokens.Rail.colourInset * 2 - let toggle = Tokens.Rail.sectionSpacing + Tokens.Size.toolCell - let total = Tokens.Rail.padding * 2 + tools + separators + colours + toggle + // The side rail runs the length of the window, so its content has to fit a + // 13-inch display's usable height or the rail starts scrolling — and a + // toolbar you scroll to reach a tool is a toolbar that hid it. + // + // Asked of the shipping arithmetic rather than of a copy of it. This test + // used to re-derive the whole sum locally, which is `CHECKS_THAT_MISS.md` + // §1: it could only ever agree with itself, and it would have gone on + // passing while the rail on screen grew a row. + let total = RailFit.tail(pairs: Tokens.Rail.swatchPairs, isVertical: true) + + RailFit.toolRunLength // 800pt of window, less the titlebar reserve and the bottom safe inset. let usable: CGFloat = 800 - Tokens.Chrome.titleReserve - Tokens.Space.safeInset #expect(total <= usable, "the rail needs \(total)pt but only \(usable)pt is on offer") } + + /// **Neither edge may cut its own colour block — measured, not asserted.** + /// + /// `RailFit` decides what to shed from arithmetic, so checking `RailFit` + /// against `Tokens` would be `docs/CHECKS_THAT_MISS.md` §1: it could only ever + /// agree with itself, and it would go on passing while the rail on screen grew + /// a row. That is exactly how this file's previous height check went wrong — it + /// kept a private copy of the same sum. + /// + /// So this renders the real `ToolRail` at the smallest window the app can be + /// dragged to and measures the picture. Both things it asserts had a real + /// failure to catch: the bottom bar was handed all fourteen palette columns + /// whatever the window was and ran off the right-hand end, and `RailFit`'s model + /// of the tool run disagreed with the layout by 16pt because it charged a flat + /// pitch for cells that are separated by a rule. + /// + /// `DrawingDocument` is an `NSDocument`, so reading its window floor means + /// touching AppKit — and AppKit class realisation off the main thread takes the + /// whole test process down rather than failing one check. + @MainActor + @Test("Both edges shed their palette rather than cutting the colours") + func neitherEdgeClipsTheColourBlock() throws { + let floor = DrawingDocument.minimumContentSize + + for edge in [EditorModel.ChromeEdge.left, .bottom] { + let isVertical = edge.isVertical + let length = isVertical + ? floor.height - Tokens.Chrome.titleReserve - Tokens.Space.safeInset + : floor.width - Tokens.Chrome.railInset * 2 + + let pairs = RailFit.swatchPairs(fitting: length, isVertical: isVertical) + let room = RailFit.toolRoom(in: length, pairs: pairs, isVertical: isVertical) + + let model = EditorModel(canvas: Bitmap(width: 400, height: 300, fill: .white)) + model.chromeEdge = edge + let renderer = ImageRenderer( + content: ToolRail(model: model, swatchPairs: pairs, toolsLength: room) + .environment(TooltipController()) + ) + renderer.scale = 1 + let drawn = try #require(renderer.nsImage, "the \(edge) rail did not render").size + let along = isVertical ? drawn.height : drawn.width + let across = isVertical ? drawn.width : drawn.height + + #expect( + along <= length, + "the \(edge) rail draws \(along)pt into the \(length)pt a smallest window offers" + ) + // The tools may be shortened, never the colours: the rail has to still + // be its declared thickness, which is the number the canvas inset and + // the tooltip column are both computed from. + #expect( + abs(across - Tokens.Rail.thickness) <= 1, + "the \(edge) rail is \(across)pt thick against a declared \(Tokens.Rail.thickness)pt" + ) + #expect( + (room ?? .infinity) >= Tokens.Size.toolCell, + "the tool run is down to \(room ?? 0)pt on the \(edge) edge" + ) + } + } + + /// The fold in a shortened tool run lands **between** cells. + /// + /// `toolRoom` used to divide by an average pitch, which is right only while the + /// fold stays inside one group: the groups are separated by a rule, so past the + /// fifth cell the answer drifts and slices a glyph down the middle. A + /// half-drawn button reads as a rendering fault, not as "there is more below". + @Test("A shortened tool run is never cut through a glyph") + func theFoldLandsBetweenCells() { + let ends = Set(RailFit.toolCellEnds) + for height in stride(from: 200.0, through: 900.0, by: 1) { + let pairs = RailFit.swatchPairs(fitting: height, isVertical: true) + guard let room = RailFit.toolRoom(in: height, pairs: pairs, isVertical: true) else { continue } + #expect( + ends.contains(room) || room == Tokens.Size.toolCell, + "at \(height)pt the run is cut at \(room)pt, which is not a cell boundary" + ) + } + } } diff --git a/AppTests/TooltipRenderTests.swift b/AppTests/TooltipRenderTests.swift index 30f5d17..94de481 100644 --- a/AppTests/TooltipRenderTests.swift +++ b/AppTests/TooltipRenderTests.swift @@ -10,33 +10,55 @@ import PaintKit @MainActor struct TooltipRenderTests { + /// Render one chip and hand back the image, so every check in this file is + /// looking at the same thing the pointer is. + /// + /// Dark, because that is where this chrome lives, and because rendering + /// `.primary` text over a black background in the light scheme paints black on + /// black: the first run of this file produced an image with the detail line + /// perfectly present and perfectly invisible. + private func render(_ title: String, _ shortcut: String?, _ detail: String?) throws -> NSImage { + let view = Tooltip(title: title, shortcut: shortcut, detail: detail) + .padding(10) + .environment(\.colorScheme, .dark) + .background(Color(white: 0.13)) + let renderer = ImageRenderer(content: view) + renderer.scale = 2 + return try #require(renderer.nsImage, "\(title) chip did not render") + } + + /// The tallest a chip may be: a title and **two** lines under it. + /// + /// Derived by rendering, not written down. `66` used to be written down, and it + /// was measured off a chip that was silently clipping its own third line — so + /// the constant was not describing the design, it was describing the bug. A + /// ceiling taken from a one-line chip plus one line's worth of growth cannot + /// drift away from the thing it is bounding. + private func twoLineCeiling() throws -> CGFloat { + let bare = try render("Title", "C", nil).size.height + let one = try render("Title", "C", "One line.").size.height + return one + (one - bare) + } + @Test("Every tool with a tip renders one line of explanation") func tipsRenderAndStayOneOrTwoLines() throws { let withTips = ToolKind.allCases.filter { $0.tip != nil } #expect(withTips.contains(.clone)) #expect(withTips.contains(.select)) + let ceiling = try twoLineCeiling() + for tool in withTips { - let view = Tooltip( - title: tool.displayName, - shortcut: String(tool.shortcut).uppercased(), - detail: tool.tip + let image = try render( + tool.displayName, String(tool.shortcut).uppercased(), tool.tip ) - .padding(10) - // Dark, because that is where this chrome lives, and because rendering - // `.primary` text over a black background in the light scheme paints - // black on black: the first run of this test produced an image with the - // detail line perfectly present and perfectly invisible. - .environment(\.colorScheme, .dark) - .background(Color(white: 0.13)) - - let renderer = ImageRenderer(content: view) - renderer.scale = 2 - let image = try #require(renderer.nsImage, "\(tool) tooltip did not render") // Wide enough to read, and not so tall it has wrapped into a paragraph. #expect(image.size.width <= 260, "\(tool) tip is \(image.size.width)pt wide") - #expect(image.size.height <= 66, "\(tool) tip wrapped to \(image.size.height)pt") + #expect( + image.size.height <= ceiling, + "\(tool) tip is \(image.size.height)pt — past the \(ceiling)pt two-line ceiling" + ) if let out = ProcessInfo.processInfo.environment["ITSPAINT_TIP_DIR"], let tiff = image.tiffRepresentation, @@ -46,6 +68,37 @@ struct TooltipRenderTests { } } + /// **The check the size caps above cannot make.** + /// + /// A chip that measures itself for one line and then draws three is still + /// comfortably under 66pt — it is under it *because* the last two lines were + /// clipped away by the chip's own rounded-rectangle mask. Every assertion in + /// this file that bounds the chip from above gets *more* true as the bug gets + /// worse, which is the shape `docs/CHECKS_THAT_MISS.md` warns about. + /// + /// The only honest property is the one the eye uses: a longer sentence makes a + /// taller chip. No font arithmetic, no magic constant to tune. + @Test("A chip grows with the sentence it carries") + func chipHeightTracksItsDetail() throws { + func height(_ detail: String) throws -> CGFloat { + let view = Tooltip(title: "Copy image", shortcut: "⌘C", detail: detail) + .environment(\.colorScheme, .dark) + let renderer = ImageRenderer(content: view) + renderer.scale = 1 + return try #require(renderer.nsImage, "chip did not render").size.height + } + + let one = try height("Short.") + let two = try height("Puts it on the clipboard, ready to paste anywhere") + let three = try height( + "Puts it on the clipboard, ready to paste anywhere, in any app that " + + "happens to be open at the time" + ) + + #expect(two > one, "a two-line tip is \(two)pt — the same as a one-line one") + #expect(three > two, "a three-line tip is \(three)pt — the same as a two-line one") + } + /// A tip must not restate the name, which is directly above it. @Test func aTipSaysSomethingTheTitleDoesNot() { for tool in ToolKind.allCases { @@ -101,6 +154,63 @@ struct TooltipRenderTests { #expect(EditorView.tooltipTop >= Tokens.Chrome.titleReserve - Tokens.Space.snug) } + /// **The chip points at what it names, and stays in the window doing it.** + /// + /// The rail's chip used to be drawn by a second, dumber layer that offset it by + /// a constant — so hovering the eyedropper, nine cells down, put the answer up + /// beside the pencil, and a tall chip beside a low cell in a short window went + /// off the bottom entirely. One layer now, one rule: the chip's near edge sits + /// on a fixed line beside the chrome and slides along it to follow the control. + @Test("The chip follows the control it names and never leaves the window") + func theChipFollowsAndStaysInside() { + let window = CGRect(x: 0, y: 0, width: 900, height: 600) + let chip = CGSize(width: 220, height: 60) + + func origin(_ anchor: CGRect, rail: EditorModel.ChromeEdge) -> CGPoint { + EditorView.tooltipOrigin(beside: anchor, in: window, chip: chip, rail: rail) + } + + // A header control hangs from the header's one line, whatever the rail is + // doing — that is what makes reading along the header move the text + // sideways only. + let headerButton = CGRect(x: 440, y: 9, width: 26, height: 26) + for edge in [EditorModel.ChromeEdge.left, .bottom] { + #expect(origin(headerButton, rail: edge).y == EditorView.tooltipTop) + } + + // Down the side rail the chip stays in one column and tracks the cell. + let high = CGRect(x: 8, y: 120, width: 34, height: 34) + let low = CGRect(x: 8, y: 430, width: 34, height: 34) + let column = Tokens.Chrome.railInset + Tokens.Rail.thickness + Tokens.Space.snug + #expect(origin(high, rail: .left).x == column) + #expect(origin(low, rail: .left).x == column) + #expect(origin(low, rail: .left).y > origin(high, rail: .left).y, + "the chip did not follow the cell down the rail") + + // Along the bottom bar it is the other way round: one line, sliding + // sideways. + let leftCell = CGRect(x: 20, y: 545, width: 34, height: 34) + let rightCell = CGRect(x: 700, y: 545, width: 34, height: 34) + #expect(origin(leftCell, rail: .bottom).y == origin(rightCell, rail: .bottom).y) + #expect(origin(rightCell, rail: .bottom).x > origin(leftCell, rail: .bottom).x) + + // And at every extreme, both corners of the chip land inside the window. + let corners = [ + (CGRect(x: 8, y: 60, width: 34, height: 34), EditorModel.ChromeEdge.left), + (CGRect(x: 8, y: 590, width: 34, height: 34), .left), + (CGRect(x: 2, y: 545, width: 34, height: 34), .bottom), + (CGRect(x: 880, y: 545, width: 34, height: 34), .bottom), + (CGRect(x: 880, y: 9, width: 26, height: 26), .left), + ] + for (anchor, edge) in corners { + let point = origin(anchor, rail: edge) + #expect(point.x >= 0, "chip starts at x \(point.x)") + #expect(point.y >= 0, "chip starts at y \(point.y)") + #expect(point.x + chip.width <= window.width, "chip ends at x \(point.x + chip.width)") + #expect(point.y + chip.height <= window.height, "chip ends at y \(point.y + chip.height)") + } + } + /// The header's own chips carry a line of explanation, and the buttons they /// belong to are the ones people reported not recognising. They get the same /// size check the tools' tips get. @@ -112,18 +222,23 @@ struct TooltipRenderTests { ("Drag image out", "Pick the picture up and drop it into Slack, Mail or the Finder"), ("Signature", "Sign here, or drop in a signature you have already saved"), ("Duplicate", "Opens a copy in a new window"), + // The rail's colour chips, which briefly put three clauses and a live + // hex value in the *title* — a heading has no width cap, so the chip + // came out about 380pt wide and nothing in this file could see it. + ("Colour 1 · FF000059", "35% opaque. Double-click for another colour"), + ("Colour 2 · 00000000", "Fully transparent, so it rubs paint out. Click to use it as Colour 1"), + ("More colours", "Any colour, at any opacity. A fully clear one rubs paint out."), ] + let ceiling = try twoLineCeiling() + for (title, detail) in chips { - let view = Tooltip(title: title, shortcut: "⌘C", detail: detail) - .padding(10) - .environment(\.colorScheme, .dark) - .background(Color(white: 0.13)) - let renderer = ImageRenderer(content: view) - renderer.scale = 2 - let image = try #require(renderer.nsImage, "\(title) chip did not render") + let image = try render(title, "⌘C", detail) #expect(image.size.width <= 260, "\(title) chip is \(image.size.width)pt wide") - #expect(image.size.height <= 66, "\(title) chip wrapped to \(image.size.height)pt") + #expect( + image.size.height <= ceiling, + "\(title) chip is \(image.size.height)pt — past the \(ceiling)pt two-line ceiling" + ) } } } diff --git a/AppTests/WindowCaptureTests.swift b/AppTests/WindowCaptureTests.swift index 1ed9f75..5ac1367 100644 --- a/AppTests/WindowCaptureTests.swift +++ b/AppTests/WindowCaptureTests.swift @@ -69,13 +69,26 @@ struct WindowCaptureTests { let tool = ToolKind(rawValue: toolName) { document.model.selectTool(tool) } + // A translucent Colour 1 and a fully clear Colour 2 — the state the rail's + // checkerboard exists for, and one there is no other way to reach from a + // capture: it needs the system colour panel and a pointer. + if environment["ITSPAINT_CAPTURE_ALPHA"] != nil { + document.model.applySwatch( + PaintColour(red: 0.85, green: 0.15, blue: 0.15, alpha: 0.35), to: .foreground + ) + document.model.applySwatch(.clear, to: .background) + } if let grid = environment["ITSPAINT_CAPTURE_SNAP"], let spacing = Int(grid) { document.model.snapGrid = spacing } // The bottom rail lays its options panel out along the other axis, so a // control that only misbehaves down there needs a capture down there. - if environment["ITSPAINT_CAPTURE_EDGE"] == "bottom" { - document.model.chromeEdge = .bottom + // Named both ways: the edge is a saved preference, so a capture that only + // knows how to say "bottom" leaves the next run stuck down there. + switch environment["ITSPAINT_CAPTURE_EDGE"] { + case "bottom": document.model.chromeEdge = .bottom + case "side", "left": document.model.chromeEdge = .left + default: break } if environment["ITSPAINT_CAPTURE_SELECTION"] != nil { // Make the marquee with the select tool, then restore whichever tool @@ -89,6 +102,15 @@ struct WindowCaptureTests { document.model.noteChange(.empty) } + // A specific window size, because most of what goes wrong in chrome goes + // wrong at a size nobody opened it at. `WIDTHxHEIGHT`, in points. + if let size = environment["ITSPAINT_CAPTURE_SIZE"] { + let parts = size.split(separator: "x").compactMap { Double($0) } + if parts.count == 2 { + window.setContentSize(NSSize(width: parts[0], height: parts[1])) + } + } + // The traffic lights only take their colours once the app is active, // and activation is asynchronous. var patience = 100 diff --git a/ItsPaint.xcodeproj/project.pbxproj b/ItsPaint.xcodeproj/project.pbxproj index b2915be..fa62dc8 100644 --- a/ItsPaint.xcodeproj/project.pbxproj +++ b/ItsPaint.xcodeproj/project.pbxproj @@ -24,10 +24,12 @@ 4865D8A8471B873E6AC768C7 /* DraggedImage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5838B6603DAE5E9838603224 /* DraggedImage.swift */; }; 48905D56B247546BA33C44F8 /* ItsPaintApp.swift in Sources */ = {isa = PBXBuildFile; fileRef = 14E7B1F407FDB6F47C079660 /* ItsPaintApp.swift */; }; 4BC22C9E41431096754A0997 /* PaintKit in Frameworks */ = {isa = PBXBuildFile; productRef = 61964C2B3FE22724FE4E4A8B /* PaintKit */; }; + 50DD869D047AD6B3DBCA671F /* HeaderGeometryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D830570256790D00CC81E37C /* HeaderGeometryTests.swift */; }; 530689DA33232E54BB8DFD63 /* ToolRail.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7833326AD2D84AF29B6DE601 /* ToolRail.swift */; }; 531FB0243541A216A57A5D38 /* ImageServiceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CCB01892F8CC5C23F9945754 /* ImageServiceTests.swift */; }; 57E1D3BD09515B40DFDBEDD9 /* ColourPanelController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1EC319E836430BA852D8EDB0 /* ColourPanelController.swift */; }; 5B6FFEF7D300E81FCAE852C6 /* DesignTokens.swift in Sources */ = {isa = PBXBuildFile; fileRef = 392ADD8A7EEC36D1855BA3D5 /* DesignTokens.swift */; }; + 606F10A678F1341E41CE5CCB /* OptionMarkTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37F131FD6459BD946232DF6A /* OptionMarkTests.swift */; }; 623D371268183070226716CB /* SettingsWindow.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1BDFB5F76AC906C0238C2DA8 /* SettingsWindow.swift */; }; 62EE08BC08488DAE1C672EB1 /* EditorView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E86C21F2F8518B426159B77 /* EditorView.swift */; }; 6B8140C107D4A209AD6CD789 /* RotateSheet.swift in Sources */ = {isa = PBXBuildFile; fileRef = 86C35AB5A1478944CF8934D3 /* RotateSheet.swift */; }; @@ -35,7 +37,6 @@ 70251274389D468920E0FF44 /* DrawingDocument.swift in Sources */ = {isa = PBXBuildFile; fileRef = 18BC1714C722B756C83FFF6E /* DrawingDocument.swift */; }; 71B8FC7E198B4F762F94C6A2 /* DocumentTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 54F4A9F5632360B181403BDA /* DocumentTests.swift */; }; 736B873DA05CA677039DB143 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 81D5AB95221C3E1A9DD3CFC7 /* Assets.xcassets */; }; - 762CD1BC00FE680EB32BD80B /* ColourPopover.swift in Sources */ = {isa = PBXBuildFile; fileRef = C83A429AB8B62EFEDC176D65 /* ColourPopover.swift */; }; 769B43F0367492E56AB1DB43 /* SelectionBarTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A0C20D1B4B16258328D4D9D1 /* SelectionBarTests.swift */; }; 7D7E217925BBD597F47772AE /* PDFDocumentTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D072F2FA0424641934102963 /* PDFDocumentTests.swift */; }; 8CFB1C6C1779CAF721DD34EA /* FixedGrid.swift in Sources */ = {isa = PBXBuildFile; fileRef = E80562B1588560C2E61CCE65 /* FixedGrid.swift */; }; @@ -81,6 +82,7 @@ 20B04711CAED447B409481B1 /* EditorModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EditorModel.swift; sourceTree = ""; }; 30AE5E34F15348E6ACA11DFC /* ItsPaintTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = ItsPaintTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; 34D68AE3B45C7E5A426262AB /* IntentImageTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IntentImageTests.swift; sourceTree = ""; }; + 37F131FD6459BD946232DF6A /* OptionMarkTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OptionMarkTests.swift; sourceTree = ""; }; 392ADD8A7EEC36D1855BA3D5 /* DesignTokens.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DesignTokens.swift; sourceTree = ""; }; 3ABAEBA2B88281FDAA9CF273 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist; path = Info.plist; sourceTree = ""; }; 54F4A9F5632360B181403BDA /* DocumentTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DocumentTests.swift; sourceTree = ""; }; @@ -106,10 +108,10 @@ BC3E9F635AA81443BB4BF957 /* PrivacyInfo.xcprivacy */ = {isa = PBXFileReference; path = PrivacyInfo.xcprivacy; sourceTree = ""; }; BFEF09B51CF5FC0B71765BA1 /* DocumentCommands.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DocumentCommands.swift; sourceTree = ""; }; C06E4A12B57303E5FE405DFC /* EssentialsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EssentialsTests.swift; sourceTree = ""; }; - C83A429AB8B62EFEDC176D65 /* ColourPopover.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ColourPopover.swift; sourceTree = ""; }; CA3B7CF910C5BF4084044565 /* PaintKit */ = {isa = PBXFileReference; lastKnownFileType = folder; name = PaintKit; path = Packages/PaintKit; sourceTree = SOURCE_ROOT; }; CCB01892F8CC5C23F9945754 /* ImageServiceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageServiceTests.swift; sourceTree = ""; }; D072F2FA0424641934102963 /* PDFDocumentTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PDFDocumentTests.swift; sourceTree = ""; }; + D830570256790D00CC81E37C /* HeaderGeometryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HeaderGeometryTests.swift; sourceTree = ""; }; E7360C126943F6A4289FC4B7 /* ReduceMotionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReduceMotionTests.swift; sourceTree = ""; }; E80562B1588560C2E61CCE65 /* FixedGrid.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FixedGrid.swift; sourceTree = ""; }; EA6F6F30090979763C866F84 /* SignatureSheet.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SignatureSheet.swift; sourceTree = ""; }; @@ -152,7 +154,6 @@ children = ( 8468DB5919B3A21F63903298 /* CanvasOverlays.swift */, 1EC319E836430BA852D8EDB0 /* ColourPanelController.swift */, - C83A429AB8B62EFEDC176D65 /* ColourPopover.swift */, 392ADD8A7EEC36D1855BA3D5 /* DesignTokens.swift */, 6E86C21F2F8518B426159B77 /* EditorView.swift */, E80562B1588560C2E61CCE65 /* FixedGrid.swift */, @@ -190,8 +191,10 @@ 11CCE83773644C45A0171C62 /* CanvasRenderingTests.swift */, 54F4A9F5632360B181403BDA /* DocumentTests.swift */, C06E4A12B57303E5FE405DFC /* EssentialsTests.swift */, + D830570256790D00CC81E37C /* HeaderGeometryTests.swift */, CCB01892F8CC5C23F9945754 /* ImageServiceTests.swift */, 34D68AE3B45C7E5A426262AB /* IntentImageTests.swift */, + 37F131FD6459BD946232DF6A /* OptionMarkTests.swift */, B063B5E24D6EBB4CEB198E5B /* PasteGrowthTests.swift */, D072F2FA0424641934102963 /* PDFDocumentTests.swift */, E7360C126943F6A4289FC4B7 /* ReduceMotionTests.swift */, @@ -374,7 +377,6 @@ 43DF040CF6A5A908A865E871 /* CanvasScrollView.swift in Sources */, 8DE72B90DE9FDDBCEA63C2FD /* ClipboardHotKey.swift in Sources */, 57E1D3BD09515B40DFDBEDD9 /* ColourPanelController.swift in Sources */, - 762CD1BC00FE680EB32BD80B /* ColourPopover.swift in Sources */, 5B6FFEF7D300E81FCAE852C6 /* DesignTokens.swift in Sources */, 95E1D3A13CB44069D1DAE0AC /* DocumentCommands.swift in Sources */, 4865D8A8471B873E6AC768C7 /* DraggedImage.swift in Sources */, @@ -407,8 +409,10 @@ 397D8480B03F1D4E483412F6 /* CanvasRenderingTests.swift in Sources */, 71B8FC7E198B4F762F94C6A2 /* DocumentTests.swift in Sources */, 924C2B7A35DD5F3985A29406 /* EssentialsTests.swift in Sources */, + 50DD869D047AD6B3DBCA671F /* HeaderGeometryTests.swift in Sources */, 531FB0243541A216A57A5D38 /* ImageServiceTests.swift in Sources */, A8567364BE05578FC20DCD4C /* IntentImageTests.swift in Sources */, + 606F10A678F1341E41CE5CCB /* OptionMarkTests.swift in Sources */, 7D7E217925BBD597F47772AE /* PDFDocumentTests.swift in Sources */, 1DE130CDBF4B6616B66BF5F1 /* PasteGrowthTests.swift in Sources */, 9726B1CCB8E749B46B08CFF6 /* ReduceMotionTests.swift in Sources */, diff --git a/Packages/PaintKit/Sources/PaintKit/Codec/TextRenderer.swift b/Packages/PaintKit/Sources/PaintKit/Codec/TextRenderer.swift index 572499d..81a7088 100644 --- a/Packages/PaintKit/Sources/PaintKit/Codec/TextRenderer.swift +++ b/Packages/PaintKit/Sources/PaintKit/Codec/TextRenderer.swift @@ -138,9 +138,37 @@ public enum TextRenderer { // hollow; drawing the filled text over it leaves the stroke showing on // the outside only, which is the whole point — a rim that ate the letter // it was meant to make legible would be worse than no rim. - let wantsHalo = style.haloColour != nil && style.haloWidth > 0 - let passes = wantsHalo ? [frame(isHaloPass: true), frame(isHaloPass: false)] - : [frame(isHaloPass: false)] + // **Text set in a transparent colour cuts the letters out of the picture.** + // + // Core Graphics cannot express that as a colour: source-over with a zero + // alpha source is a no-op, so typing in a clear colour changed nothing and + // said nothing — the same silence every other tool used to keep. The + // letterforms are drawn opaque and the blend mode does the erasing, which + // is what `destination-out` is: it takes the *source's* alpha away from + // the destination and ignores the source's colour entirely. + // + // The halo goes with it. A rim around a hole is a rim around nothing. + let erases = style.colour.alpha == 0 + var opaque = style + if erases { + opaque.colour = PaintColour(red: 0, green: 0, blue: 0, alpha: 1) + opaque.haloColour = nil + } + func erasingFrame() -> CTFrame { + let attributed = attributedString(string, style: opaque, isHaloPass: false) + return CTFramesetterCreateFrame( + CTFramesetterCreateWithAttributedString(attributed), CFRangeMake(0, 0), path, nil + ) + } + + let wantsHalo = !erases && style.haloColour != nil && style.haloWidth > 0 + let passes = if erases { + [erasingFrame()] + } else if wantsHalo { + [frame(isHaloPass: true), frame(isHaloPass: false)] + } else { + [frame(isHaloPass: false)] + } bitmap.drawWithCoreGraphics { context in // A bitmap `CGContext` does not turn these on for you, and without them @@ -161,6 +189,7 @@ public enum TextRenderer { context.setShouldSubpixelPositionFonts(true) context.setAllowsFontSubpixelQuantization(false) context.setShouldSubpixelQuantizeFonts(false) + if erases { context.setBlendMode(.destinationOut) } for pass in passes { context.saveGState() // The surrounding context is flipped so canvas coordinates work; diff --git a/Packages/PaintKit/Sources/PaintKit/Pixels/Bitmap.swift b/Packages/PaintKit/Sources/PaintKit/Pixels/Bitmap.swift index 90c8d50..ef3a17a 100644 --- a/Packages/PaintKit/Sources/PaintKit/Pixels/Bitmap.swift +++ b/Packages/PaintKit/Sources/PaintKit/Pixels/Bitmap.swift @@ -174,10 +174,18 @@ public struct Bitmap: Equatable, Sendable { } /// Source-over composite a solid colour across `rect`. + /// + /// **Except at zero alpha, where it takes the paint away instead.** Source-over + /// with a transparent source is arithmetically a no-op, so a filled shape drawn + /// in a transparent colour used to leave the canvas untouched — the same silent + /// nothing `Raster.stamp` used to do, and the same fix: a colour with no colour + /// in it means "remove what is here". The bucket has meant that since the first + /// build. public mutating func blend(_ rect: PixelRect, with colour: RGBA8) { if colour.a == 255 { return fill(rect, with: colour) } + if colour.a == 0 { return fill(rect, with: .clear) } let clipped = rect.intersection(bounds) - guard !clipped.isEmpty, colour.a > 0 else { return } + guard !clipped.isEmpty else { return } for row in clipped.minY..= runs.on { continue } } + let i = bitmap.index(PixelPoint(x: x, y: y)) + + // **The antialiased path erases too.** + // + // This is the one that mattered most and the one that was missed: + // the shipping default is a round brush with smooth edges, and + // `PaintEngine.strokePath` sends exactly that combination here. So + // a transparent colour rubbed out a single dot at mouse-down — + // `beginStroke` stamps directly — and then did nothing for the + // rest of the drag. Half a fix reads worse than none, because the + // first dot proves the feature works. + // + // Same arithmetic as `stamp`: destination-out, scaled by the + // antialiasing coverage, which is `withCoverage` on the + // destination because these pixels are premultiplied. A smooth + // edge therefore erases with a smooth edge. + if colour.a == 0 { + bitmap.pixels[i] = bitmap.pixels[i] + .withCoverage(UInt8(((1 - coverage) * 255).rounded())) + dirty = dirty.union(PixelRect(x: x, y: y, width: 1, height: 1)) + continue + } + let alpha = Double(colour.a) * coverage let src = RGBA8( r: UInt8((Double(colour.r) * coverage).rounded()), @@ -269,7 +317,6 @@ public enum Raster { a: UInt8(alpha.rounded()) ) guard src.a > 0 else { continue } - let i = bitmap.index(PixelPoint(x: x, y: y)) bitmap.pixels[i] = src.overCompositing(bitmap.pixels[i]) dirty = dirty.union(PixelRect(x: x, y: y, width: 1, height: 1)) } @@ -600,7 +647,11 @@ public enum Raster { ) guard bitmap.isInBounds(target) else { continue } let index = bitmap.index(target) - bitmap.pixels[index] = colour.overCompositing(bitmap.pixels[index]) + // A dot lands or it does not, so a transparent airbrush takes the + // pixel out entirely — the same meaning every other tool now gives it. + bitmap.pixels[index] = colour.a == 0 + ? .clear + : colour.overCompositing(bitmap.pixels[index]) dirty = dirty.union(PixelRect(x: target.x, y: target.y, width: 1, height: 1)) } return dirty diff --git a/Packages/PaintKit/Tests/PaintKitTests/TransparentPaintTests.swift b/Packages/PaintKit/Tests/PaintKitTests/TransparentPaintTests.swift new file mode 100644 index 0000000..183e87f --- /dev/null +++ b/Packages/PaintKit/Tests/PaintKitTests/TransparentPaintTests.swift @@ -0,0 +1,148 @@ +import Testing +@testable import PaintKit + +/// **A colour with no colour in it takes paint away.** +/// +/// The bucket has meant that since the first build — `SelectionMask.fill` assigns +/// the colour straight into the pixels, so filling a region with a transparent +/// colour knocks a hole in it. Every other tool disagreed, and disagreed silently: +/// source-over compositing with a transparent source is `out = dst`, so the pencil, +/// the brush, the shapes, the text and the *eraser* all did precisely nothing and +/// said nothing about it. +/// +/// The eraser is the one that mattered. It paints Colour 2, and a transparent +/// Colour 2 is exactly what somebody loads when they want a hole — so the app's +/// most obvious route to transparency was the one route that was dead. +@Suite("Painting with a transparent colour") +struct TransparentPaintTests { + + private func opaqueCanvas() -> Bitmap { + Bitmap(width: 16, height: 16, fill: RGBA8(r: 255, g: 0, b: 0, a: 255)) + } + + @Test("A hard brush loaded with a transparent colour clears what it covers") + func stampErases() { + var canvas = opaqueCanvas() + Raster.stamp(Brush(shape: .square, size: 4), colour: .clear, at: PixelPoint(x: 8, y: 8), into: &canvas) + + #expect(canvas.pixel(at: PixelPoint(x: 8, y: 8))! == .clear, "a transparent stamp left the pixel at \(canvas.pixel(at: PixelPoint(x: 8, y: 8))!)") + // And only where it covered: an erase that runs to the edges is a bug + // wearing the same result as a working one on a small canvas. + #expect(canvas.pixel(at: PixelPoint(x: 0, y: 0))!.a == 255, "the stamp cleared a pixel it never touched") + } + + /// The property that makes this worth doing rather than special-casing the + /// eraser: a soft brush erases *softly*, because coverage does the scaling. + @Test("A soft brush erases by the same coverage it would have painted by") + func softStampFeathers() { + var painted = opaqueCanvas() + var erased = opaqueCanvas() + let brush = Brush(shape: .soft, size: 9) + let at = PixelPoint(x: 8, y: 8) + + Raster.stamp(brush, colour: RGBA8(r: 0, g: 0, b: 255, a: 255), at: at, into: &painted) + Raster.stamp(brush, colour: .clear, at: at, into: &erased) + + // Wherever the brush laid down blue, it took away exactly that much red. + for y in 4..<13 { + for x in 4..<13 { + let laid = Int(painted.pixel(at: PixelPoint(x: x, y: y))!.b) + let left = Int(erased.pixel(at: PixelPoint(x: x, y: y))!.a) + #expect(abs((255 - laid) - left) <= 1, "at \(x),\(y): laid \(laid), left \(left)") + } + } + } + + @Test("A filled shape in a transparent colour cuts a hole rather than doing nothing") + func blendErases() { + var canvas = opaqueCanvas() + canvas.blend(PixelRect(x: 2, y: 2, width: 4, height: 4), with: .clear) + + #expect(canvas.pixel(at: PixelPoint(x: 3, y: 3))! == .clear) + #expect(canvas.pixel(at: PixelPoint(x: 10, y: 10))!.a == 255) + } + + /// **A drag, not a click — and every freehand tool, not one.** + /// + /// The first version of this test pressed and released on the same pixel, + /// which only ever reaches `PaintEngine.beginStroke`'s direct `Raster.stamp`. + /// It passed with the entire antialiased stroke path still dead, and that path + /// is the *shipping default*: a round brush with smooth edges. So a + /// transparent colour rubbed out one dot under the press and then did nothing + /// for the rest of the drag — which is worse than doing nothing at all, + /// because the first dot proves the feature works. + /// + /// A test that cannot see the middle of a stroke cannot check a stroke. + @Test( + "Every freehand tool erases along a whole drag when its colour is transparent", + arguments: [ToolKind.eraser, .brush, .pencil] + ) + func freehandToolsEraseAlongADrag(tool: ToolKind) { + let engine = PaintEngine(canvas: Bitmap(width: 64, height: 32, fill: RGBA8(r: 0, g: 0, b: 0, a: 255))) + engine.settings.tool = tool + engine.settings.brushSize = 6 + // The eraser paints Colour 2; everything else paints Colour 1. + engine.colours.background = .clear + engine.colours.foreground = .clear + + engine.beginStroke(at: PixelPoint(x: 8, y: 16)) + engine.continueStroke(to: PixelPoint(x: 32, y: 16)) + _ = engine.endStroke(at: PixelPoint(x: 56, y: 16)) + + // The middle of the drag, which is the part a press-and-release cannot see. + for x in [8, 32, 56] { + let pixel = engine.canvas.pixel(at: PixelPoint(x: x, y: 16))! + #expect(pixel.a == 0, "\(tool) left \(pixel) at x \(x)") + } + } + + /// The airbrush lays loose dots rather than a continuous band, so it has its + /// own compositor and its own way of having been missed. + @Test("A transparent airbrush takes paint out") + func sprayErases() { + let engine = PaintEngine(canvas: Bitmap(width: 48, height: 48, fill: RGBA8(r: 0, g: 0, b: 0, a: 255))) + engine.settings.tool = .brush + engine.settings.brushShape = .spray + engine.settings.brushSize = 16 + engine.settings.sprayDensity = 0.6 + engine.colours.foreground = .clear + + engine.beginStroke(at: PixelPoint(x: 24, y: 24)) + engine.continueStroke(to: PixelPoint(x: 24, y: 24)) + _ = engine.endStroke(at: PixelPoint(x: 24, y: 24)) + + let cleared = (0..<48).flatMap { y in (0..<48).map { engine.canvas.pixel(at: PixelPoint(x: $0, y: y))! } } + .filter { $0.a == 0 } + #expect(!cleared.isEmpty, "the airbrush cleared nothing") + } + + /// Text set in a transparent colour cuts the letters out. Core Graphics cannot + /// say that with a colour — source-over at zero alpha is a no-op — so it is + /// said with a blend mode instead. + @Test("Transparent text knocks a hole in the shape of the letters") + func textErases() { + var canvas = Bitmap(width: 200, height: 60, fill: RGBA8(r: 0, g: 0, b: 255, a: 255)) + TextRenderer.draw( + "Hole", + in: PixelRect(x: 4, y: 4, width: 192, height: 52), + style: TextRenderer.Style(fontName: "Helvetica", pointSize: 36, colour: .clear), + into: &canvas + ) + + let cleared = (0..<60).flatMap { y in (0..<200).map { canvas.pixel(at: PixelPoint(x: $0, y: y))! } } + .filter { $0.a == 0 } + #expect(cleared.count > 50, "only \(cleared.count) pixels went clear") + } + + /// And the case that still has to behave exactly as it did: *partial* alpha is + /// paint, not erasure. Nothing above may turn a 50% wash into a 50% hole. + @Test("Partial alpha still composites, and still darkens what is under it") + func partialAlphaStillPaints() { + var canvas = Bitmap(width: 8, height: 8, fill: RGBA8(r: 255, g: 255, b: 255, a: 255)) + // Premultiplied: half-alpha black is (0, 0, 0, 128). + canvas.blend(canvas.bounds, with: RGBA8(r: 0, g: 0, b: 0, a: 128)) + + #expect(canvas.pixel(at: PixelPoint(x: 4, y: 4))!.a == 255, "compositing over opaque paint must stay opaque") + #expect(canvas.pixel(at: PixelPoint(x: 4, y: 4))!.r < 200, "a half-alpha black wash did not darken the page") + } +} diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e9b103b..6d05b1c 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -68,7 +68,7 @@ App/ ToolOptions.swift the active tool's options, expanded from its button EditorView.swift window layout: canvas, chrome, status, title DesignTokens.swift spacing/size/radius/motion tokens + chrome material - ColourPopover.swift · SizeSheet.swift · Tooltip.swift · CanvasOverlays.swift + SizeSheet.swift · Tooltip.swift · CanvasOverlays.swift FixedGrid.swift deterministic grid (see "Grids", below) ```