diff --git a/ios/HouseScan/Contract/ScanContract.swift b/ios/HouseScan/Contract/ScanContract.swift index 904052b06..25135c1ec 100644 --- a/ios/HouseScan/Contract/ScanContract.swift +++ b/ios/HouseScan/Contract/ScanContract.swift @@ -757,6 +757,9 @@ struct ResultPresentation: Equatable, Sendable { var checks: [CheckRow] var clearances: [ClearanceZone] var missing: [MissingEvidence] + /// The server's sentence for each mark it set aside past an unexplored end + /// (`PlacementResult.objectsNotUsed`), so the homeowner knows it wasn't counted (#42). + var marksNotUsed: [String] = [] /// Where the scan stopped on a side it didn't finish, nearer the meter than the spot, so a /// closer spot may lie past it (`PlacementResult.closerUnseenEnd`). var unseenEnd: UnseenEnd? = nil diff --git a/ios/HouseScan/HouseScanApp.swift b/ios/HouseScan/HouseScanApp.swift index 3bcd66c8d..9d3092c7b 100644 --- a/ios/HouseScan/HouseScanApp.swift +++ b/ios/HouseScan/HouseScanApp.swift @@ -16,6 +16,9 @@ struct HouseScanApp: App { if let index = arguments.firstIndex(of: "-engineGuidanceCheck"), index + 1 < arguments.count { return ScanEngine.checkGuidanceEngine(arguments[index + 1]) } + if arguments.contains("-enginePastEndReplayCheck") { + return ScanEngine.checkPastEndReplay() + } #endif return nil }() diff --git a/ios/HouseScan/Runtime/LiveCapture.swift b/ios/HouseScan/Runtime/LiveCapture.swift index 62ce41a8b..4395a7075 100644 --- a/ios/HouseScan/Runtime/LiveCapture.swift +++ b/ios/HouseScan/Runtime/LiveCapture.swift @@ -50,8 +50,6 @@ final class LiveCapture { /// True when the mesh can carry ARKit's per-face classification (wall, floor, door, ...), /// which the packet's mesh records. static var supportsClassifiedMesh: Bool { ARWorldTrackingConfiguration.supportsSceneReconstruction(.meshWithClassification) } - /// True when this phone can hide virtual content behind a person standing in front of it. - static var supportsPeopleOcclusion: Bool { ARWorldTrackingConfiguration.supportsFrameSemantics(.personSegmentationWithDepth) } /// The AR result in the camera view, while one is shown (`showResult`). private var result: AnchorEntity? @@ -84,7 +82,10 @@ final class LiveCapture { configuration.sceneReconstruction = .mesh } if supportsDepth { configuration.frameSemantics.insert(.sceneDepth) } - if occludingPeople, supportsPeopleOcclusion { configuration.frameSemantics.insert(.personSegmentationWithDepth) } + // Asked of the set that will run, not of people occlusion alone: a phone could support + // each semantic by itself and not both with scene depth (review of #51). + let withPeople = configuration.frameSemantics.union(.personSegmentationWithDepth) + if occludingPeople, ARWorldTrackingConfiguration.supportsFrameSemantics(withPeople) { configuration.frameSemantics = withPeople } return configuration } @@ -170,11 +171,16 @@ final class LiveCapture { /// without the view: the AR scene still has the result, whether or not it is in view /// (`ResultOverlayPolicy.update(drawn:held:time:)`). `anchored` leaves out whether it is /// enabled: the engine disables it while tracking is limited (`setResultVisible`). + /// + /// Anchored counts only while the meter's anchor is still in the session. An anchor entity + /// can report itself anchored after ARKit has dropped the anchor it follows, and the screen's + /// own drawing should take over then rather than wait on a scene that lost its place + /// (review of #132 and #137). func resultIsDrawn() -> (drawn: Bool, held: Bool, anchored: Bool) { guard let result, let resultFocus else { return (false, false, false) } let id = delegate.shared.withLock { $0.meterAnchorID } let inSession = id.map { id in arView.session.currentFrame?.anchors.contains(where: { $0.identifier == id }) == true } ?? false - let anchored = result.isAnchored + let anchored = result.isAnchored && inSession let enabled = result.isEnabledInHierarchy let world = resultFocus.position(relativeTo: nil) let inCamera = arView.cameraTransform.matrix.inverse * SIMD4(world, 1) diff --git a/ios/HouseScan/Runtime/ScanEngine+Actions.swift b/ios/HouseScan/Runtime/ScanEngine+Actions.swift index f7891cc86..ee29e537b 100644 --- a/ios/HouseScan/Runtime/ScanEngine+Actions.swift +++ b/ios/HouseScan/Runtime/ScanEngine+Actions.swift @@ -92,6 +92,7 @@ extension ScanEngine: ScanActions { guard state.phase == .meterCloseUp else { return } state.closeUp = .skipped state.meterNumber = .skipped + state.meterBrand = nil // A photo on disk (one the reader turned down, or one confirmed just before the skip) // stays a plain photo of the scan: scene.json doesn't list it and the meter mark doesn't // link it. @@ -472,6 +473,7 @@ extension ScanEngine: ScanActions { func deleteFeature(_ id: UUID) { state.features.removeAll { $0.id == id } + publishFeaturesPastEnds() } func setWindowOpens(_ id: UUID, opens: Bool?) { diff --git a/ios/HouseScan/Runtime/ScanEngine+Export.swift b/ios/HouseScan/Runtime/ScanEngine+Export.swift index 1c1832946..9b239485b 100644 --- a/ios/HouseScan/Runtime/ScanEngine+Export.swift +++ b/ios/HouseScan/Runtime/ScanEngine+Export.swift @@ -307,6 +307,7 @@ extension ScanEngine { checks: checks, clearances: clearances, missing: missing, + marksNotUsed: result.objectsNotUsed.map(\.message), unseenEnd: unseen, isSample: isSample, rulesSHA256: result.policy.rulesSHA256 diff --git a/ios/HouseScan/Runtime/ScanEngine.swift b/ios/HouseScan/Runtime/ScanEngine.swift index a1e5dab66..2b49b2049 100644 --- a/ios/HouseScan/Runtime/ScanEngine.swift +++ b/ios/HouseScan/Runtime/ScanEngine.swift @@ -127,6 +127,9 @@ final class ScanEngine { private var gapCounter = 0 /// Keyframes stored when the current gap request began: a request is closed only by new views. private var keyframesAtGapStart = 0 + /// The walk-out request's "the space ends" reading, with the request it belongs to + /// (`noteWalkOut`). + private var spaceEndsShown: (request: Int, reading: SlowRelease)? /// Tilt-up views kept when the current gap request began: an overhead request is closed by a /// new one (`keepOverheadView`); counting keyframes would not do, since the walk keeps them too. private var overheadViewsAtGapStart = 0 @@ -189,7 +192,8 @@ final class ScanEngine { /// Watches whether the AR scene draws the result (`watchResultInCamera`) while "See it on /// your wall" is up on the live camera. private var resultWatch: Task? - /// The wall the model in the AR scene was built from; nil when none is in it. + /// The wall the AR result's model was built from, in the frame of the meter's anchor at the + /// time (`ScanEngine.inAnchorFrame`). private var resultBuiltFor: WallGeometry? // Upload @@ -363,7 +367,9 @@ final class ScanEngine { closeUpRetake = nil meterReadout = nil closeUpCredit = CloseUpCredit() + // The brand belongs to the number it was read with, so it goes with it (review of #89). state.meterNumber = nil + state.meterBrand = nil state.closeUpFailedAttempts = 0 live?.setMode(.closeUp) if let replay { replay.play(range: 0.. (target: SIMD3, standOut: Float) { let standOff = planner.config.standOff @@ -1708,6 +1728,7 @@ final class ScanEngine { state.target = nil state.path = [] state.features = [] + state.featuresPastEnds = [] // A mark half placed holds taps in the old frame's wall coordinates. state.marking = nil pendingTaps = [] @@ -1858,10 +1879,12 @@ final class ScanEngine { guard LiveCapture.supportsMesh, let live, let wall = state.wall, let result = state.result, let pose = meterTracking?.pose, state.spatialResultAvailable else { return } if rising || resultWatch == nil { watchResultInCamera() } - // Each rebuild takes the model out of the scene and puts a new one in, so a wall that - // moved by a centimeter or two (the ground or the meter's anchor refined) keeps the one - // it has. It hangs on the meter's anchor, so it follows the anchor's corrections anyway. - if !rising, let built = resultBuiltFor, Self.movedLittle(from: built, to: wall) { return } + // Each rebuild takes the model out of the scene and puts a new one in, cutting short its + // rise. It hangs on the meter's anchor, so it follows the anchor's corrections anyway: + // the wall is compared in the anchor's frame, and only a change to the wall itself (the + // ground or an end refined by more than a few centimeters) rebuilds it (review of #51). + let local = Self.inAnchorFrame(wall, pose: pose) + if !rising, let built = resultBuiltFor, Self.movedLittle(from: built, to: local) { return } let model = ResultARModel.build(wall: wall, result: result) // The battery's middle (the outline's, on the ground, for a spot that isn't clean), or the // meter without a spot, in the model's coordinates. @@ -1870,7 +1893,7 @@ final class ScanEngine { resultBuiltFor = nil return } - resultBuiltFor = wall + resultBuiltFor = local live.setResultVisible(state.tracking == .normal) if rising { ResultARModel.rise(model) } } @@ -1913,6 +1936,37 @@ final class ScanEngine { private static let resultRebuildDistance: Float = 0.05 private static let resultRebuildTurnCosine: Float = cos(2 * Float.pi / 180) + /// `wall` in the frame of the meter's anchor at `pose`: what the result's model is built + /// from, less the anchor's own place and heading, which the model follows anyway. The ends + /// and spans are already measured along the wall. A corner piece's anchor is an offset from + /// the meter's foot, so it turns like a direction. + static func inAnchorFrame(_ wall: WallGeometry, pose: simd_float4x4) -> WallGeometry { + let inverse = pose.inverse + func point(_ p: SIMD3) -> SIMD3 { + let v = inverse * SIMD4(p, 1) + return SIMD3(v.x, v.y, v.z) + } + func direction(_ d: SIMD3) -> SIMD3 { + let v = inverse * SIMD4(d, 0) + return SIMD3(v.x, v.y, v.z) + } + var local = wall + local.meter = point(wall.meter) + local.along = direction(wall.along) + local.outward = direction(wall.outward) + // Under .gravity alignment the anchor's corrections turn about the vertical, so height + // above the anchor is the ground's place in its frame. + local.groundY = wall.groundY - pose.columns.3.y + local.cornerSegments = wall.cornerSegments.map { piece in + var piece = piece + piece.anchor = direction(piece.anchor) + piece.along = direction(piece.along) + piece.outward = direction(piece.outward) + return piece + } + return local + } + /// True when `new` is `old` moved by less than `resultRebuildDistance` and turned by less /// than 2° in every part the model is built from: the meter, the ground, the ends and each /// piece of wall. @@ -2035,6 +2089,7 @@ final class ScanEngine { gapPlan = plan keyframesAtGapStart = store.keyframes.count overheadViewsAtGapStart = coverage?.overheadCameras.count ?? 0 + spaceEndsShown = nil let progress = coverage.map { gapPlanner.progress(of: plan, $0) } ?? 0 state.gap = GapRequest( id: gapCounter, origin: origin, reason: reason, band: plan.band == .ground ? .ground : .wall, span: plan.span, @@ -2463,6 +2518,7 @@ final class ScanEngine { state.wall = nil state.coverage = .empty state.features = [] + state.featuresPastEnds = [] state.groundAnswer = nil state.marking = nil state.gap = nil @@ -2478,6 +2534,7 @@ final class ScanEngine { state.closeUp = .aiming(hold: 0, problem: nil) state.closeUpFailedAttempts = 0 state.meterNumber = nil + state.meterBrand = nil closeUpRetake = nil meterReadout = nil state.isPracticeScan = false @@ -2680,6 +2737,39 @@ extension ScanEngine { #if DEBUG extension ScanEngine { + /// Checks the server-request entry point without running a camera or sending a capture. + static func checkPastEndReplay() -> String { + guard let wall = WallFrame(meter: SIMD3(0, 1.5, 0), outward: SIMD3(0, 0, 1), groundY: 0), + let item = try? JSONDecoder().decode(PlacementMissingEvidence.self, + from: Data(#"{"kind":"past_end","side":"right","message":"Look past the end"}"#.utf8)) else { + return "invalid past-end fixture" + } + for count in [2, 10] { + let engine = ScanEngine(options: LaunchOptions(arguments: ["-sampleResult"], defaultServerURL: nil)) + var map = CoverageMap(wall: wall) + map.setEnd(.right, at: 0) + for index in 0.. 0 else { return "request lost restored sightings" } + if count == 2, request.progress >= 1 { return "partial fixture unexpectedly completed coverage" } + if count == 10, request.progress != 1 { return "complete fixture lost coverage" } + guard !request.isSatisfied else { return "old sightings bypassed the fresh-photo requirement" } + } + return "passed" + } + /// Deterministic integration checks launched by EngineGuidanceTests in the existing journey /// target. Live sessions are never run; no replay, network, or elapsed-time waits participate. static func checkGuidanceEngine(_ scenario: String) -> String { diff --git a/ios/HouseScan/UI/Camera/CameraOverlays.swift b/ios/HouseScan/UI/Camera/CameraOverlays.swift index 22ecd5db0..e917f261c 100644 --- a/ios/HouseScan/UI/Camera/CameraOverlays.swift +++ b/ios/HouseScan/UI/Camera/CameraOverlays.swift @@ -10,6 +10,9 @@ struct CameraOverlays: View { /// The open camera between the card and the actions (`CameraChrome.cameraWindow`), for the /// aim ring to stay inside. Passed through unread, so its changes don't redraw the fog. var cameraWindow: CameraWindow? = nil + /// Set to what stands for the aim step's target on screen (`WayfindingOverlay.onAimMarker`), + /// for the screen's card to name it. Nil on screens whose card doesn't. + var aimMarker: Binding? = nil /// The step the aim ring's legend first showed with. The legend explains the first ring that /// fills and retires once that step ends (#81); it isn't needed on every ring after. @@ -83,10 +86,14 @@ struct CameraOverlays: View { wall: wall, path: state.path, target: state.target, + step: state.guidance, progress: state.aimProgress, completed: heldTarget, onFillingRingShown: { shown in fillingRingShown = shown }, - cameraWindow: cameraWindow + cameraWindow: cameraWindow, + onAimMarker: { marker in + if let aimMarker, aimMarker.wrappedValue != marker { aimMarker.wrappedValue = marker } + } ) .transition(.opacity) } diff --git a/ios/HouseScan/UI/Camera/WayfindingOverlay.swift b/ios/HouseScan/UI/Camera/WayfindingOverlay.swift index 07600b1e2..0f856079b 100644 --- a/ios/HouseScan/UI/Camera/WayfindingOverlay.swift +++ b/ios/HouseScan/UI/Camera/WayfindingOverlay.swift @@ -1,3 +1,4 @@ +import HouseScanKit import simd import SwiftUI @@ -14,6 +15,7 @@ struct WayfindingOverlay: View { var wall: WallGeometry? var path: [SIMD3] var target: SIMD3? + var step: GuidanceStep /// How far the aim task is toward done, 0...1 (`ScanViewState.aimProgress`): the ring fills /// with it. Nil when the target only marks a place. var progress: Double? = nil @@ -29,6 +31,27 @@ struct WayfindingOverlay: View { /// shows only inside it and the edge arrows keep to it. Until the chrome has been laid out, /// fixed bands stand in. var cameraWindow: CameraWindow? = nil + /// Called with what stands for an aim step's filling target on screen: its ring, or an arrow + /// and the way it points, or covered when neither can be drawn; nil without an aim. The + /// card's title follows it, so the words name what is drawn: the camera's angle to the target and the open part of the + /// screen disagree near the card and the screen's edges (review of #120). + var onAimMarker: ((AimMarker?) -> Void)? = nil + + /// Remember the marker with the exact aim that produced it. The planner can change targets + /// without replacing this view, so another aim must not inherit its axis or entry margin. + private struct Report: Equatable { + var step: GuidanceStep + var target: SIMD3? + var marker: AimMarker? + } + + @State private var reported: Report? + + /// Check identity before drawing, not in a later reset callback that leaves one stale frame. + private var previousMarker: AimMarker? { + guard let reported, reported.step == step, reported.target == target else { return nil } + return reported.marker + } var body: some View { // Read here, in this view's body, so a change redraws this overlay alone. @@ -43,8 +66,10 @@ struct WayfindingOverlay: View { .accessibilityHidden(true) // One marker either way, so a ring that completes keeps its identity and animates // from its last fill to green. + let semanticMarker = aimMarker(in: size, band: band) if let shown = marker(in: size, band: band) { - TargetMarker(placement: shown.placement, progress: shown.progress) + let direction: AimDirection? = if case .arrow(let direction)? = semanticMarker { direction } else { nil } + TargetMarker(placement: shown.placement, progress: shown.progress, aimDirection: direction) } if fillingTargetOnScreen(in: size) { Color.clear @@ -52,6 +77,13 @@ struct WayfindingOverlay: View { .onAppear { onFillingRingShown?(true) } .onDisappear { onFillingRingShown?(false) } } + Color.clear + .frame(width: 0, height: 0) + .onChange(of: Report(step: step, target: target, marker: aimMarker(in: size, band: band)), initial: true) { _, report in + reported = report + onAimMarker?(report.marker) + } + .onDisappear { onAimMarker?(nil) } } .frame(width: size.width, height: size.height) } @@ -70,12 +102,15 @@ struct WayfindingOverlay: View { /// The edge arrow's disc (`TargetMarker`). static let arrowDiameter: CGFloat = 52 + /// How far inside the ring's bounds an arrow's target must come before the ring replaces it + /// (`reported`). A display choice, about a fingertip; not measured on a phone. + static let ringEntryMargin: CGFloat = 12 /// How far inside the band the edge arrows sit: half the arrow's disc and a margin. static let arrowInset: CGFloat = 32 /// The measured window in this view's coordinates, cut to the part on screen: scrolling the /// chrome at large text sizes can carry the window partly past an edge. - private static func verticalBand(_ clearArea: CGRect?, origin: CGPoint, height: CGFloat) -> Band? { + static func verticalBand(_ clearArea: CGRect?, origin: CGPoint, height: CGFloat) -> Band? { clearArea.map { let top = min(max($0.minY - origin.y, 0), height) return Band(top: top, bottom: max(min($0.maxY - origin.y, height), top)) @@ -135,21 +170,65 @@ struct WayfindingOverlay: View { if case .onScreen = held { return (placement: held, progress: 1.0) } } guard let target else { return nil } - return (placement: placement(for: target, in: size, band: band), progress: progress) + return (placement: placement(for: target, in: size, band: band, entryMargin: targetEntryMargin), progress: progress) + } + + /// A new aim starts without a margin. Once its arrow shows, it must come farther inside + /// before becoming a ring; a ring keeps its place to the edge without flickering there. + private var targetEntryMargin: CGFloat { + progress == nil || previousMarker == nil || previousMarker == .ring ? 0 : Self.ringEntryMargin + } + + /// What the card can name for an aim target (`onAimMarker`). `.covered` means the chrome + /// hides it; nil while a completed ring holds the screen or off an aim step. + private func aimMarker(in size: CGSize, band: Band?) -> AimMarker? { + guard progress != nil, let target else { return nil } + // A completed ring holding the screen belongs to the step before (`marker`). + if let completed, case .onScreen = placement(for: completed, in: size, band: band) { return nil } + switch placement(for: target, in: size, band: band, entryMargin: targetEntryMargin) { + case .onScreen: + return .ring + case .offScreen(_, let angle): + let previous: AimDirection? = if case .arrow(let direction)? = previousMarker { direction } else { nil } + return .arrow(Self.direction(of: angle, previous: previous)) + case .hidden: + return .covered + } + } + + /// The way an arrow at `angle` (screen, y down) points, in the card's terms. With the + /// direction named before, the other axis must be `AimHint.axisBias` times larger to change + /// it, as for the hint itself. + static func direction(of angle: Angle, previous: AimDirection?) -> AimDirection { + let prior: OpenCameraDirection.Direction? = previous.flatMap { + switch $0 { + case .above: .above + case .below: .below + case .left: .left + case .right: .right + case .onScreen, .behind: nil + } + } + switch OpenCameraDirection.classify(CGVector(dx: cos(angle.radians), dy: sin(angle.radians)), previous: prior) { + case .above: return .above + case .below: return .below + case .left: return .left + case .right: return .right + } } - private func placement(for target: SIMD3, in size: CGSize, band: Band?) -> TargetMarker.Placement { + private func placement(for target: SIMD3, in size: CGSize, band: Band?, entryMargin: CGFloat = 0) -> TargetMarker.Placement { // Less of the open camera on screen than an arrow's disc: at the largest text sizes the // chrome has scrolled it away, and anything drawn would sit on the card or the actions. if let band, band.bottom - band.top < Self.arrowDiameter { return .hidden } if let point = projection.viewPoint(for: target, in: size) { let scale = wall.flatMap { WallProjection(projection: projection, wall: $0, size: size).pointsPerMeter(at: target) } let radius = min(64, max(30, (scale ?? 90) * 0.28)) - if Self.ringBounds(in: size, band: band, radius: radius).contains(point) { + if Self.ringBounds(in: size, band: band, radius: radius).insetBy(dx: entryMargin, dy: entryMargin).contains(point) { return .onScreen(point, radius: radius) } // Near the card or the actions a smaller ring can still fit whole. - if let band, let fitted = Self.fittedRadius(at: point, in: size, band: band), fitted < radius { + if let band, let fitted = Self.fittedRadius(at: point, in: size, band: band, margin: entryMargin), fitted < radius { return .onScreen(point, radius: fitted) } // On screen but under the card or the actions: the arrow in the open camera points @@ -198,9 +277,10 @@ struct WayfindingOverlay: View { } /// The largest ring radius, down to the smallest ring's 30 pt, that fits whole at `point` in - /// the band and on screen, pulse included. Nil when not even that fits. - static func fittedRadius(at point: CGPoint, in size: CGSize, band: Band) -> CGFloat? { - let clearance = min(point.y - band.top, band.bottom - point.y, point.x, size.width - point.x) + /// the band and on screen, pulse included, `margin` clear of their edges. Nil when not even + /// that fits. + static func fittedRadius(at point: CGPoint, in size: CGSize, band: Band, margin: CGFloat = 0) -> CGFloat? { + let clearance = min(point.y - band.top, band.bottom - point.y, point.x, size.width - point.x) - margin let radius = clearance / 1.08 return radius >= 30 ? radius : nil } @@ -230,6 +310,13 @@ struct WayfindingOverlay: View { } } +/// What stands for an aim step's target on screen (`WayfindingOverlay.onAimMarker`). +enum AimMarker: Equatable { + case covered + case ring + case arrow(AimDirection) +} + /// The aim ring, or an arrow at the screen edge pointing toward it. Also the arrow toward the /// battery spot on "See it on your wall" (`ResultARScreen`). struct TargetMarker: View { @@ -244,6 +331,8 @@ struct TargetMarker: View { /// at 1, like the meter photo's ring; nil draws the plain pulsing ring. A ring that never /// changed while it was held on target gave no sign of what it wanted (#81). var progress: Double? = nil + /// The hysteresis-adjusted direction used by the aim card, when this arrow stands for an aim. + var aimDirection: AimDirection? = nil @Environment(\.accessibilityReduceMotion) private var reduceMotion var body: some View { @@ -269,7 +358,7 @@ struct TargetMarker: View { .background(Palette.signal, in: .circle) .overlay(Circle().strokeBorder(.white.opacity(0.85), lineWidth: 2)) .shadow(color: .black.opacity(0.3), radius: 6, y: 2) - .modifier(ArrowAccessibility(progress: progress, angle: angle)) + .modifier(ArrowAccessibility(progress: progress, angle: angle, direction: aimDirection)) .position(point) .transition(.opacity) case .hidden: @@ -338,6 +427,7 @@ struct TargetMarker: View { private struct ArrowAccessibility: ViewModifier { var progress: Double? var angle: Angle + var direction: AimDirection? func body(content: Content) -> some View { if let progress { @@ -352,8 +442,18 @@ private struct ArrowAccessibility: ViewModifier { } } - /// The arrow's direction in words, by its larger component. Screen y grows downward. + /// The arrow's direction in words. Aim arrows use the same hysteresis-adjusted direction as + /// the card; other arrows keep their geometric direction and remain hidden without progress. private var way: String { + if let direction { + switch direction { + case .above: return "above" + case .below: return "below" + case .left: return "to the left" + case .right: return "to the right" + case .onScreen, .behind: break + } + } let dx = cos(angle.radians), dy = sin(angle.radians) if abs(dx) >= abs(dy) { return dx > 0 ? "to the right" : "to the left" } return dy > 0 ? "below" : "above" diff --git a/ios/HouseScan/UI/Components/CameraChrome.swift b/ios/HouseScan/UI/Components/CameraChrome.swift index 2e5919009..d8b6b0e4b 100644 --- a/ios/HouseScan/UI/Components/CameraChrome.swift +++ b/ios/HouseScan/UI/Components/CameraChrome.swift @@ -1,3 +1,4 @@ +import HouseScanKit import SwiftUI /// The frame shared by every camera screen: status row and one instruction at the top, the @@ -26,13 +27,23 @@ struct CameraChrome: View { /// the card folds its detail and the legend under "Details" (`InstructionCard.foldsDetail`) /// and leaves the camera open. Questions and refusals keep every word in view. var aims = false + /// Whether the card's note takes its room out of the least open camera instead of pushing + /// the actions down. For a screen whose note shows only when the camera has nothing to draw + /// ("See it on your wall"): at AX5 its off-screen note is about 290 pt, and pushed below it + /// Done left the screen (CI run of #241 at d5d137ed). + var noteTakesCameraRoom = false @ViewBuilder var bottom: Bottom @Environment(\.accessibilityReduceMotion) private var reduceMotion @Environment(\.dynamicTypeSize) private var typeSize + /// The card's note's height, kept only for `noteTakesCameraRoom`. + @State private var noteRoom: CGFloat = 0 private var folds: Bool { aims && typeSize.isAccessibilitySize } + /// The least open camera without a note (`CameraChromeLayout.minCameraWindow` when folded). + private var leastCameraWindow: CGFloat { folds ? CameraChromeLayout.minCameraWindow : 0 } + var body: some View { // One layout at every text size: the chrome fills the screen and, only when the largest // text makes it taller than the screen, scrolls instead of squeezing the words. The @@ -51,9 +62,26 @@ struct CameraChrome: View { .accessibilityHidden(true) } } + .background { + GeometryReader { geometry in + Color.clear.preference(key: CameraChromeMeasurements.self, + value: .init(content: geometry.frame(in: .global))) + } + } } .scrollBounceBehavior(.basedOnSize) .scrollIndicators(.hidden) + .onPreferenceChange(CameraChromeMeasurements.self) { measurements in + // Publish one layout, never a new note height against the old camera frame. + // At AX5 that mixed snapshot toggled the note between 0 and 253.33 pt after + // scrolling Details, keeping the main thread in layout (PR #241 trace). + // This reader is the safe-area viewport. The ScrollView's global frame extends + // under the system bars and shifted the reconstructed camera in #241 at 602bd92. + guard let layout = measurements.layout(viewport: proxy.frame(in: .global), + minimumHeight: leastCameraWindow) else { return } + cameraWindow?.layout = layout + if noteTakesCameraRoom { noteRoom = layout.noteRoom } + } } } @@ -69,7 +97,9 @@ struct CameraChrome: View { .frame(minHeight: 36) InstructionCard( instruction: instruction, tone: tone, reply: reply, eyebrow: eyebrow, - foldsDetail: folds, legend: folds ? legend : nil + foldsDetail: folds, legend: folds ? legend : nil, + onDetailsShown: { shown in cameraWindow?.detailsShown = shown }, + measuresNoteRoom: cameraWindow != nil ) if let legend, !folds { legendLine(legend) @@ -77,9 +107,12 @@ struct CameraChrome: View { // The open camera between the card and the actions; the aim ring shows only inside // it (B-36). On an aiming step at the accessibility sizes it keeps at least // `minCameraWindow`, and the actions below scroll into reach. - Spacer(minLength: folds ? CameraChromeLayout.minCameraWindow : 0) - .onGeometryChange(for: CGRect.self, of: { $0.frame(in: .global) }) { window in - if let cameraWindow, cameraWindow.frame != window { cameraWindow.frame = window } + Spacer(minLength: max(leastCameraWindow - (noteTakesCameraRoom ? noteRoom : 0), 0)) + .background { + GeometryReader { geometry in + Color.clear.preference(key: CameraChromeMeasurements.self, + value: .init(window: geometry.frame(in: .global))) + } } bottom } @@ -109,12 +142,42 @@ struct CameraChrome: View { } /// The open camera between the instruction card and the actions, in global coordinates. -/// `CameraChrome` writes it and only `WayfindingOverlay` reads it, so scrolling the chrome -/// redraws the aim overlay, not the whole screen. +/// `CameraChrome` measures it and writes its disclosure state. The overlay uses the frame; the +/// walk uses both to explain when open Details has pushed the camera below the screen. @MainActor @Observable final class CameraWindow { - var frame: CGRect? + private(set) var frame: CGRect? + /// `frame` as it would be if the card had no note (`OpenCameraWithoutNote`). A note shown + /// because of what the camera holds is judged against this, so its own height can't keep it + /// up. It restores the minimum camera height and settled scroll even just after note removal. + private(set) var noteFreeFrame: CGRect? + var detailsShown = false + + /// What `CameraChrome` measures. Not observed itself: it changes on every scroll, and + /// readers follow `frame` and `noteFreeFrame`. + struct Layout: Equatable { + var window: CGRect? + var content: CGRect? + var viewport: CGRect? + /// How much taller the card's note makes the card (`InstructionCard.measuresNoteRoom`). + var noteRoom: CGFloat = 0 + /// The least open camera the chrome keeps without a note. + var minimumHeight: CGFloat = 0 + } + + @ObservationIgnored var layout = Layout() { + didSet { + guard layout != oldValue else { return } + if frame != layout.window { frame = layout.window } + let noteFree = layout.window.map { window in + guard let content = layout.content, let viewport = layout.viewport else { return window } + return OpenCameraWithoutNote.frame(window: window, content: content, viewport: viewport, + noteRoom: layout.noteRoom, minimumHeight: layout.minimumHeight) + } + if noteFreeFrame != noteFree { noteFreeFrame = noteFree } + } + } } /// `CameraChrome`'s layout constants, outside it because a generic type can't hold stored diff --git a/ios/HouseScan/UI/Components/CameraChromeMeasurements.swift b/ios/HouseScan/UI/Components/CameraChromeMeasurements.swift new file mode 100644 index 000000000..f5d5a9609 --- /dev/null +++ b/ios/HouseScan/UI/Components/CameraChromeMeasurements.swift @@ -0,0 +1,28 @@ +import SwiftUI + +/// Measurements from one layout pass. Separate geometry callbacks can combine a new note's +/// height with the previous camera frame, making a note decided from that frame oscillate. +struct CameraChromeMeasurements: PreferenceKey { + struct Value: Equatable { + var window: CGRect? + var content: CGRect? + var messageHeight: CGFloat? + var noteFreeHeight: CGFloat? + + func layout(viewport: CGRect, minimumHeight: CGFloat) -> CameraWindow.Layout? { + guard let window, let content, let messageHeight, let noteFreeHeight else { return nil } + return CameraWindow.Layout(window: window, content: content, viewport: viewport, + noteRoom: max(messageHeight - noteFreeHeight, 0), minimumHeight: minimumHeight) + } + } + + static var defaultValue: Value { Value() } + + static func reduce(value: inout Value, nextValue: () -> Value) { + let next = nextValue() + value.window = next.window ?? value.window + value.content = next.content ?? value.content + value.messageHeight = next.messageHeight ?? value.messageHeight + value.noteFreeHeight = next.noteFreeHeight ?? value.noteFreeHeight + } +} diff --git a/ios/HouseScan/UI/Components/InstructionCard.swift b/ios/HouseScan/UI/Components/InstructionCard.swift index 8ba32e09a..1c97982a8 100644 --- a/ios/HouseScan/UI/Components/InstructionCard.swift +++ b/ios/HouseScan/UI/Components/InstructionCard.swift @@ -43,6 +43,10 @@ struct InstructionCard: View { /// The aim ring's legend, inside the folded details (`foldsDetail`). Otherwise the legend /// is the chrome's own line under the card (`CameraChrome.legend`). var legend: String? = nil + var onDetailsShown: ((Bool) -> Void)? = nil + /// Reports both message heights with the chrome's other measurements, so it can judge + /// the open camera without the note (`CameraWindow.noteFreeFrame`). + var measuresNoteRoom = false /// How long a new card's reply ignores taps, so a tap meant for the card that just left /// can't answer the next one: in the 4.1 field test, quick taps on "Can't get there" each @@ -54,8 +58,8 @@ struct InstructionCard: View { /// The task whose reply has been on screen for `replyLock`. Compared with the current task /// in the same update that changes it, so a new reply is locked from its first frame. @State private var unlockedTask: Instruction? - /// Whether the folded details are open. Closed again for each new step (a new title), so the - /// camera opens up again; the distance to go and coaching change within a step and keep it. + /// Whether the folded details are open. Each new aim target closes them; marker titles + /// within that target and ride-along coaching keep them. @State private var detailsShown = false private var replyTask: Instruction { reply?.task ?? instruction } @@ -63,7 +67,9 @@ struct InstructionCard: View { private var hasDetails: Bool { foldsDetail && (foldedDetail != nil || legend != nil) } /// The words the card shows: folded, a step that leads with its action does (`Instruction.folded`). - private var title: String { foldsDetail ? (instruction.folded?.title ?? instruction.title) : instruction.title } + private func title(of instruction: Instruction) -> String { + foldsDetail ? (instruction.folded?.title ?? instruction.title) : instruction.title + } private var foldedDetail: String? { instruction.folded?.detail ?? instruction.detail } var body: some View { @@ -71,6 +77,25 @@ struct InstructionCard: View { message .id(instruction) .transition(.identity) + .background { + if measuresNoteRoom { + GeometryReader { geometry in + Color.clear.preference(key: CameraChromeMeasurements.self, + value: .init(messageHeight: geometry.size.height)) + } + } + } + .background(alignment: .top) { + if measuresNoteRoom { + noteFreeMessage + .background { + GeometryReader { geometry in + Color.clear.preference(key: CameraChromeMeasurements.self, + value: .init(noteFreeHeight: geometry.size.height)) + } + } + } + } if hasDetails { details } @@ -113,7 +138,13 @@ struct InstructionCard: View { // Only the scrim's height eases. Reduce Motion asks for no size changes, so it snaps. .animation(reduceMotion ? nil : Motion.text, value: instruction) .animation(reduceMotion ? nil : Motion.text, value: reply == nil) - .onChange(of: instruction.title) { detailsShown = false } + .onChange(of: detailsShown && hasDetails, initial: true) { _, shown in onDetailsShown?(shown) } + // Every aim uses the same reserved titles, so those words alone cannot identify a task. + .onChange(of: instruction.aimTarget) { _, _ in detailsShown = false } + .onChange(of: instruction.title) { old, new in + let reserved = instruction.titleReserve + if !(reserved.contains(old) && reserved.contains(new)) { detailsShown = false } + } .task(id: replyTask) { // Cleared first, so a task that comes back within the lock (A, B, A) is locked again. let shown = replyTask @@ -125,6 +156,23 @@ struct InstructionCard: View { } private var message: some View { + messageContent(instruction) + .accessibilityElement(children: .combine) + .accessibilityAddTraits(.updatesFrequently) + .accessibilityIdentifier("instruction") + } + + /// The message without `Instruction.note`, laid out unseen behind the card for `measuresNoteRoom`. + private var noteFreeMessage: some View { + var noteFree = instruction + noteFree.note = nil + return messageContent(noteFree) + .fixedSize(horizontal: false, vertical: true) + .hidden() + .accessibilityHidden(true) + } + + private func messageContent(_ instruction: Instruction) -> some View { HStack(alignment: .firstTextBaseline, spacing: 12) { if let symbol = iconName { Image(systemName: symbol) @@ -141,10 +189,20 @@ struct InstructionCard: View { .fixedSize(horizontal: false, vertical: true) .padding(.bottom, 2) } - Text(title) - .font(Typeface.instruction) - .foregroundStyle(Palette.chalk) - .fixedSize(horizontal: false, vertical: true) + ZStack(alignment: .topLeading) { + // As tall as the tallest title this card may swap to (`Instruction.titleReserve`). + ForEach(instruction.titleReserve, id: \.self) { reserved in + Text(reserved) + .font(Typeface.instruction) + .fixedSize(horizontal: false, vertical: true) + .hidden() + .accessibilityHidden(true) + } + Text(title(of: instruction)) + .font(Typeface.instruction) + .foregroundStyle(Palette.chalk) + .fixedSize(horizontal: false, vertical: true) + } // Folded, only the coaching stays under the title; the detail is in `details`. if let line = foldsDetail ? instruction.note : instruction.detailAndNote { Text(line) @@ -157,9 +215,6 @@ struct InstructionCard: View { } .padding(.horizontal, 18) .padding(.vertical, 16) - .accessibilityElement(children: .combine) - .accessibilityAddTraits(.updatesFrequently) - .accessibilityIdentifier("instruction") } /// The detail and the legend under "Details", the word and control the result screen diff --git a/ios/HouseScan/UI/Copy/ScanCopy.swift b/ios/HouseScan/UI/Copy/ScanCopy.swift index 1e54607be..934d1587c 100644 --- a/ios/HouseScan/UI/Copy/ScanCopy.swift +++ b/ios/HouseScan/UI/Copy/ScanCopy.swift @@ -14,6 +14,20 @@ struct Instruction: Hashable { /// situation and the detail holds what to do: the doing leads, and the situation moves under /// Details. Nil keeps the title and folds the detail. var folded: Folded? = nil + /// Other titles this card may swap to while it stays up, laid out unseen behind the title so + /// the card keeps one height whichever shows (`InstructionCard`). On an aim step the title + /// follows the marker the overlay draws, and the overlay draws within the camera the card + /// leaves open: a title that wrapped to a different height moved that camera, which could + /// flip the marker and the title back (review of #241). + var titleReserve: [String] = [] + /// A planner aim keeps its identity while its marker, coaching and rounded distance change. + /// Exact coordinates distinguish adjacent targets whose displayed distances can be equal. + var aimTarget: AimTarget? = nil + + enum AimTarget: Hashable { + case ground(s: Float) + case wall(s: Float) + } struct Folded: Hashable { /// What to do now, in place of `title`. @@ -76,15 +90,18 @@ enum ScanCopy { ) case .aimAtGround(let s): // A cell counts once seen from two places at least 0.25 m apart (`coveringBaseline`), - // so tilting down without moving never clears it. + // so tilting down without moving never clears it. A whole step, not a small one: a + // frame is kept for its place only 0.5 m from the last (`AutoCaptureConfig + // .spacingMeters`), and a shuffle can stop short of both (review of #117). The legend + // and the second-position title say the same step (`aimRingLegend`, `aim`). aim( - Instruction(title: "Tilt down to show the ground", detail: "The strip along the wall, \(Distance.fromMeter(s)). Take a small step sideways as you look."), + Instruction(title: "Tilt down to show the ground", detail: "The strip along the wall, \(Distance.fromMeter(s)). Take a step to the side as you look.", aimTarget: .ground(s: s)), ground: true, hint: hint ) case .aimAtWall(let s): // "Around at your meter" read wrong once the walk starts at the meter. aim( - Instruction(title: "Tilt up to show more wall", detail: abs(s) < Distance.metersPerInch * 3 ? "At your meter." : "Around \(Distance.fromMeter(s))."), + Instruction(title: "Tilt up to show more wall", detail: abs(s) < Distance.metersPerInch * 3 ? "At your meter." : "Around \(Distance.fromMeter(s)).", aimTarget: .wall(s: s)), ground: false, hint: hint ) case .stepBack: @@ -122,18 +139,22 @@ enum ScanCopy { /// the side is the whole instruction (#77). Too close, "step back" joins the same card /// instead of replacing it with another (#77). private static func aim(_ base: Instruction, ground: Bool, hint: GuidanceHint?) -> Instruction { - guard let hint else { return base } var copy = base + // The same target can gain or lose its second-position advice without moving the card. + copy.titleReserve = ground + ? [base.title, aimTiltUp, aimRingOnStrip, aimSecondPosition] + : [base.title, aimTiltDown, aimSecondPosition] + guard let hint else { return copy } if hint.needsSecondPosition { - copy.title = "Now take one step to the side and look again" + copy.title = aimSecondPosition } else { switch hint.aim { case .above? where ground: - copy.title = "Tilt up a little" + copy.title = aimTiltUp case .below? where !ground: - copy.title = "Tilt down a little" + copy.title = aimTiltDown case .onScreen? where ground: - copy.title = "Put the ring on the strip" + copy.title = aimRingOnStrip default: break } @@ -144,6 +165,11 @@ enum ScanCopy { return copy } + private static let aimSecondPosition = "Step sideways, look again" + private static let aimTiltUp = "Tilt up a little" + private static let aimTiltDown = "Tilt down a little" + private static let aimRingOnStrip = "Put the ring on the strip" + // MARK: Coaching /// `meterPhoto` says whether the screen shows the meter close-up beside the card @@ -229,10 +255,10 @@ enum ScanCopy { /// its stretch is captured (#81). /// A stretch counts only once it is seen from two places a step apart /// (`CoverageConfig.coveringBaseline`), so holding still never fills the ring: the line asks - /// for a step, in the tilt-down card's own words ("Take a small step sideways as you look."), + /// for a step, in the tilt-down card's own words ("Take a step to the side as you look."), /// so the two lines on screen together ask for the same thing. If the card's wording /// changes, change this with it. - static let aimRingLegend = "Keep the ring in view and take a small step sideways. It fills as your phone captures this spot." + static let aimRingLegend = "Keep the ring in view and take a step to the side. It fills as your phone captures this spot." // MARK: Wall ends @@ -341,6 +367,16 @@ enum ScanCopy { /// Over "Looks complete" after its first tap found a question unanswered (#65). static let reviewUnanswered = "A question above has no answer yet. Answer it, or tap Looks complete again to send." + /// In place of the review's "Add something" chips while the phone has lost its place. It says + /// "finish with", not "tap to send": with a question unanswered, that tap first points to it + /// (review of #131). + static let reviewLostPlace = "Your phone lost its place. Point it back at the wall to add more, or finish with Looks complete." + + /// Over the result's list of marks the server set aside (`ResultPresentation.marksNotUsed`). + static let marksNotUsedTitle = "Not counted" + /// Why the marks were set aside; the available follow-up is determined by the server answer. + static let marksNotUsedHint = "These marks are outside the part of the wall used for this answer." + /// Under a reviewed mark that lies wholly past a marked end (`ScanViewState.featuresPastEnds`). static let featurePastEnd = "Past the end of your scan" @@ -785,7 +821,24 @@ enum ScanCopy { /// The AR title over a possible spot, and the words for what the AR view draws there. static let proposedSpotOverlay = "A proposed battery spot, drawn on your wall" static let proposedSpotThisWay = "The proposed spot is this way" - static let proposedSpotOffScreen = "The proposed spot is off screen. Turn the phone toward the arrow." + /// The note shown outside Details when the open camera cannot hold the chevron, so there + /// is no arrow to turn toward (review of #241), and the chevron's VoiceOver label when it is + /// drawn. Not while open Details covers the camera: `cameraBelowDetails` says how to get it + /// back instead. + static func proposedSpotOffScreen(turn direction: AimDirection) -> String { + let turn = switch direction { + case .above: "Tilt the phone up" + case .below: "Tilt the phone down" + case .left: "Turn the phone left" + case .right: "Turn the phone right" + case .onScreen, .behind: "Turn the phone around" + } + return "The proposed spot is off screen. \(turn) to find it." + } + + /// When open Details at the largest text sizes leaves almost none of the camera on screen, + /// on the walk and in "See it on your wall". + static let cameraBelowDetails = "The camera view is below. Close Details or scroll." static let seeOnWall = "See this spot on your wall" /// For a spot an installer still has to confirm against the meter's working space. diff --git a/ios/HouseScan/UI/Preview/DemoEngine.swift b/ios/HouseScan/UI/Preview/DemoEngine.swift index f60b86629..de07b2a56 100644 --- a/ios/HouseScan/UI/Preview/DemoEngine.swift +++ b/ios/HouseScan/UI/Preview/DemoEngine.swift @@ -1,5 +1,6 @@ import HouseScanKit import SwiftUI +import simd /// A fake engine for the UI demo: implements `ScanActions` by scripting `ScanViewState` over /// the made-up wall in `DemoScene`. It mimics the real engine's phases and timing closely enough @@ -198,6 +199,25 @@ final class DemoEngine: ScanActions { } if (arguments.contains("-uiDemoAim") || arguments.contains("-uiDemoAimOffScreen")), state.phase == .wallWalk { enterAim(offScreen: arguments.contains("-uiDemoAimOffScreen")) + // The engine's direction to the target, which can disagree with what the overlay + // draws near the card or an edge (review of #120): the card must follow the drawing. + if let hint = value("-uiDemoAimHint").flatMap(Self.aimDirection) { state.guidanceHint = GuidanceHint(aim: hint) } + // Leave the aim step and its engine hint in place while removing the target, so UI + // tests can verify that a missing marker does not leave a stale direction on the card. + if arguments.contains("-uiDemoAimNoMarker") { state.target = nil } + // A UI test changes the camera direction or advances the target after reading the + // card. A timer from launch raced that baseline on a slow simulator (#241). + if let later = value("-uiDemoAimHintLater").flatMap(Self.aimDirection) { + laterAimHint = later + } + laterAimTarget = value("-uiDemoAimTargetLater").flatMap(Float.init) + if laterAimHint != nil || laterAimTarget != nil { + Self.listeningForAimHint = self + CFNotificationCenterAddObserver( + CFNotificationCenterGetDarwinNotifyCenter(), nil, + { _, _, _, _, _ in Task { @MainActor in DemoEngine.listeningForAimHint?.applyLaterAimHint() } }, + Self.aimHintNotification as CFString, nil, .deliverImmediately) + } } if arguments.contains("-uiDemoCorner") { // The walk followed an outside corner right of the meter, between the battery spot and @@ -236,6 +256,19 @@ final class DemoEngine: ScanActions { if arguments.contains("-uiDemoFollowUp") { enterFollowUp(at: state.phase) } + // A mark past a marked end: flagged on the review, and on the result set aside by the + // server (`objects_not_used`), as for run 2's window (#42, review of #53). + if arguments.contains("-uiDemoMarkPastEnd") { + let span: ClosedRange = -4.1 ... -3.3 + let window = MarkedFeature( + id: UUID(), kind: .window, span: span, bottom: DemoScene.windowHeights.lowerBound, + top: DemoScene.windowHeights.upperBound, out: nil, + points: [DemoScene.wall.world(s: span.lowerBound, height: DemoScene.windowHeights.lowerBound), + DemoScene.wall.world(s: span.upperBound, height: DemoScene.windowHeights.upperBound)], opens: false) + state.features.append(window) + state.featuresPastEnds = [window.id] + state.result?.marksNotUsed = [Self.markNotUsedSample] + } if arguments.contains("-uiDemoNoFeed") { state.feed = .none } @@ -248,6 +281,13 @@ final class DemoEngine: ScanActions { state.meterBrand = Self.demoBrand state.meterNumber = .choose(Self.demoCandidates) } + if arguments.contains("-uiDemoSpotLow"), state.phase == .resultAR { + state.projection = DemoScene.lowSpotProjection + } + if arguments.contains("-uiDemoSpotOffScreen"), state.phase == .resultAR, var projection = state.projection { + projection.cameraToWorld.columns.3.x -= 10 + state.projection = projection + } // Frozen, the upload script never runs, so show where it would end. if freeze, state.phase == .uploading { if rejectUpload || offline || unusableAnswers > 0 { @@ -338,6 +378,7 @@ final class DemoEngine: ScanActions { state.guidance = .holdOnMeter state.closeUp = .aiming(hold: 0, problem: nil) state.meterNumber = nil + state.meterBrand = nil run { engine in await engine.closeUpScript() } } @@ -377,6 +418,51 @@ final class DemoEngine: ScanActions { run { engine in await engine.walkScript() } } + /// The Darwin notification a UI test posts for `-uiDemoAimHintLater`. ScreenStatesUITests + /// sends the same name. + static let aimHintNotification = "dev.housescanning.uiDemo.aimHintLater" + /// The demo waiting for `aimHintNotification`; the notification's C callback can't capture. + private static weak var listeningForAimHint: DemoEngine? + /// The direction `-uiDemoAimHintLater` makes the same target's drawn marker take. + private var laterAimHint: AimDirection? + private var laterAimTarget: Float? + + private func applyLaterAimHint() { + if let s = laterAimTarget { + state.guidance = .aimAtGround(s: s) + state.target = DemoScene.wall.world(s: s, height: 0, out: state.coverage.groundBandDepth / 2) + state.path = DemoScene.path(toward: s) + } + if let laterAimHint, let target = state.target, var projection = state.projection { + // Move the camera while retaining the planner's target. The card now follows the + // drawn marker, so changing only the engine's hint would not exercise a title swap. + let local: SIMD4 = switch laterAimHint { + case .left: SIMD4(0, -5, -3, 0) + case .right: SIMD4(0, 5, -3, 0) + // Above the tall card's open lane, but still within the full camera image. + case .above: SIMD4(-1, 0, -3, 0) + case .below: SIMD4(5, 0, -3, 0) + case .onScreen: SIMD4(0, 0, -3, 0) + case .behind: SIMD4(0, 1, 3, 0) + } + let offset = projection.cameraToWorld * local + projection.cameraToWorld.columns.3 = SIMD4(target, 1) - offset + state.projection = projection + // Retain the engine hint so the test can expose disagreement with the marker. + } + } + + static func aimDirection(_ raw: String) -> AimDirection? { + switch raw { + case "onScreen": .onScreen + case "above": .above + case "below": .below + case "left": .left + case "right": .right + default: nil + } + } + /// `-uiDemoAim`: the ground lags the wall right of the meter, so the walk asks to tilt down /// there, and the ring is half full: 2 of the 5 ground cells within 0.3 m of it are covered, /// and 80 % completes it. `-uiDemoAimOffScreen`: the same ask for ground left of the view, so @@ -876,6 +962,7 @@ final class DemoEngine: ScanActions { func skipCloseUp() { state.closeUp = .skipped state.meterNumber = .skipped + state.meterBrand = nil enterWalk() } @@ -889,6 +976,7 @@ final class DemoEngine: ScanActions { guard let candidate else { // "None of these": another photo, with the advice the reader gives for small text. state.meterNumber = nil + state.meterBrand = nil state.closeUpFailedAttempts += 1 state.closeUp = .aiming(hold: 0, problem: .numberTooSmall) run { engine in await engine.closeUpScript() } @@ -1195,6 +1283,7 @@ final class DemoEngine: ScanActions { state.lastCapture = nil state.closeUpFailedAttempts = 0 state.meterNumber = nil + state.meterBrand = nil state.upload = .idle state.shareableScan = nil state.marking = nil @@ -1300,6 +1389,9 @@ final class DemoEngine: ScanActions { } } + /// The server's sentence for a mark it set aside (solver.py `objects_not_used`). + static let markNotUsedSample = "The window marked 12 ft left of the meter is past the left end of the scan, where the wall may turn, so it was not used." + /// The offline sample result. Placeholder rules, so the decision is manual review. static let reviewSample = ResultPresentation( decision: .manualReview, diff --git a/ios/HouseScan/UI/Preview/DemoScene.swift b/ios/HouseScan/UI/Preview/DemoScene.swift index fcea399cc..e811e0f27 100644 --- a/ios/HouseScan/UI/Preview/DemoScene.swift +++ b/ios/HouseScan/UI/Preview/DemoScene.swift @@ -24,6 +24,11 @@ enum DemoScene { /// A phone held upright 5 m from the wall, a little right of the meter, tilted 18° down: /// the walk. static let projection = camera(at: SIMD3(0.6, 1.5, 5.0), pitchDegrees: 18, focal: 1100) + /// The walk's phone tilted 10° up instead of 18° down, for "See it on your wall" with the + /// sample spot low in the open camera (`-uiDemoSpotLow`). Worked out from this projection, + /// not measured: the spot's middle lands at about y = 634 pt on a 402 × 874 pt iPhone 17 + /// screen, where the default pose puts it at about 380 pt, under the folded AX5 card. + static let lowSpotProjection = camera(at: SIMD3(0.6, 1.5, 5.0), pitchDegrees: -10, focal: 1100) /// Standing 2.4 m back and aiming at the meter: finding it. static let meterProjection = camera(at: SIMD3(0.05, 1.5, 2.4), pitchDegrees: 0, focal: 1400) /// Close to the meter for its photo. diff --git a/ios/HouseScan/UI/Preview/HysteresisDemo.swift b/ios/HouseScan/UI/Preview/HysteresisDemo.swift new file mode 100644 index 000000000..21ca54958 --- /dev/null +++ b/ios/HouseScan/UI/Preview/HysteresisDemo.swift @@ -0,0 +1,133 @@ +import simd +import SwiftUI + +/// Controlled poses for ScreenStatesUITests, using the real marker and result screen. Each +/// button advances one state so the test observes it before changing the target or tracking. +struct AimHysteresisDemo: View { + @State private var stage = 0 + @State private var marker: AimMarker? + + private var target: SIMD3 { + // Exact changes smaller than a displayed distance must still start a fresh aim. + SIMD3(stage >= 6 ? 0.002 : stage >= 2 ? 0.001 : 0, 0, 0) + } + + private var step: GuidanceStep { + stage >= 9 ? .aimAtWall(s: 0.4) : .aimAtGround(s: 0.4) + } + + var body: some View { + GeometryReader { proxy in + ZStack { + Palette.ink + WayfindingOverlay( + projection: HysteresisDemoProjection.camera(showing: target, at: point(in: proxy.size), in: proxy.size), + path: [], target: target, step: step, progress: 0.5, + onAimMarker: { marker = $0 } + ) + VStack { + Text(markerName) + .accessibilityIdentifier("demo.marker") + Spacer() + Button("Next pose") { stage += 1 } + .frame(minHeight: Metrics.minTarget) + .accessibilityIdentifier("demo.nextPose") + } + .padding(.vertical, 80) + } + } + .ignoresSafeArea() + } + + private func point(in size: CGSize) -> CGPoint { + switch stage { + case 0, 4, 7: + CGPoint(x: -100, y: size.height / 2) + case 1, 2, 8, 9: + // Vertical is 1.1 times horizontal: above without memory, left with left-axis bias. + CGPoint(x: -100, y: size.height / 2 - (size.width / 2 + 100) * 1.1) + case 3: + CGPoint(x: size.width / 2, y: size.height / 2) + case 5, 6: + // Inside the unmeasured ring bounds, but not the previous arrow's 12 pt margin. + CGPoint(x: 40, y: size.height / 2) + default: + preconditionFailure("Aim hysteresis fixture has no pose \(stage)") + } + } + + private var markerName: String { + switch marker { + case .ring?: "ring" + case .covered?: "covered" + case .arrow(let direction)?: String(describing: direction) + case nil: "none" + } + } +} + +struct SpotHysteresisDemo: View { + @State private var engine = DemoEngine(arguments: ["-uiDemoPhase", "resultAR", "-uiDemoFreeze"]) + @State private var stage = 0 + + var body: some View { + GeometryReader { proxy in + let size = CGSize(width: proxy.size.width, height: proxy.size.height + proxy.safeAreaInsets.top + proxy.safeAreaInsets.bottom) + ResultARScreen(state: engine.state, actions: engine) + .background(Palette.ink) + .overlay(alignment: .topTrailing) { + Button("Next pose") { + stage += 1 + apply(in: size) + } + .frame(minHeight: Metrics.minTarget) + .accessibilityIdentifier("demo.nextPose") + } + .onAppear { apply(in: size) } + } + } + + private func apply(in size: CGSize) { + guard let wall = engine.state.wall, let spot = engine.state.result?.spotCenter(on: wall) else { + preconditionFailure("Spot hysteresis fixture requires a result with a spot") + } + // Exercise both ways a layout becomes unavailable, then a valid in-view layout. + switch stage { + case 0, 7: + engine.state.tracking = .normal + engine.state.projection = camera(x: -100, spot: spot, size: size) + case 1, 8: + engine.state.tracking = .limited(.excessiveMotion) + case 2, 4, 6, 10: + engine.state.tracking = .normal + // Inside the clear window's x = 24 edge but outside its re-entry margin at x = 36. + engine.state.projection = camera(x: 30, spot: spot, size: size) + case 3: + engine.state.projection = nil + case 5, 9: + engine.state.tracking = .normal + engine.state.projection = camera(x: size.width / 2, spot: spot, size: size) + default: + preconditionFailure("Spot hysteresis fixture has no pose \(stage)") + } + } + + private func camera(x: CGFloat, spot: SIMD3, size: CGSize) -> CameraProjection { + HysteresisDemoProjection.camera(showing: spot, at: CGPoint(x: x, y: size.height * 0.65), in: size) + } +} + +@MainActor +private enum HysteresisDemoProjection { + /// Invert the portrait projection at a fixed 3 m depth, moving only the synthetic camera. + static func camera(showing target: SIMD3, at point: CGPoint, in size: CGSize) -> CameraProjection { + var projection = DemoScene.projection + let pixel = projection.imagePixel(forViewPoint: point, in: size) + let local = SIMD4( + (pixel.x - projection.intrinsics.z) * 3 / projection.intrinsics.x, + (projection.intrinsics.w - pixel.y) * 3 / projection.intrinsics.y, + -3, 0) + projection.cameraToWorld.columns.3 = SIMD4(target, 1) - projection.cameraToWorld * local + return projection + } +} diff --git a/ios/HouseScan/UI/Preview/UIDemo.swift b/ios/HouseScan/UI/Preview/UIDemo.swift index 0141045ea..82f403814 100644 --- a/ios/HouseScan/UI/Preview/UIDemo.swift +++ b/ios/HouseScan/UI/Preview/UIDemo.swift @@ -46,9 +46,19 @@ import SwiftUI /// the meter, and the ring is half full with its legend beside it. Unfrozen, it fills, the walk /// goes on in the same update as on a phone, and the ring holds green with a tick for a moment. /// - `-uiDemoAimOffScreen`: as `-uiDemoAim`, for ground left of the view: the edge arrow shows. +/// - `-uiDemoAimNoMarker`: keep the aim step's hint but remove its target, so no marker is drawn. /// - `-uiDemoCorner`: the wall turns an outside corner 1.8 m right of the meter and the walk /// followed it, so the window and part of its clearance zone are round the corner. For the /// result model: `-uiDemoPhase result -uiDemoCorner`. +/// - `-uiDemoAimCardTitles`: an isolated folded aim card, cycling ring, arrow and covered +/// titles for the same target. Exercises a nonempty reserve alongside a folded title. +/// `-uiDemoAimSecondPositionToggle` also lets the test toggle second-position advice. +/// - `-uiDemoAimHysteresis`: button-driven target and step changes in the real aim overlay. +/// - `-uiDemoSpotHysteresis`: button-driven tracking loss and recovery in the real result screen. +/// - `-uiDemoSpotOffScreen`: with `-uiDemoPhase resultAR`, place the camera 10 m left so the +/// proposed spot needs an off-screen direction, including at accessibility text sizes. +/// - `-uiDemoSpotLow`: with `-uiDemoPhase resultAR`, tilt the camera up so the proposed spot sits +/// low in the open camera, below the folded card at AX5, where open Details covers it. /// - `-uiDemoPhase spotConfirm`: the spot check before the result, on the made-up sample spot. /// - `-uiDemoSpotAnswered `: with `-uiDemoPhase spotConfirm`, /// the check is answered and says what happens next. With `-uiDemoPhase result`, the result @@ -66,8 +76,17 @@ import SwiftUI /// when the answer lists a view the camera can take. enum UIDemo { @MainActor + @ViewBuilder static func makeRoot() -> some View { - DemoHost() + if ProcessInfo.processInfo.arguments.contains("-uiDemoAimCardTitles") { + FoldedAimCardDemo() + } else if ProcessInfo.processInfo.arguments.contains("-uiDemoAimHysteresis") { + AimHysteresisDemo() + } else if ProcessInfo.processInfo.arguments.contains("-uiDemoSpotHysteresis") { + SpotHysteresisDemo() + } else { + DemoHost() + } } } @@ -78,3 +97,58 @@ private struct DemoHost: View { ScanRootView(state: engine.state, actions: engine) } } + +/// The card contract permits a folded lead and reserved titles together, even though today's +/// live ground/wall aims have no separate folded lead. Keep that combination stable at AX5. +private struct FoldedAimCardDemo: View { + private enum Variant { + case ring, arrow, covered + + var hint: AimDirection? { + switch self { + case .ring: .onScreen + case .arrow: .above + case .covered: nil + } + } + } + + @State private var variant = Variant.ring + @State private var needsSecondPosition = false + private let showsPositionToggle = ProcessInfo.processInfo.arguments.contains("-uiDemoAimSecondPositionToggle") + + private var instruction: Instruction { + var copy = ScanCopy.guidance(.aimAtGround(s: 0.4), hint: GuidanceHint(aim: variant.hint, needsSecondPosition: needsSecondPosition)) + copy.folded = Instruction.Folded(title: copy.title, detail: copy.detail ?? "") + return copy + } + + var body: some View { + VStack(spacing: 24) { + InstructionCard(instruction: instruction, foldsDetail: true) + .transaction { transaction in + transaction.animation = nil + transaction.disablesAnimations = true + } + .accessibilityElement(children: .contain) + .accessibilityIdentifier("demo.aimCard") + Button("Next title") { + switch variant { + case .ring: variant = .arrow + case .arrow: variant = .covered + case .covered: variant = .ring + } + } + .frame(minHeight: Metrics.minTarget) + .accessibilityIdentifier("demo.nextAimTitle") + if showsPositionToggle { + Button("Change position advice") { needsSecondPosition.toggle() } + .frame(minHeight: Metrics.minTarget) + .accessibilityIdentifier("demo.toggleAimPosition") + } + Spacer() + } + .padding(Metrics.edge) + .background(Palette.ink) + } +} diff --git a/ios/HouseScan/UI/Screens/MarkFeaturesScreen.swift b/ios/HouseScan/UI/Screens/MarkFeaturesScreen.swift index ed79f831f..c7a9dd6a7 100644 --- a/ios/HouseScan/UI/Screens/MarkFeaturesScreen.swift +++ b/ios/HouseScan/UI/Screens/MarkFeaturesScreen.swift @@ -6,7 +6,9 @@ import SwiftUI /// /// "Looks complete" is a soft gate (#65): with a question still unanswered, the first tap /// scrolls to the first one and outlines it, and the next tap sends anyway. The ground question -/// and every window's question count; "Not sure" is an answer to both. +/// and every window's question count; "Not sure" is an answer to both. Once every question has +/// an answer the gate re-arms, so a window added afterwards gets its own first-tap warning +/// (review of #131). /// /// A panel over the dimmed camera rather than a new page: the homeowner is still standing at /// the wall, and the list refers to things they can see. @@ -20,8 +22,10 @@ struct MarkFeaturesScreen: View { let actions: any ScanActions @Environment(\.accessibilityReduceMotion) private var reduceMotion - /// The unanswered question the first "Looks complete" pointed to; set, the next tap sends. - @State private var nudged: ReviewQuestion? = nil + /// The first "Looks complete" found a question unanswered and pointed to it; while set, the + /// outline follows whichever question is now first without an answer, and the next tap + /// sends. Cleared once every question has an answer. + @State private var warned = false var body: some View { ZStack { @@ -34,6 +38,11 @@ struct MarkFeaturesScreen: View { } } .animation(reduceMotion ? .easeOut(duration: 0.15) : Motion.screen, value: state.marking == nil) + // Re-arm here rather than in the review panel: the panel is swapped out while a mark + // from "Add something" is open, and that mark can be the window that needs the warning. + .onChange(of: firstUnanswered == nil) { _, allAnswered in + if allAnswered { warned = false } + } } private var review: some View { @@ -59,11 +68,10 @@ struct MarkFeaturesScreen: View { return state.features.first(where: \.awaitsOpensAnswer).map { .window($0.id) } } - private func isUnanswered(_ question: ReviewQuestion) -> Bool { - switch question { - case .ground: state.groundAnswer == nil - case .window(let id): state.features.first { $0.id == id }?.awaitsOpensAnswer ?? false - } + /// Outlined: the question "Looks complete" is waiting on. It moves to the next one when + /// this one is answered, so a second unanswered window is not left for the homeowner to find. + private func isOutlined(_ question: ReviewQuestion) -> Bool { + warned && firstUnanswered == question } private var panel: some View { @@ -73,7 +81,7 @@ struct MarkFeaturesScreen: View { VStack(alignment: .leading, spacing: 20) { // First, so it is seen before "Looks complete"; skipping it counts as not sure. GroundQuestion(answer: state.groundAnswer, actions: actions, - highlighted: nudged == .ground && isUnanswered(.ground)) + highlighted: isOutlined(.ground)) .id(ReviewQuestion.ground) heading featureList @@ -83,7 +91,7 @@ struct MarkFeaturesScreen: View { } .scrollBounceBehavior(.basedOnSize) - if nudged != nil, firstUnanswered != nil { + if warned, firstUnanswered != nil { Text(ScanCopy.reviewUnanswered) .font(.subheadline) .foregroundStyle(.primary) @@ -106,7 +114,7 @@ struct MarkFeaturesScreen: View { .padding(.top, 8) .padding(.bottom, 12) } - .animation(Motion.text, value: nudged) + .animation(Motion.text, value: warned) } .background { UnevenRoundedRectangle(topLeadingRadius: 28, topTrailingRadius: 28, style: .continuous) @@ -116,15 +124,15 @@ struct MarkFeaturesScreen: View { .environment(\.colorScheme, .light) } - /// Sends, unless a question is unanswered and none has been pointed to yet: then scrolls to - /// the first and outlines it, once. The phone having lost its place changes nothing here, so - /// the second tap still sends the scan. + /// Sends, unless a question is unanswered and the homeowner hasn't been warned since the last + /// time every question had an answer: then scrolls to the first and outlines it. The phone + /// having lost its place changes nothing here, so the second tap still sends the scan. private func confirm(_ proxy: ScrollViewProxy) { - guard nudged == nil, let question = firstUnanswered else { + guard !warned, let question = firstUnanswered else { actions.confirmFeatures() return } - nudged = question + warned = true withAnimation(reduceMotion ? nil : Motion.settle) { proxy.scrollTo(question, anchor: .top) } @@ -156,7 +164,7 @@ struct MarkFeaturesScreen: View { VStack(spacing: 10) { ForEach(state.features) { feature in FeatureRow(feature: feature, pastEnd: state.featuresPastEnds.contains(feature.id), - highlighted: nudged == .window(feature.id) && feature.awaitsOpensAnswer, actions: actions) + highlighted: isOutlined(.window(feature.id)), actions: actions) .id(ReviewQuestion.window(feature.id)) .transition(reduceMotion ? .opacity : .opacity.combined(with: .scale(scale: 0.98))) } @@ -167,7 +175,7 @@ struct MarkFeaturesScreen: View { } /// A mark is a tap into the scene, so while the phone has lost its place the chips give way - /// to a line that says so; "Looks complete" still sends the scan. + /// to a line that says so; "Looks complete" still finishes the review. private var addSomething: some View { VStack(alignment: .leading, spacing: 10) { Text("Add something") @@ -175,7 +183,7 @@ struct MarkFeaturesScreen: View { .accessibilityAddTraits(.isHeader) if state.tracking.hasLostItsPlace { Label { - Text("Your phone lost its place. Point it back at the wall to add more, or tap Looks complete to send the scan.") + Text(ScanCopy.reviewLostPlace) } icon: { Image(systemName: "location.slash.fill") .foregroundStyle(Palette.muted) @@ -247,6 +255,7 @@ private struct FeatureRow: View { } .frame(maxWidth: .infinity, alignment: .leading) .accessibilityElement(children: .combine) + .accessibilityIdentifier(pastEnd ? "review.featurePastEnd" : "review.feature") Button(role: .destructive) { actions.deleteFeature(feature.id) } label: { diff --git a/ios/HouseScan/UI/Screens/ResultARScreen.swift b/ios/HouseScan/UI/Screens/ResultARScreen.swift index fa37ca3b5..f6bf407dc 100644 --- a/ios/HouseScan/UI/Screens/ResultARScreen.swift +++ b/ios/HouseScan/UI/Screens/ResultARScreen.swift @@ -12,6 +12,17 @@ struct ResultARScreen: View { let actions: any ScanActions @State private var appeared = false + /// The open camera between the card and Done, for the chevron (`CameraChrome`). + @State private var cameraWindow = CameraWindow() + /// The whole screen in global coordinates, which the chevron and the battery are drawn in. + @State private var screen: CGRect = .zero + /// The chevron's words as laid out at the current text size (`SpotDirection.label`). The guess + /// stands in for the first layout pass only. + @State private var spotLabelHeight: CGFloat = 36 + /// The way the spot lay last time Details was closed, nil while it was in view: the memory + /// behind the direction's axis bias and the entry margin (`SpotDirection.layout`). Not + /// updated while Details is open, since covering the camera doesn't move the spot. + @State private var spotPrevious: AimDirection? = nil @Environment(\.accessibilityReduceMotion) private var reduceMotion var body: some View { @@ -34,26 +45,65 @@ struct ResultARScreen: View { .accessibilityAddTraits(.isImage) .accessibilityIdentifier("ar.overlay") } - if let spot = result.spotCenter(on: wall) { - SpotDirection(projection: projection, spot: spot) + if let spotLayout { + SpotDirection(layout: spotLayout.layout, size: screen.size) { height in + if abs(height - spotLabelHeight) > 0.5 { spotLabelHeight = height } + } } } CameraChrome( - instruction: instruction, + instruction: visibleInstruction, isReplay: state.isReplay, isAutopilot: state.isAutopilot, - aims: showsSpot + cameraWindow: cameraWindow, + aims: showsSpot, + noteTakesCameraRoom: true ) { Button("Done") { actions.closeAR() } .buttonStyle(.primary) .accessibilityIdentifier("action.closeAR") } } + .background { + // Measured inside `ignoresSafeArea`, so it is the whole screen, as the drawing is. + // Measured outside, it was the safe area (62 to 840 pt on an iPhone 17). + Color.clear + .onGeometryChange(for: CGRect.self, of: { $0.frame(in: .global) }) { screen = $0 } + .ignoresSafeArea() + } + .onChange(of: cameraWindow.detailsShown ? nil : spotLayout?.layout) { _, layout in + // Missing tracking or projection says nothing about visibility. Observe the layout, + // not its optional direction, so recovery to a valid `.none` still clears memory. + guard let layout else { return } + if spotPrevious != layout.direction { spotPrevious = layout.direction } + } .onAppear { withAnimation(reduceMotion ? .easeOut(duration: 0.2) : .spring(duration: 0.7, bounce: 0.15)) { appeared = true } } } + /// Where the chevron goes, and the open camera it was judged in; nil without a spot to show. + /// + /// Worked out here in the body, as is the card's line from it (`visibleInstruction`), not + /// reported up from the chevron's view: a line passed up through `onChange` changed the + /// card, which moved the camera in the same frame, and SwiftUI dropped the second update + /// ("onChange(of: Outcome) action tried to update multiple times per frame"). The card then + /// kept a line the chevron no longer gave: "The camera view is below" with Details closed + /// (screenStates run 38687149601 at 602bd92) and "Turn the phone right" with the spot in + /// view (run 38691053767 at 131f81d, and locally on an iPhone 17 simulator). + private var spotLayout: (layout: SpotDirection.Layout, band: WayfindingOverlay.Band?)? { + guard state.tracking == .normal, !screen.isEmpty, let projection = state.projection, let wall = state.wall, + let spot = state.result?.spotCenter(on: wall) else { return nil } + // Invariant: the line follows from the spot, Details and the camera without the card's + // note only, so showing it changes nothing it was decided from and its own height can't + // keep it up. Judged against the camera it shrinks, a five-line note at AX5 covered the + // spot, stayed, and pushed Done off screen (CI run of #241 at d5d137ed). + let band = WayfindingOverlay.verticalBand(cameraWindow.noteFreeFrame, origin: screen.origin, height: screen.height) + let layout = SpotDirection.layout(projection: projection, spot: spot, in: screen.size, band: band, + labelHeight: spotLabelHeight, previous: spotPrevious) + return (layout, band) + } + /// Whether the spot is on the wall in view: the phone tracks and the result has a spot. Then /// the homeowner is looking at the spot, so at the accessibility text sizes the card folds /// its detail (the placement, and what the fit still needs) under Details and leaves the @@ -71,6 +121,17 @@ struct ResultARScreen: View { : "An outline of the spot an installer needs to check, drawn on your wall" } + private var visibleInstruction: Instruction { + var copy = instruction + let note = spotLayout.flatMap { SpotDirection.note(layout: $0.layout, band: $0.band, detailsShown: cameraWindow.detailsShown) } + switch showsSpot ? note : nil { + case .turn(let direction)?: copy.note = ScanCopy.proposedSpotOffScreen(turn: direction) + case .cameraBelow?: copy.note = ScanCopy.cameraBelowDetails + case nil: break + } + return copy + } + private var instruction: Instruction { if state.tracking != .normal { return Instruction(title: "Point at your meter", detail: "The battery comes back once your phone finds its place.") @@ -107,56 +168,148 @@ extension ResultPresentation { } /// An edge chevron toward the battery spot while its middle is off screen, with words saying -/// what it points at. Nothing while the spot is on screen. +/// what it points at. Nothing while the spot is in view. Draws what `ResultARScreen` works out. private struct SpotDirection: View { - var projection: CameraProjection - var spot: SIMD3 + /// The card's line when the camera can't show the spot or the chevron. + enum Note: Equatable { + /// Details is closed and the open camera is too short for the chevron and its words: + /// the way to turn, which the card shows outside Details at every text size. + case turn(AimDirection) + /// Details is open and the camera it leaves can't show the spot or the chevron: less + /// camera on screen than the chevron's disc, as on the walk + /// (`WallWalkScreen.detailsCoverCamera`), or too little for the chevron's words. + /// Closing Details brings the camera back, so the card says that and gives no direction. + case cameraBelow + } + + enum Layout: Equatable { + case none + case chevron(CGPoint, angle: Angle, label: CGPoint, direction: AimDirection) + case unshown(AimDirection) + + var direction: AimDirection? { + switch self { + case .none: nil + case .chevron(_, _, _, let direction), .unshown(let direction): direction + } + } + + var unshownDirection: AimDirection? { + if case .unshown(let direction) = self { direction } else { nil } + } + } + + /// Where the chevron goes, in a view of the whole screen's `size`. + var layout: Layout + var size: CGSize + /// Called with the chevron's words' height (`label`), measured from an unseen copy that is + /// always laid out, so the chevron can be placed before it first shows. + var onLabelHeight: (CGFloat) -> Void var body: some View { - GeometryReader { proxy in - let size = proxy.size - if let chevron = chevronPlacement(in: size) { - ZStack { - TargetMarker(placement: .offScreen(chevron.point, angle: chevron.angle)) - Text(ScanCopy.proposedSpotThisWay) - .font(.subheadline.weight(.semibold)) - .foregroundStyle(Palette.chalk) - .multilineTextAlignment(.center) - .padding(.horizontal, 12) - .padding(.vertical, 6) - // Solid, as the aim ring's legend: see-through black over a bright wall - // failed the accessibility audit's contrast check there. - .background(ScrimShape.capsule) - .frame(maxWidth: 220) - .fixedSize(horizontal: false, vertical: true) - .position(x: min(max(chevron.point.x, 120), max(size.width - 120, 120)), y: chevron.point.y + 50) + // Keep the drawing from resizing the screen it was measured from. Without this + // boundary, the AX3 fixture cycles through screen heights of 874...900.25 pt as the + // chevron appears and disappears, keeping SwiftUI in layout before its first frame. + GeometryReader { _ in + ZStack { + label + .hidden() + .accessibilityHidden(true) + .onGeometryChange(for: CGFloat.self, of: { $0.size.height }) { onLabelHeight($0) } + if case let .chevron(point, angle, label, direction) = layout { + ZStack { + TargetMarker(placement: .offScreen(point, angle: angle)) + self.label + .position(label) + } + .frame(width: size.width, height: size.height) + .accessibilityElement(children: .ignore) + // The way to turn in words: below AX5 the card no longer says it beside a drawn + // chevron, so this is all VoiceOver has. + .accessibilityLabel(ScanCopy.proposedSpotOffScreen(turn: direction)) + .accessibilityIdentifier("ar.spotDirection") } - .frame(width: size.width, height: size.height) - .accessibilityElement(children: .ignore) - .accessibilityLabel(ScanCopy.proposedSpotOffScreen) - .accessibilityIdentifier("ar.spotDirection") } + .frame(maxWidth: .infinity, maxHeight: .infinity) } .ignoresSafeArea() .allowsHitTesting(false) } - /// Where the chevron goes and which way it points, or nil while the spot is in view. In view - /// means in the part of the screen the chrome leaves clear: a spot under the instruction card - /// (its bottom edge is about 220 pt down with a two-line detail) or under the Done button - /// (its top edge is about 100 pt up) can't be seen, so it gets the chevron too. The chevron - /// sits in the same lane as the walk's aim chevron (`WayfindingOverlay`), clear of the card - /// above and the buttons below. - private func chevronPlacement(in size: CGSize) -> (point: CGPoint, angle: Angle)? { - let clear = CGRect(x: 24, y: 260, width: size.width - 48, height: max(size.height - 260 - 160, 80)) - if let point = projection.viewPoint(for: spot, in: size), clear.contains(point) { return nil } - guard let direction = projection.screenDirection(toward: spot) else { return nil } - let lane = CGRect(x: 40, y: 260, width: size.width - 80, height: max(size.height - 260 - 300, 80)) + private var label: some View { + Text(ScanCopy.proposedSpotThisWay) + .font(.subheadline.weight(.semibold)) + .foregroundStyle(Palette.chalk) + .multilineTextAlignment(.center) + .padding(.horizontal, 12) + .padding(.vertical, 6) + // Solid, as the aim ring's legend: see-through black over a bright wall failed the + // accessibility audit's contrast check there. + .background(ScrimShape.capsule) + .frame(maxWidth: 220) + .fixedSize(horizontal: false, vertical: true) + } + + /// The gap between the chevron's disc and its words. + static let labelGap: CGFloat = 6 + + /// The card's line for `layout` in the open camera `band`. With Details closed, the way to + /// turn whenever the chevron can't be drawn, never folded away. With Details open, only + /// `Note.cameraBelow`: turning can't bring back a camera that Details covers. It shows + /// whenever the chevron can't be drawn as well, or an off-screen spot with Details open + /// and a camera between 52 pt and the chevron's height had neither (review of #241). + static func note(layout: Layout, band: WayfindingOverlay.Band?, detailsShown: Bool) -> Note? { + guard detailsShown else { return layout.unshownDirection.map(Note.turn) } + if layout.unshownDirection != nil { return .cameraBelow } + guard let band, band.bottom - band.top < WayfindingOverlay.arrowDiameter else { return nil } + return .cameraBelow + } + + /// Where the chevron goes, which way it points and where its words sit; `.none` while the + /// spot is in view, `.unshown` with the way to turn when the open camera can't hold them. + /// `labelHeight` is the words' measured height: at the accessibility sizes they wrap to + /// several lines, and a fixed allowance let them run under Done (review of #241). + /// + /// In view means in the open camera between the card and Done, as measured + /// (`CameraWindow`): a spot under the card or the button can't be seen, so it gets the + /// chevron. The fixed bands before the chrome reports were right only at the default text + /// size; from xxxLarge up the card passes them and covered both the spot and the chevron + /// (review of #132 and #137). The chevron keeps to the same window, inset as the walk's + /// edge arrows are (`WayfindingOverlay.lane`), with room for its words left under it. + static func layout( + projection: CameraProjection, spot: SIMD3, in size: CGSize, band: WayfindingOverlay.Band?, labelHeight: CGFloat, previous: AimDirection? + ) -> Layout { + let disc = WayfindingOverlay.arrowDiameter + let labelRoom = labelGap + labelHeight + let clear = band.map { CGRect(x: 24, y: $0.top, width: size.width - 48, height: $0.bottom - $0.top) } + ?? CGRect(x: 24, y: 260, width: size.width - 48, height: max(size.height - 260 - 160, 80)) + // A spot that was off screen (`previous`) comes back into view only a margin inside the + // window, so at its edge the card's note doesn't come and go every frame. + guard let direction = OpenCameraDirection.toward( + projection.viewPoint(for: spot, in: size), in: clear, fallback: projection.screenDirection(toward: spot), + wasOffScreen: previous != nil) else { return .none } + let namedDirection = WayfindingOverlay.direction(of: .radians(atan2(direction.dy, direction.dx)), previous: previous) + let lane: CGRect + if let band { + guard band.bottom - band.top >= disc + labelRoom else { + return .unshown(namedDirection) + } + // The words end by the band's bottom. A band too short for the inset lane leaves its + // middle line, where the guard above still fits the disc and the words. + let bottomLimit = band.bottom - labelRoom + let middle = (band.top + bottomLimit) / 2 + let top = min(band.top + WayfindingOverlay.arrowInset, middle) + let bottom = max(bottomLimit - WayfindingOverlay.arrowInset, middle) + lane = CGRect(x: 40, y: top, width: max(size.width - 80, 0), height: bottom - top) + } else { + lane = CGRect(x: 40, y: 260, width: size.width - 80, height: max(size.height - 260 - 300, 80)) + } let tx = direction.dx == 0 ? CGFloat.infinity : lane.width / 2 / abs(direction.dx) let ty = direction.dy == 0 ? CGFloat.infinity : lane.height / 2 / abs(direction.dy) let t = min(tx, ty) let point = CGPoint(x: lane.midX + direction.dx * t, y: lane.midY + direction.dy * t) - return (point, .radians(atan2(direction.dy, direction.dx))) + let label = CGPoint(x: min(max(point.x, 120), max(size.width - 120, 120)), y: point.y + disc / 2 + labelGap + labelHeight / 2) + return .chevron(point, angle: .radians(atan2(direction.dy, direction.dx)), label: label, direction: namedDirection) } } diff --git a/ios/HouseScan/UI/Screens/ResultScreen.swift b/ios/HouseScan/UI/Screens/ResultScreen.swift index e5b3e7ad1..d7f9a3970 100644 --- a/ios/HouseScan/UI/Screens/ResultScreen.swift +++ b/ios/HouseScan/UI/Screens/ResultScreen.swift @@ -180,6 +180,9 @@ struct ResultScreen: View { if !result.missing.isEmpty { MissingList(missing: result.missing, checks: result.checks, sourceAvailable: state.spatialResultAvailable, actions: actions) } + if !result.marksNotUsed.isEmpty { + MarksNotUsedList(messages: result.marksNotUsed) + } VStack(spacing: 16) { if let scan = state.shareableScan { ShareScanButton(url: scan) @@ -227,6 +230,34 @@ private struct Notice: View { } } +/// Marks the server set aside past an unexplored end (review of #53, #42): the review listed +/// them, so the result says they weren't counted rather than leaving the homeowner to think +/// they were measured. The server's sentence says which and why. +private struct MarksNotUsedList: View { + var messages: [String] + + var body: some View { + VStack(alignment: .leading, spacing: 12) { + Text(ScanCopy.marksNotUsedTitle) + .font(Typeface.sectionTitle) + .accessibilityAddTraits(.isHeader) + ForEach(Array(messages.enumerated()), id: \.offset) { _, message in + Text(message) + .font(Typeface.hint) + .fixedSize(horizontal: false, vertical: true) + .padding(14) + .frame(maxWidth: .infinity, alignment: .leading) + .background(Palette.surface, in: .rect(cornerRadius: 18, style: .continuous)) + .accessibilityIdentifier("result.markNotUsed") + } + Text(ScanCopy.marksNotUsedHint) + .font(.subheadline) + .foregroundStyle(Palette.muted) + .fixedSize(horizontal: false, vertical: true) + } + } +} + /// A text-only action inside Details: the card's primary button stays the one filled button. private struct TextActionStyle: ButtonStyle { func makeBody(configuration: Configuration) -> some View { diff --git a/ios/HouseScan/UI/Screens/WallWalkScreen.swift b/ios/HouseScan/UI/Screens/WallWalkScreen.swift index 699259dd0..e5fd03ade 100644 --- a/ios/HouseScan/UI/Screens/WallWalkScreen.swift +++ b/ios/HouseScan/UI/Screens/WallWalkScreen.swift @@ -18,13 +18,15 @@ struct WallWalkScreen: View { @State private var taps: [TapRipple.Ripple] = [] /// The aim ring's legend while it shows, drawn under the card (#81). @State private var cardLegend: String? = nil + /// What the overlay draws for the aim step's target (`WayfindingOverlay.onAimMarker`). + @State private var aimMarker: AimMarker? = nil @Environment(\.accessibilityReduceMotion) private var reduceMotion @Environment(\.dynamicTypeSize) private var typeSize var body: some View { ZStack { CameraSizeReader(size: $cameraSize) - CameraOverlays(state: state, highlight: nil, cardLegend: $cardLegend, cameraWindow: cameraWindow) + CameraOverlays(state: state, highlight: nil, cardLegend: $cardLegend, cameraWindow: cameraWindow, aimMarker: $aimMarker) if showsMeterPhoto, let meterPhoto { // "Point at the meter like this.": the saved close-up shows what to aim at. SavedMeterPhoto(image: meterPhoto) @@ -117,7 +119,38 @@ struct WallWalkScreen: View { let folded = Instruction.Folded(title: ScanCopy.wallTooShortLead, detail: "\(ScanCopy.wallTooShort) \(step)") return ScanCopy.withCoaching(Instruction(title: ScanCopy.wallTooShort, detail: step, folded: folded), coaching) } - return ScanCopy.withCoaching(ScanCopy.guidance(state.guidance, hint: state.guidanceHint), coaching) + var copy = ScanCopy.withCoaching(ScanCopy.guidance(state.guidance, hint: shownHint), coaching) + if aimMarker == .covered, detailsCoverCamera { + copy.note = [copy.note, ScanCopy.cameraBelowDetails].compactMap { $0 }.joined(separator: " ") + } + return copy + } + + private var detailsCoverCamera: Bool { + guard cameraWindow.detailsShown, cameraSize.height > 0, + let band = WayfindingOverlay.verticalBand(cameraWindow.frame, origin: .zero, height: cameraSize.height) else { return false } + return band.bottom - band.top < WayfindingOverlay.arrowDiameter + } + + /// The card follows every marker the overlay draws. For a target behind the camera the + /// overlay reports the horizontal turn direction, which the card follows too. Without a + /// marker for the current aim, keep the step's title and other advice: an engine hint of + /// above otherwise leaves "Tilt up a little" even when Details hides the arrow (#241). + private var shownHint: GuidanceHint? { + guard var hint = state.guidanceHint else { return nil } + guard let aimMarker else { + hint.aim = nil + return hint + } + switch aimMarker { + case .covered: + hint.aim = nil + case .ring: + hint.aim = .onScreen + case .arrow(let direction): + hint.aim = direction + } + return hint } /// "Slow down", "Turn more slowly" and "Hold steady" are for walking and aiming. With the tray diff --git a/ios/HouseScanKit/Sources/HouseScanKit/Coverage/CoverageMap.swift b/ios/HouseScanKit/Sources/HouseScanKit/Coverage/CoverageMap.swift index 7462e2a61..07128b453 100644 --- a/ios/HouseScanKit/Sources/HouseScanKit/Coverage/CoverageMap.swift +++ b/ios/HouseScanKit/Sources/HouseScanKit/Coverage/CoverageMap.swift @@ -1299,8 +1299,8 @@ public struct CoverageMap: Sendable { case .left: leftEnd = s case .right: rightEnd = s } - if limitEnds.remove(side) != nil { replayPastLimits() } - revision += 1 + limitEnds.remove(side) + replayForChangedEnd() } /// Says whether the marked end on `side` is a real limit (scene.json's `limit`) or not @@ -1314,13 +1314,27 @@ public struct CoverageMap: Sendable { } /// Forgets a marked end, for example to walk past it when the server asks what lies beyond. + /// Replays the kept cameras to restore sightings the end clipped, preserving skipped and + /// withdrawn cells. Without kept cameras, sightings supplied through `record` stay as they are. public mutating func clearEnd(_ side: WalkSide) { switch side { case .left: leftEnd = nil case .right: rightEnd = nil } - if limitEnds.remove(side) != nil { replayPastLimits() } - revision += 1 + limitEnds.remove(side) + replayForChangedEnd() + } + + private mutating func replayForChangedEnd() { + // Precomputed `record` maps have no kept frames to replay; preserve their sightings. + guard !observedCameras.isEmpty else { + replayPastLimits() + revision += 1 + return + } + // Rebuild rather than merge: clear restores clipped sightings, and re-mark clips them + // again, without counting the same view twice. + replayObservedCameras(shiftingSkippedBy: 0) } /// Marks the not-yet-covered cells of `range` as skipped ("I can't get there"). @@ -1366,8 +1380,8 @@ public struct CoverageMap: Sendable { /// the air. So seen and covered cells are rebuilt by replaying `observedCameras`; marked ends /// move exactly (s is measured from the meter, so moving it by d along the wall moves every /// end by -d), and skipped cells move by whole cells, the remainder (under half a cell, - /// 7.6 cm, below tap error) waiting for later moves. The replay clips to the current ends, so - /// cells beyond an end seen before it was marked are gone until a new frame sees them. + /// 7.6 cm, below tap error) waiting for later moves. The replay clips to the current ends. + /// Clearing an end replays the kept cameras and restores sightings it clipped. /// Corners the walk followed move like the ends, so they keep their place in the world: pass /// `frame` with the corners it had (a copy of `wall` with a new meter or ground). /// diff --git a/ios/HouseScanKit/Sources/HouseScanKit/Guidance/GuidancePlanner.swift b/ios/HouseScanKit/Sources/HouseScanKit/Guidance/GuidancePlanner.swift index 8b18c8da8..9f873a96c 100644 --- a/ios/HouseScanKit/Sources/HouseScanKit/Guidance/GuidancePlanner.swift +++ b/ios/HouseScanKit/Sources/HouseScanKit/Guidance/GuidancePlanner.swift @@ -202,10 +202,6 @@ public struct GuidancePlanner: Sendable { var output = cues(for: task, coverage: coverage, camera: camera) output.switched = switched output.stalled = stalled - if let aim = Self.aim(task), let camera, let window = aimWindow(for: task, coverage: coverage) { - output.stepBack = coverage.wall.wallPoint(camera.position).out < config.tooClose - output.needsSecondPosition = coverage.needsSecondPosition(band: aim.band, range: window, from: camera.position) - } return output } @@ -221,11 +217,18 @@ public struct GuidancePlanner: Sendable { lastProgressAt = time } - /// The target and path of `task` seen from `camera`, without choosing a task. The engine calls - /// this for frames that must not move the task (a replay frame shown for a tap) so the cues - /// never lag behind the camera the screen shows. + /// The target, path and aim advice (`stepBack`, `needsSecondPosition`) of `task` seen from + /// `camera`, without choosing a task. The engine calls this for frames that must not move the + /// task (a replay frame shown for a tap) so the cues never lag behind the camera the screen + /// shows. The advice is worked out here rather than in `update` so such a frame can't keep the + /// step-back or step-aside line from where the phone was before (review of #120). public func cues(for task: GuidanceTask, coverage: CoverageMap, camera: CameraFrame?) -> GuidanceOutput { - GuidanceOutput(task: task, target: target(for: task, coverage: coverage, camera: camera), path: path(for: task, coverage: coverage, camera: camera)) + var output = GuidanceOutput(task: task, target: target(for: task, coverage: coverage, camera: camera), path: path(for: task, coverage: coverage, camera: camera)) + if let aim = Self.aim(task), let camera, let window = aimWindow(for: task, coverage: coverage) { + output.stepBack = coverage.wall.wallPoint(camera.position).out < config.tooClose + output.needsSecondPosition = coverage.needsSecondPosition(band: aim.band, range: window, from: camera.position) + } + return output } // MARK: Choosing diff --git a/ios/HouseScanKit/Sources/HouseScanKit/Guidance/SlowRelease.swift b/ios/HouseScanKit/Sources/HouseScanKit/Guidance/SlowRelease.swift new file mode 100644 index 000000000..09fb536e2 --- /dev/null +++ b/ios/HouseScanKit/Sources/HouseScanKit/Guidance/SlowRelease.swift @@ -0,0 +1,43 @@ +import Foundation + +/// A reading that shows as soon as it is first found and then holds still: later readings don't +/// change it, and it clears only once nothing has been found for `release` seconds. +/// +/// For a card line that must neither move nor come and go with every frame, but must not stick +/// once the thing it reports has gone. The walk-out request's "the space ends" line (#164) is the +/// one user: ARKit refines its planes about ten times a second, and a line that followed them +/// re-armed the card reply's lock each time. Latched for the whole request instead, a plane ARKit +/// later moved or removed left the card saying the walk was impossible (review of #168). +public struct SlowRelease: Sendable, Equatable { + /// Seconds with nothing found before the shown reading clears. + public let release: Double + /// The reading shown, if any. + public private(set) var value: Value? + /// When readings stopped being found while `value` was shown. + private var absentSince: Double? + + public init(release: Double) { + self.release = release + } + + /// Takes this frame's reading (nil when nothing was found) at `time`, seconds on any steady + /// clock, and returns the reading to show. + @discardableResult + public mutating func update(_ found: Value?, time: Double) -> Value? { + guard value != nil else { + value = found + return value + } + if found != nil { + absentSince = nil + } else if let absentSince { + if time - absentSince >= release { + value = nil + self.absentSince = nil + } + } else { + absentSince = time + } + return value + } +} diff --git a/ios/HouseScanKit/Sources/HouseScanKit/Overlay/OpenCameraDirection.swift b/ios/HouseScanKit/Sources/HouseScanKit/Overlay/OpenCameraDirection.swift new file mode 100644 index 000000000..5ac4ea6c3 --- /dev/null +++ b/ios/HouseScanKit/Sources/HouseScanKit/Overlay/OpenCameraDirection.swift @@ -0,0 +1,43 @@ +import CoreGraphics + +/// A direction to a projected spot outside the visible camera between the card and actions. +public enum OpenCameraDirection { + public enum Direction: Sendable, Equatable { + case above, below, left, right + } + + /// Names the direction drawn toward the open camera window. Keeping the previous axis + /// prevents diagonal jitter from changing the card's sentence on every frame. + public static func classify(_ direction: CGVector, previous: Direction? = nil) -> Direction { + let horizontal = abs(direction.dx), vertical = abs(direction.dy) + let bias = CGFloat(AimHint.axisBias) + let useVertical: Bool + switch previous { + case .above?, .below?: useVertical = vertical * bias >= horizontal + case .left?, .right?: useVertical = vertical > horizontal * bias + case nil: useVertical = vertical >= horizontal + } + if useVertical { return direction.dy < 0 ? .above : .below } + return direction.dx < 0 ? .left : .right + } + + /// How far inside the window a spot that was off screen must come before it counts as in + /// view again. At AX5 the card's off-screen note is about 290 pt, so a hand-held spot on the + /// window's edge toggled it on and off. 12 pt is the walk's ring entry margin + /// (`WayfindingOverlay.ringEntryMargin`), about a fingertip; not measured on a phone. + public static let entryMargin: CGFloat = 12 + + /// Projected spots use the visible window's center, since the full camera's center can be + /// covered by the card. A spot behind the camera has no projection and uses `fallback`. + /// Returns nil when the projected spot is already visible: anywhere in `window`, or, for a + /// spot that was off screen (`wasOffScreen`), `entryMargin` inside it. + public static func toward(_ projected: CGPoint?, in window: CGRect, fallback: CGVector?, wasOffScreen: Bool = false) -> CGVector? { + guard let projected else { return fallback } + let visible = wasOffScreen ? window.insetBy(dx: entryMargin, dy: entryMargin) : window + guard !visible.contains(projected) else { return nil } + let dx = projected.x - window.midX, dy = projected.y - window.midY + let length = (dx * dx + dy * dy).squareRoot() + guard length > 0 else { return nil } + return CGVector(dx: dx / length, dy: dy / length) + } +} diff --git a/ios/HouseScanKit/Sources/HouseScanKit/Overlay/OpenCameraWithoutNote.swift b/ios/HouseScanKit/Sources/HouseScanKit/Overlay/OpenCameraWithoutNote.swift new file mode 100644 index 000000000..1ea7ef843 --- /dev/null +++ b/ios/HouseScanKit/Sources/HouseScanKit/Overlay/OpenCameraWithoutNote.swift @@ -0,0 +1,41 @@ +import CoreGraphics + +/// Where the open camera between the instruction card and the actions would be if the card had +/// no note, worked out from the layout that has one. +/// +/// A line the card shows because of what the open camera holds must not be judged against the +/// camera it shrinks. In "See it on your wall" at AX5, a five-line off-screen note made the card +/// cover the spot, so the spot stayed off screen and the note stayed with it, pushing Done out of +/// reach (CI run of #241 at d5d137ed, ProvisionalPlacementUITests). Judged here, the note's own +/// height can't keep it up. +/// +/// The camera chrome is a column, as tall as the screen or scrolling when taller, whose open +/// camera stretches to fill the screen but keeps at least `minimumHeight`. The note sits in the +/// card above it. Without the note the camera starts `noteRoom` higher; it is as tall as the +/// screen leaves it, and a column that gets shorter can't stay scrolled past its new end. The +/// answer is the settled layout: a pull past the top while dragging is left out. +public enum OpenCameraWithoutNote { + /// - Parameters: + /// - window: The open camera as laid out with the note, in global coordinates. + /// - content: The whole column as laid out with the note, in global coordinates. Its top + /// moves up as the column scrolls. + /// - viewport: The safe area the column fills and scrolls in, in global coordinates: at + /// rest the column's top is the viewport's top. Not a scroll view's frame that runs under + /// the status bar and the home indicator. + /// - noteRoom: How much taller the note makes the card. Zero still restores the minimum + /// camera height and clamps the scroll, which can lag behind the note's removal. + /// - minimumHeight: The least open camera the column keeps without the note. + public static func frame(window: CGRect, content: CGRect, viewport: CGRect, noteRoom: CGFloat, minimumHeight: CGFloat) -> CGRect { + // Reconstruct even with no note: the AX5 Details trace in #241 measured a zero-height + // spacer just after note removal, then a collapsed column still scrolled past its end. + // Returning either transient window brings the off-screen note straight back. + // Everything in the column but the open camera, without the note. + let rest = content.height - window.height - noteRoom + let height = max(minimumHeight, viewport.height - rest) + let endOffset = max(rest + height - viewport.height, 0) + // Settled: a pull past the top springs back, and a column that fits doesn't scroll. + let offset = min(max(viewport.minY - content.minY, 0), endOffset) + let top = viewport.minY - offset + (window.minY - content.minY) - noteRoom + return CGRect(x: window.minX, y: top, width: window.width, height: height) + } +} diff --git a/ios/HouseScanKit/Sources/HouseScanKit/Replay/ReplayPlanning.swift b/ios/HouseScanKit/Sources/HouseScanKit/Replay/ReplayPlanning.swift index d207eb48c..7461fa511 100644 --- a/ios/HouseScanKit/Sources/HouseScanKit/Replay/ReplayPlanning.swift +++ b/ios/HouseScanKit/Sources/HouseScanKit/Replay/ReplayPlanning.swift @@ -137,9 +137,10 @@ public enum ReplayPlanning { public enum EndPolicy: Sendable, Equatable { /// Marked at the covered extremes, as the autopilot's `markEnds` does. case coveredExtremes - /// Set where the phone stood farthest each way ("Can't get there", `WalkedEnd`), after - /// "Can't get there" on the ground by the meter. The gap must then lie `margin` meters - /// inside those ends: "Can't get there" at an end also skips the cells there. + /// Set as "Can't get there" sets them with the phone at the frame that stood farthest + /// each way (`walkedEnds`), after "Can't get there" on the ground by the meter. The gap + /// must then lie `margin` meters inside those ends: "Can't get there" at an end also + /// skips the cells there. case walked(margin: Float) } @@ -174,7 +175,8 @@ public enum ReplayPlanning { rest.record(sightings[index], from: frames[index].camera.position) } let stood = frames.indices.filter { !window.contains($0) }.map { frames[$0] } - guard let ends = self.ends(policy, rest, stood: stood, wall: wall) else { continue } + let keptPositions = kept.sorted().filter { !window.contains($0) }.map { frames[$0].camera.position } + guard let ends = self.ends(policy, rest, stood: stood, walked: keptPositions, wall: wall) else { continue } prepare(&rest, ends: ends, policy: policy) guard let gap = planner.plan(rest), isWithinWalk(gap, kept.sorted().filter { !window.contains($0) }.map { frames[$0] }, wall: wall), @@ -222,24 +224,47 @@ public enum ReplayPlanning { var rest = frames rest.removeSubrange(window) var walked = simulateWalk(rest, wall: wall, config: config) - guard let ends = self.ends(policy, walked, stood: rest, wall: wall) else { return nil } + guard let ends = self.ends(policy, walked, stood: rest, walked: walked.walkedPositions, wall: wall) else { return nil } prepare(&walked, ends: ends, policy: policy) guard let gap = planner.plan(walked), isWithinWalk(gap, rest, wall: wall), isInside(gap, ends: ends, policy: policy), satisfiedWithLeadIn(frames, window: window, walked: walked, gap: gap, planner: planner) else { return nil } return HeldBackWindow(frames: window, ends: ends, gap: gap) } - private static func ends(_ policy: EndPolicy, _ map: CoverageMap, stood: [PlannedFrame], wall: WallFrame) -> ClosedRange? { + /// `walked` holds the positions of the kept views, as `CoverageMap.walkedPositions` does. + private static func ends( + _ policy: EndPolicy, _ map: CoverageMap, stood: [PlannedFrame], walked: [SIMD3], wall: WallFrame + ) -> ClosedRange? { switch policy { case .coveredExtremes: return coveredExtremes(map) case .walked: - let s = stood.map { wall.wallPoint($0.camera.position).s } - guard let low = s.min(), let high = s.max(), low < 0, high > 0 else { return nil } - return low...high + return walkedEnds(stood: stood, walked: walked, seen: map.seenExtent, wall: wall) } } + /// The ends the autopilot's "Can't get there" sets (`Autopilot.endWalkByCantGetThere`): on + /// each side the phone stands again at the frame that went farthest that way, and the engine + /// puts the end where `WalkedEnd.choose` does from there. That isn't always the frame's own + /// place: limited tracking, a phone far out from the wall or one well past the kept views + /// keeps the end short of it. Planning the held-back window against the frame's place alone + /// could leave a gap the walk never asks for (review of #169). Nil unless the ends fall + /// either side of the meter. + static func walkedEnds( + stood: [PlannedFrame], walked: [SIMD3], seen: ClosedRange?, wall: WallFrame + ) -> ClosedRange? { + func end(_ side: WalkSide) -> Float? { + let along = { (frame: PlannedFrame) in side.sign * wall.wallPoint(frame.camera.position).s } + guard let farthest = stood.max(by: { along($0) < along($1) }) else { return nil } + return WalkedEnd.choose( + side, phone: farthest.camera.position, trackingNormal: farthest.trackingNormal, walked: walked, wall: wall, + seen: seen + ).s + } + guard let low = end(.left), let high = end(.right), low < 0, high > 0 else { return nil } + return low...high + } + /// Sets the ends, and for a walked flow skips the ground by the meter, as "Can't get there" /// on the walk's first request does (`GuidancePlanner.groundByMeterDone`, ±0.5 m). private static func prepare(_ map: inout CoverageMap, ends: ClosedRange, policy: EndPolicy) { diff --git a/ios/HouseScanKit/Sources/HouseScanKit/Scene/PlacementResult.swift b/ios/HouseScanKit/Sources/HouseScanKit/Scene/PlacementResult.swift index e17efbf92..a6d48adf4 100644 --- a/ios/HouseScanKit/Sources/HouseScanKit/Scene/PlacementResult.swift +++ b/ios/HouseScanKit/Sources/HouseScanKit/Scene/PlacementResult.swift @@ -464,6 +464,23 @@ public struct PlacementMissingEvidence: Codable, Sendable, Equatable { } } +/// A marked object the server did not use (result.schema.json `objects_not_used`). +public struct PlacementObjectNotUsed: Codable, Sendable, Equatable { + /// "objects[] ", as in a check's `subject`. + public var object: String + /// "left" or "right". Kept as a string: the app doesn't read it, so a value the server adds + /// later mustn't stop the result screen. + public var side: String + /// The server's sentence for the homeowner. + public var message: String + + public init(object: String, side: String, message: String) { + self.object = object + self.side = side + self.message = message + } +} + public struct PlacementEnd: Codable, Sendable, Equatable { public var kind: PlacementEndKind /// Where the wall chain ends, in s feet. @@ -576,6 +593,9 @@ public struct PlacementResult: Codable, Sendable, Equatable { public var checks: [PlacementCheck] public var nearestConsidered: PlacementSpot? public var missingEvidence: [PlacementMissingEvidence] + /// Marks the server set aside: no footprint, past an unexplored end where the wall may turn + /// (`objects_not_used`). Optional in schema 1.0; empty when the answer has none. + public var objectsNotUsed: [PlacementObjectNotUsed] public var ends: PlacementEnds public var sweep: [PlacementSweepRun] public var stats: PlacementStats @@ -585,6 +605,7 @@ public struct PlacementResult: Codable, Sendable, Equatable { case schemaVersion = "schema_version" case nearestConsidered = "nearest_considered" case missingEvidence = "missing_evidence" + case objectsNotUsed = "objects_not_used" } /// Decodes a server result, throwing `PlacementDecodingError` for anything outside the schema. @@ -618,6 +639,7 @@ public struct PlacementResult: Codable, Sendable, Equatable { checks = try c.decode([PlacementCheck].self, forKey: .checks) nearestConsidered = try c.decodeIfPresent(PlacementSpot.self, forKey: .nearestConsidered) missingEvidence = try c.decode([PlacementMissingEvidence].self, forKey: .missingEvidence) + objectsNotUsed = try c.decodeIfPresent([PlacementObjectNotUsed].self, forKey: .objectsNotUsed) ?? [] ends = try c.decode(PlacementEnds.self, forKey: .ends) sweep = try c.decode([PlacementSweepRun].self, forKey: .sweep) stats = try c.decode(PlacementStats.self, forKey: .stats) @@ -635,6 +657,7 @@ public struct PlacementResult: Codable, Sendable, Equatable { try c.encode(checks, forKey: .checks) try c.encodeIfPresent(nearestConsidered, forKey: .nearestConsidered) try c.encode(missingEvidence, forKey: .missingEvidence) + if !objectsNotUsed.isEmpty { try c.encode(objectsNotUsed, forKey: .objectsNotUsed) } try c.encode(ends, forKey: .ends) try c.encode(sweep, forKey: .sweep) try c.encode(stats, forKey: .stats) diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/CaptureSessionCoordinatorTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/CaptureSessionCoordinatorTests.swift index 008d40758..9999ac588 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/CaptureSessionCoordinatorTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/CaptureSessionCoordinatorTests.swift @@ -349,19 +349,23 @@ struct NativeCaptureFixture: Sendable { @Test func aWorldResetStartsANewPacketAndDropsTheOld() async throws { defer { try? FileManager.default.removeItem(at: root) } let server = try LoopbackCaptureAPI() - server.state.withLock { $0.held = ["POST captures/files"] } + let http = HeldCaptureReplyHTTP(UncancellableHTTP(URLSessionCaptureHTTP.ephemeral(timeout: 10)), pathSuffix: "/files") + defer { http.release() } let coordinator = CaptureSessionCoordinator( - environment: NativeCaptureFixture.environment(endpoint: server.base, http: UncancellableHTTP(URLSessionCaptureHTTP.ephemeral(timeout: 10)), captures: captures)) + environment: NativeCaptureFixture.environment(endpoint: server.base, http: http, captures: captures)) coordinator.begin(recording: fixture.recording) coordinator.answerConsent(true) coordinator.kept(try fixture.photo(at: fixture.start + 2)) let first = try #require(coordinator.session) - for _ in 0..<500 where server.requests("POST captures/files").isEmpty { try await Task.sleep(for: .milliseconds(10)) } + try await waitUntilHeld(http) coordinator.newWorld("world reset", recording: fixture.recording, newScan: false) + let stateURL = CaptureUploader.stateURL(in: first.folder) + // Retirement is synchronous, but abandonment runs on the uploader's next turn. + // Release the old reply only after that turn, so the test exercises a late reply. + try await waitUntilAbandoned(stateURL, reason: "world reset") + http.release() coordinator.kept(try fixture.photo(at: fixture.start + 4)) - server.release("POST captures/files") - server.state.withLock { $0.held = [] } await coordinator.settle() await first.uploader!.settled() @@ -570,15 +574,38 @@ struct NativeCaptureFixture: Sendable { /// Waits, suspending, until the loopback has parked an answer for `route`. func waitUntilParked(_ server: LoopbackCaptureAPI, _ route: String) async throws { - for _ in 0..<1000 where server.state.withLock({ $0.parked[route]?.isEmpty ?? true }) { try await Task.sleep(for: .milliseconds(10)) } - #expect(server.state.withLock { !($0.parked[route]?.isEmpty ?? true) }) + for _ in 0..<1000 { + if server.state.withLock({ !($0.parked[route]?.isEmpty ?? true) }) { return } + try await Task.sleep(for: .milliseconds(10)) + } + try #require(server.state.withLock { !($0.parked[route]?.isEmpty ?? true) }) } - /// Blocks the main actor until `stateURL` holds something other than `before`, so nothing - /// queued on the main actor (the abandon task a withdrawal schedules) can run meanwhile. - func blockUntilSaved(_ stateURL: URL, differentFrom before: Data?) { - let deadline = Date().addingTimeInterval(5) - while Date() < deadline, (try? Data(contentsOf: stateURL)) == before { usleep(10_000) } + func waitUntilHeld(_ http: HeldCaptureReplyHTTP) async throws { + for _ in 0..<500 { + if http.parked { return } + try await Task.sleep(for: .milliseconds(10)) + } + try #require(http.parked) + } + + func waitUntilAbandoned(_ stateURL: URL, reason: String) async throws { + for _ in 0..<500 { + if (try? CaptureUploadState.load(from: stateURL))?.end == .abandoned(reason) { return } + try await Task.sleep(for: .milliseconds(10)) + } + try #require(try CaptureUploadState.load(from: stateURL).end == .abandoned(reason)) + } + + /// Keeps the main actor's queued abandon task from masking the late reply's own save. + /// A changed file is insufficient: another queued write can replace the withdrawal first. + func blockUntilAbandoned(_ stateURL: URL, reason: String) -> CaptureUploadState? { + for _ in 0..<500 { + if let saved = try? CaptureUploadState.load(from: stateURL), saved.end == .abandoned(reason) { return saved } + usleep(10_000) + } + let saved = try? CaptureUploadState.load(from: stateURL) + return saved?.end == .abandoned(reason) ? saved : nil } /// A reply that lands after the homeowner withdrew, but before the uploader is abandoned, @@ -589,9 +616,10 @@ struct NativeCaptureFixture: Sendable { @Test(arguments: [false, true]) func aReplyAfterWithdrawalCannotSaveAResumableCapture(markerBlocked: Bool) async throws { defer { try? FileManager.default.removeItem(at: root) } let server = try LoopbackCaptureAPI() - server.state.withLock { $0.held = ["POST captures/files"] } + let http = HeldCaptureReplyHTTP(UncancellableHTTP(URLSessionCaptureHTTP.ephemeral(timeout: 10)), pathSuffix: "/files") + defer { http.release() } let environment = NativeCaptureFixture.environment( - endpoint: server.base, http: UncancellableHTTP(URLSessionCaptureHTTP.ephemeral(timeout: 10)), captures: captures) + endpoint: server.base, http: http, captures: captures) let coordinator = CaptureSessionCoordinator(environment: environment) coordinator.begin(recording: fixture.recording) coordinator.answerConsent(true) @@ -599,7 +627,7 @@ struct NativeCaptureFixture: Sendable { let session = try #require(coordinator.session) let uploader = try #require(session.uploader) let stateURL = CaptureUploader.stateURL(in: session.folder) - try await waitUntilParked(server, "POST captures/files") + try await waitUntilHeld(http) if markerBlocked { // A directory where the marker goes makes writing it fail. try FileManager.default.createDirectory( @@ -610,10 +638,9 @@ struct NativeCaptureFixture: Sendable { // No suspension from here to the checks. let before = try? Data(contentsOf: stateURL) #expect((before == nil) == markerBlocked) - server.release("POST captures/files") + http.release() // The uploader saves the register reply on its own executor. - blockUntilSaved(stateURL, differentFrom: before) - let saved = try CaptureUploadState.load(from: stateURL) + let saved = try #require(blockUntilAbandoned(stateURL, reason: "consent withdrawn")) #expect(saved.end == .abandoned("consent withdrawn")) if markerBlocked { // The blocking directory sits at the marker's path and would stop a resume by itself; @@ -625,7 +652,6 @@ struct NativeCaptureFixture: Sendable { #expect(CaptureSessionCoordinator(environment: environment).resumeSealedCaptures().isEmpty) #expect(server.requests("PUT upload").isEmpty) - server.state.withLock { $0.held = [] } await uploader.settled() #expect(server.requests("PUT upload").isEmpty) } @@ -635,9 +661,10 @@ struct NativeCaptureFixture: Sendable { @Test func aResultAfterWithdrawalSavesTheUploadAsAbandoned() async throws { defer { try? FileManager.default.removeItem(at: root) } let server = try LoopbackCaptureAPI() - server.state.withLock { $0.held = ["GET captures/result"] } + let http = HeldCaptureReplyHTTP(UncancellableHTTP(URLSessionCaptureHTTP.ephemeral(timeout: 10)), pathSuffix: "/result") + defer { http.release() } let environment = NativeCaptureFixture.environment( - endpoint: server.base, http: UncancellableHTTP(URLSessionCaptureHTTP.ephemeral(timeout: 10)), captures: captures) + endpoint: server.base, http: http, captures: captures) let coordinator = CaptureSessionCoordinator(environment: environment) coordinator.begin(recording: fixture.recording) coordinator.answerConsent(true) @@ -649,16 +676,14 @@ struct NativeCaptureFixture: Sendable { let session = try #require(coordinator.session) let uploader = try #require(session.uploader) let stateURL = CaptureUploader.stateURL(in: session.folder) - try await waitUntilParked(server, "GET captures/result") + try await waitUntilHeld(http) coordinator.answerConsent(false) // No suspension from here to the check. - let before = try? Data(contentsOf: stateURL) - server.release("GET captures/result") - blockUntilSaved(stateURL, differentFrom: before) + http.release() + _ = try #require(blockUntilAbandoned(stateURL, reason: "consent withdrawn")) #expect(try CaptureUploadState.load(from: stateURL).end == .abandoned("consent withdrawn")) - server.state.withLock { $0.held = [] } await uploader.settled() #expect(await uploader.snapshot.end == .abandoned("consent withdrawn")) } diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/CaptureUploaderRecoveryTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/CaptureUploaderRecoveryTests.swift index d7b586417..4f073091d 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/CaptureUploaderRecoveryTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/CaptureUploaderRecoveryTests.swift @@ -30,8 +30,22 @@ import Testing waiting.forEach { $0() } } - static func saved(_ rig: Rig) throws -> CaptureUploadState { - try CaptureUploadState.load(from: CaptureUploader.stateURL(in: rig.capture.folder)) + /// The next request being parked does not prove the preceding state reached disk. + static func saved(_ rig: Rig) async throws -> CaptureUploadState { + let expected = await rig.uploader.snapshot + let url = CaptureUploader.stateURL(in: rig.capture.folder) + for _ in 0..<2000 { + if let saved = try? CaptureUploadState.load(from: url), saved == expected { return saved } + try await Task.sleep(for: .milliseconds(10)) + } + let saved = try CaptureUploadState.load(from: url) + try #require(saved == expected) + return saved + } + + static func savedBytes(_ rig: Rig) async throws -> Data { + _ = try await saved(rig) + return try Data(contentsOf: CaptureUploader.stateURL(in: rig.capture.folder)) } static func routes(_ server: LoopbackCaptureAPI) -> [String] { server.state.withLock { $0.log.map(LoopbackCaptureAPI.route) } } @@ -89,7 +103,7 @@ import Testing } Self.deliver(rig.server, "PUT upload") try #require(await Self.parked(rig.server, "POST captures")) - let gone = try Self.saved(rig) + let gone = try await Self.saved(rig) #expect(gone.captureID == nil) #expect(gone.files.values.map(\.attempts) == Array(repeating: 0, count: n)) #expect(gone.files.values.allSatisfy { $0.phase == .queued }) @@ -121,7 +135,7 @@ import Testing try #require(await Self.parked(rig.server, "POST captures/finalize")) Self.deliver(rig.server, "POST captures/finalize") try #require(await Self.parked(rig.server, "POST captures/finalize")) - #expect(try Self.saved(rig).finalizeRetries == 1) + #expect(try await Self.saved(rig).finalizeRetries == 1) // The second finalize is accepted. The server forgets the capture before the next poll. rig.server.state.withLock { s in @@ -132,7 +146,7 @@ import Testing } Self.deliver(rig.server, "POST captures/finalize") try #require(await Self.parked(rig.server, "POST captures")) - let gone = try Self.saved(rig) + let gone = try await Self.saved(rig) #expect(gone.captureID == nil) #expect(gone.finalizeRetries == 0) rig.server.release("POST captures") @@ -161,7 +175,7 @@ import Testing try #require(await Self.parked(rig.server, "POST captures")) let url = CaptureUploader.stateURL(in: folder) - let before = try Data(contentsOf: url) + let before = try await Self.savedBytes(rig) // A folder that takes no new file: the atomic save can't write its replacement. try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: folder.path) rig.server.release("POST captures") @@ -215,7 +229,7 @@ import Testing try #require(await Self.parked(rig.server, "POST captures")) let url = CaptureUploader.stateURL(in: folder) - let before = try Data(contentsOf: url) + let before = try await Self.savedBytes(rig) try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: folder.path) if order == .saveFailureFirst { rig.server.release("POST captures") @@ -282,7 +296,7 @@ import Testing await gate.entered() let url = CaptureUploader.stateURL(in: folder) - let before = try Data(contentsOf: url) + let before = try await Self.savedBytes(rig) try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: folder.path) await rig.uploader.add(images) gate.release() @@ -340,7 +354,7 @@ import Testing try #require(await Self.parked(rig.server, route)) let url = CaptureUploader.stateURL(in: folder) - let before = try Data(contentsOf: url) + let before = try await Self.savedBytes(rig) try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: folder.path) rig.server.release(route) try #require(await CaptureUploaderTests.settles(rig.uploader)) diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/CoverageMapTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/CoverageMapTests.swift index 07e94f33e..366e70c95 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/CoverageMapTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/CoverageMapTests.swift @@ -1,4 +1,4 @@ -import HouseScanKit +@testable import HouseScanKit import simd import Testing @@ -301,6 +301,56 @@ import Testing #expect(map.level(.wall, 0) == .seen) } + @Test func clearingAnEndReplaysSightingsPastIt() { + var map = CoverageMap(wall: standardWall()) + map.setEnd(.right, at: 0) + map.observe(Self.frontCamera(), trackingNormal: true) + #expect(map.level(.ground, 1) == .unseen) + map.clearEnd(.right) + #expect(map.level(.ground, 1) == .seen) + #expect(map.level(.wall, 1) == .seen) + } + + @Test func clearingAnEndPreservesPrecomputedSightingsWithoutKeptFrames() { + var map = CoverageMap(wall: standardWall()) + map.setEnd(.right, at: 0) + let sighting = CoverageMap.Sighting(band: .ground, index: -1, rows: Set(0...2), hiddenRows: []) + map.record([sighting], from: Self.front) + #expect(map.level(.ground, -1) == .seen) + map.clearEnd(.right) + #expect(map.level(.ground, -1) == .seen) + map.clearEnd(.right) + #expect(map.level(.ground, -1) == .seen) + #expect(map.coveredCount == 0) + } + + @Test func clearingAnEndTwiceDoesNotCountSightingsTwice() { + var map = CoverageMap(wall: standardWall()) + map.setEnd(.right, at: 0) + map.observe(Self.frontCamera(), trackingNormal: true) + map.clearEnd(.right) + let once = map + map.clearEnd(.right) + #expect(map.stateDifferences(from: once).filter { $0 != "revision" }.isEmpty) + #expect(map.level(.ground, 1) == .seen) + #expect(map.coveredCount == 0) + } + + @Test func clearingAndRemarkingAnEndKeepsTheSameCoverage() { + var map = CoverageMap(wall: standardWall()) + map.setEnd(.right, at: 0) + map.observe(Self.frontCamera(), trackingNormal: true) + map.observe(Self.frontCamera(x: 0.3), trackingNormal: true) + let neverCleared = map + map.clearEnd(.right) + map.setEnd(.right, at: 0) + #expect(map.stateDifferences(from: neverCleared).filter { $0 != "revision" }.isEmpty) + var expected = neverCleared + expected.clearEnd(.right) + map.clearEnd(.right) + #expect(map.stateDifferences(from: expected).filter { $0 != "revision" }.isEmpty) + } + @Test func coveredIntervalsOfHandBuiltRuns() { // Wall cameras at 0 and 0.3 cover cells whose lower edge L satisfies both 0.3 <= L + 0.9405 // and 0 >= L - 0.7881: L in [-0.6405, 0.7881], cells -4 ... 5, i.e. [-0.6096, 0.9144]. diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/GuidancePlannerTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/GuidancePlannerTests.swift index 159d01443..760fdb214 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/GuidancePlannerTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/GuidancePlannerTests.swift @@ -135,6 +135,23 @@ import Testing } } + /// Review of #120: a replay frame shown for review gets its cues from `cues(for:)`, which + /// must judge step-back and step-aside from that frame's camera, not the last live one. + @Test func cuesJudgeTheAdviceFromTheCameraGiven() { + var map = CoverageMap(wall: standardWall()) + map.observe(CoverageMapTests.frontCamera(), trackingNormal: true) + map.observe(CoverageMapTests.frontCamera(), trackingNormal: true) + let planner = GuidancePlanner() + let task = GuidanceTask.aimAtGround(s: 0) + let near = planner.cues(for: task, coverage: map, camera: Self.homeowner(out: 1)) + #expect(near.stepBack) + let far = planner.cues(for: task, coverage: map, camera: Self.homeowner()) + #expect(!far.stepBack) + #expect(planner.cues(for: task, coverage: map, camera: CoverageMapTests.frontCamera()).needsSecondPosition) + #expect(!planner.cues(for: task, coverage: map, camera: CoverageMapTests.frontCamera(x: 0.3)).needsSecondPosition) + #expect(!planner.cues(for: .walk(.left), coverage: map, camera: Self.homeowner(out: 1)).stepBack, "only aim steps carry the advice") + } + @Test func tooCloseAsksToStepBack() { var planner = GuidancePlanner() let map = CoverageMap(wall: standardWall()) diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/HeldCaptureReplyHTTP.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/HeldCaptureReplyHTTP.swift new file mode 100644 index 000000000..99aab14c4 --- /dev/null +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/HeldCaptureReplyHTTP.swift @@ -0,0 +1,103 @@ +import Foundation +import HouseScanKit +import Synchronization +import Testing + +/// Holds an already received reply, so test-controlled waiting does not spend a network timeout. +/// Cancellation leaves the reply waiting, as a background transfer can do after a world ends. +final class HeldCaptureReplyHTTP: CaptureHTTP { + enum HoldError: Error, Equatable { + case secondReplyWhileWaiting(path: String) + } + + private enum HoldDecision { + case parked, released, occupied + } + + private struct State { + var released = false + var waiting: CheckedContinuation? + } + + private let inner: any CaptureHTTP + private let pathSuffix: String + private let state = Mutex(State()) + + init(_ inner: any CaptureHTTP, pathSuffix: String) { + self.inner = inner + self.pathSuffix = pathSuffix + } + + var parked: Bool { state.withLock { $0.waiting != nil } } + + func release() { + let waiting = state.withLock { s in + s.released = true + let waiting = s.waiting + s.waiting = nil + return waiting + } + waiting?.resume() + } + + func send(_ request: URLRequest) async throws -> HTTPReply { + let reply = try await inner.send(request) + if request.url?.path.hasSuffix(pathSuffix) == true { + try await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in + let decision = state.withLock { s -> HoldDecision in + if s.released { return .released } + guard s.waiting == nil else { return .occupied } + s.waiting = continuation + return .parked + } + switch decision { + case .parked: break + case .released: continuation.resume() + case .occupied: + Issue.record("HeldCaptureReplyHTTP received a second reply for \(pathSuffix) while the first is waiting") + continuation.resume(throwing: HoldError.secondReplyWhileWaiting(path: pathSuffix)) + } + } + } + return reply + } + + func upload(_ request: URLRequest, file: URL) async throws -> HTTPReply { + try await inner.upload(request, file: file) + } +} + +@Suite struct HeldCaptureReplyHTTPTests { + private struct ImmediateReplyHTTP: CaptureHTTP { + func send(_ request: URLRequest) async throws -> HTTPReply { + HTTPReply(status: 200, body: Data()) + } + + func upload(_ request: URLRequest, file: URL) async throws -> HTTPReply { + try await send(request) + } + } + + @Test func aSecondHeldReplyFailsWithoutLosingTheFirst() async throws { + let http = HeldCaptureReplyHTTP(ImmediateReplyHTTP(), pathSuffix: "/result") + defer { http.release() } + let request = URLRequest(url: try #require(URL(string: "http://127.0.0.1/result"))) + let first = Task { try await http.send(request) } + let deadline = ContinuousClock.now + .seconds(5) + while !http.parked, ContinuousClock.now < deadline { + try await Task.sleep(for: .milliseconds(10)) + } + try #require(http.parked) + + var receivedError: (any Error)? + await withKnownIssue("The helper reports overlapping held replies as a test failure") { + do { _ = try await http.send(request) } + catch { receivedError = error } + } + #expect(receivedError as? HeldCaptureReplyHTTP.HoldError == .secondReplyWhileWaiting(path: "/result")) + #expect(http.parked) + http.release() + #expect(try await first.value == HTTPReply(status: 200, body: Data())) + #expect(!http.parked) + } +} diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/LoopbackCaptureAPI.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/LoopbackCaptureAPI.swift index 4a85db542..97fe59fe5 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/LoopbackCaptureAPI.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/LoopbackCaptureAPI.swift @@ -177,9 +177,9 @@ final class LoopbackCaptureAPI: Sendable { private func handle(_ request: Request, _ connection: NWConnection) { let route = Self.route(request) - let (drop, hold, redirect) = state.withLock { s -> (Bool, Bool, String?) in + let (drop, redirect) = state.withLock { s -> (Bool, String?) in s.log.append(request) - return (s.dropNext.remove(route) != nil, s.held.contains(route), s.redirects[route]) + return (s.dropNext.remove(route) != nil, s.redirects[route]) } let (status, body) = redirect == nil ? answer(request) : (307, Data()) let location = redirect.map { "Location: \($0)\r\n" } ?? "" @@ -188,11 +188,14 @@ final class LoopbackCaptureAPI: Sendable { let head = "HTTP/1.1 \(status) X\r\n\(location)Content-Type: application/json\r\nContent-Length: \(body.count)\r\nConnection: close\r\n\r\n" connection.send(content: Data(head.utf8) + body, completion: .contentProcessed { _ in connection.cancel() }) } - if hold { - state.withLock { $0.parked[route, default: []].append(send) } - } else { - send() + // Releasing a route and parking its reply share the lock: a release cannot miss a + // reply that was still being computed when it arrived. + let parked = state.withLock { s in + guard s.held.contains(route) else { return false } + s.parked[route, default: []].append(send) + return true } + if !parked { send() } } // MARK: API behaviour diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/OpenCameraDirectionTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/OpenCameraDirectionTests.swift new file mode 100644 index 000000000..09a1d1867 --- /dev/null +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/OpenCameraDirectionTests.swift @@ -0,0 +1,86 @@ +import CoreGraphics +import HouseScanKit +import Testing + +@Suite struct OpenCameraDirectionTests { + private static let camera = CGSize(width: 402, height: 874) + private static let window = CGRect(x: 24, y: 550, width: 354, height: 230) + + @Test func withoutPreviousDirectionUsesTheDominantAxis() { + #expect(OpenCameraDirection.classify(CGVector(dx: 1, dy: -2)) == .above) + #expect(OpenCameraDirection.classify(CGVector(dx: -1, dy: 2)) == .below) + #expect(OpenCameraDirection.classify(CGVector(dx: -2, dy: 1)) == .left) + #expect(OpenCameraDirection.classify(CGVector(dx: 2, dy: -1)) == .right) + #expect(OpenCameraDirection.classify(CGVector(dx: 1, dy: -1)) == .above) + } + + @Test(arguments: [OpenCameraDirection.Direction.above, .below, .left, .right]) + func diagonalJitterKeepsTheLastDirection(previous: OpenCameraDirection.Direction) { + let dx: CGFloat = previous == .left ? -1 : 1 + let dy: CGFloat = previous == .above ? -1 : 1 + var last = previous + for ratio: CGFloat in [0.99, 1.01, 0.98, 1.02] { + last = OpenCameraDirection.classify(CGVector(dx: dx, dy: dy * ratio), previous: last) + #expect(last == previous) + } + } + + @Test func aStrongAxisChangeUpdatesTheDirection() { + #expect(OpenCameraDirection.classify(CGVector(dx: -1.3, dy: -1), previous: .above) == .left) + #expect(OpenCameraDirection.classify(CGVector(dx: -1, dy: 1.3), previous: .left) == .below) + #expect(OpenCameraDirection.classify(CGVector(dx: 1.3, dy: 1), previous: .below) == .right) + #expect(OpenCameraDirection.classify(CGVector(dx: 1, dy: -1.3), previous: .right) == .above) + } + + /// The card covers this spot, even though it is below the full camera's center. Measuring + /// from that center used to point down, farther behind the card (review of #241). + @Test func aSpotBelowCameraCenterButAboveTheWindowPointsUp() throws { + let spot = CGPoint(x: Self.camera.width / 2, y: 500) + #expect(spot.y > Self.camera.height / 2) + #expect(spot.y < Self.window.minY) + let direction = try #require(OpenCameraDirection.toward(spot, in: Self.window, fallback: CGVector(dx: 0, dy: 1))) + #expect(direction == CGVector(dx: 0, dy: -1)) + } + + @Test func aSpotAtCameraCenterUnderTheCardStillHasADirection() { + let spot = CGPoint(x: Self.camera.width / 2, y: Self.camera.height / 2) + #expect(OpenCameraDirection.toward(spot, in: Self.window, fallback: nil) == CGVector(dx: 0, dy: -1)) + } + + @Test func aSpotUnderDonePointsDown() { + let spot = CGPoint(x: Self.window.midX, y: 810) + #expect(OpenCameraDirection.toward(spot, in: Self.window, fallback: nil) == CGVector(dx: 0, dy: 1)) + } + + @Test(arguments: [CGPoint(x: 10, y: 665), CGPoint(x: 390, y: 665)]) + func aSpotBesideTheWindowPointsToItsOwnSide(spot: CGPoint) { + let expected: CGFloat = spot.x < Self.window.midX ? -1 : 1 + #expect(OpenCameraDirection.toward(spot, in: Self.window, fallback: nil) == CGVector(dx: expected, dy: 0)) + } + + @Test func aVisibleSpotNeedsNoArrow() { + #expect(OpenCameraDirection.toward(CGPoint(x: 201, y: 600), in: Self.window, fallback: CGVector(dx: 1, dy: 0)) == nil) + } + + /// A spot just inside the window's edge: in view if it was, still off screen if it wasn't, + /// so a hand-held phone doesn't flip the card's note on every frame. + @Test(arguments: [CGPoint(x: 201, y: 556), CGPoint(x: 201, y: 774), CGPoint(x: 30, y: 665), CGPoint(x: 372, y: 665)]) + func aSpotOnTheWindowsEdgeKeepsItsLastState(spot: CGPoint) throws { + #expect(Self.window.contains(spot)) + #expect(OpenCameraDirection.toward(spot, in: Self.window, fallback: nil) == nil) + let direction = try #require(OpenCameraDirection.toward(spot, in: Self.window, fallback: nil, wasOffScreen: true)) + let outward = CGVector(dx: spot.x - Self.window.midX, dy: spot.y - Self.window.midY) + #expect(direction.dx * outward.dx + direction.dy * outward.dy > 0) + } + + @Test func aSpotWellInsideCountsAsInViewAgain() { + let spot = CGPoint(x: 201, y: Self.window.minY + OpenCameraDirection.entryMargin + 1) + #expect(OpenCameraDirection.toward(spot, in: Self.window, fallback: nil, wasOffScreen: true) == nil) + } + + @Test func aSpotBehindTheCameraKeepsTheCameraDirection() { + let direction = CGVector(dx: -1, dy: 0) + #expect(OpenCameraDirection.toward(nil, in: Self.window, fallback: direction) == direction) + #expect(OpenCameraDirection.toward(nil, in: Self.window, fallback: nil) == nil) + } +} diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/OpenCameraWithoutNoteTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/OpenCameraWithoutNoteTests.swift new file mode 100644 index 000000000..730f2c354 --- /dev/null +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/OpenCameraWithoutNoteTests.swift @@ -0,0 +1,148 @@ +import CoreGraphics +import HouseScanKit +import Testing + +@Suite struct OpenCameraWithoutNoteTests { + /// The iPhone 17's safe area under the status bar, about where the chrome scrolls. + private static let viewport = CGRect(x: 0, y: 62, width: 402, height: 778) + + /// The camera chrome's column as SwiftUI lays it out: `above` and `below` the open camera, + /// which fills the screen down to `minimum`, scrolled by `scroll` at most to the column's end. + private struct Column { + var above: CGFloat + var below: CGFloat + var minimum: CGFloat + var note: CGFloat = 0 + var scroll: CGFloat = 0 + + var window: CGRect { laidOut.window } + var content: CGRect { laidOut.content } + /// How far the column is scrolled once `scroll` is held to its end. + var appliedScroll: CGFloat { OpenCameraWithoutNoteTests.viewport.minY - content.minY } + + private var laidOut: (window: CGRect, content: CGRect) { + let viewport = OpenCameraWithoutNoteTests.viewport + let rest = above + note + below + let height = max(minimum, viewport.height - rest) + let offset = min(scroll, rest + height - viewport.height) + let content = CGRect(x: 0, y: viewport.minY - offset, width: viewport.width, height: rest + height) + return (CGRect(x: 16, y: content.minY + above + note, width: 370, height: height), content) + } + } + + private static func withoutNote(_ column: Column, minimum: CGFloat) -> CGRect { + OpenCameraWithoutNote.frame(window: column.window, content: column.content, viewport: viewport, + noteRoom: column.note, minimumHeight: minimum) + } + + @Test func withoutANoteTheWindowIsUnchanged() { + let column = Column(above: 300, below: 100, minimum: 180) + #expect(Self.withoutNote(column, minimum: 180) == column.window) + } + + /// Removing the note and restoring the spacer take separate SwiftUI updates. The AX5 + /// trace had no note while the spacer still had the note's zero-height minimum. + @Test func removingTheNoteRestoresTheCameraBeforeTheSpacerCatchesUp() { + let transient = Column(above: 760, below: 81, minimum: 0) + let settled = Column(above: 760, below: 81, minimum: 180) + #expect(transient.window.height == 0) + #expect(Self.withoutNote(transient, minimum: 180) == settled.window) + } + + /// Closing Details can leave the now screen-height column at its previous scroll offset + /// for one update. It must not report the spot below the camera and reinsert the note. + @Test(arguments: [-40, 63, 243, 496] as [CGFloat]) + func removingTheNoteClampsTheOldScrollOffset(offset: CGFloat) { + let settled = Column(above: 364, below: 81, minimum: 180) + let result = OpenCameraWithoutNote.frame( + window: settled.window.offsetBy(dx: 0, dy: -offset), + content: settled.content.offsetBy(dx: 0, dy: -offset), viewport: Self.viewport, + noteRoom: 0, minimumHeight: 180) + #expect(result == settled.window) + } + + @Test func removingTheNoteWhileScrollingRestoresBothMinimumAndScrollLimit() { + let transient = Column(above: 760, below: 81, minimum: 0) + let settled = Column(above: 760, below: 81, minimum: 180, scroll: 496) + let result = OpenCameraWithoutNote.frame( + window: transient.window.offsetBy(dx: 0, dy: -496), + content: transient.content.offsetBy(dx: 0, dy: -496), viewport: Self.viewport, + noteRoom: 0, minimumHeight: 180) + #expect(result == settled.window) + #expect(result.height == 180) + } + + /// The column fits: the note pushes the camera's top down and Done stays at the bottom. + @Test func aNoteThatFitsGivesBackItsRoomAboveTheCamera() { + let with = Column(above: 300, below: 100, minimum: 180, note: 150) + #expect(with.content.height == Self.viewport.height) + let without = Column(above: 300, below: 100, minimum: 180) + #expect(Self.withoutNote(with, minimum: 180) == without.window) + #expect(without.window.maxY == with.window.maxY) + } + + /// The failure in #241's CI run at d5d137ed: at AX5 the five-line note made the column + /// overflow, so Done moved below the screen. Without the note the camera ends above Done + /// again, not where the pushed-down camera ended. + @Test func aNoteThatPushesDoneOffScreenIsUndoneAtBothEdges() { + let with = Column(above: 306, below: 106, minimum: 180, note: 292) + #expect(with.content.height > Self.viewport.height) + let without = Column(above: 306, below: 106, minimum: 180) + let estimate = Self.withoutNote(with, minimum: 180) + #expect(estimate == without.window) + #expect(estimate.maxY < with.window.maxY) + } + + /// Scrolled to Done with the note up: without it the column is shorter than that scroll, + /// so it settles back to its end and the camera sits lower than a plain shift would put it. + @Test func aScrollPastTheShorterColumnsEndSettlesAtItsEnd() { + let with = Column(above: 306, below: 106, minimum: 180, note: 292, scroll: 200) + #expect(with.content.minY < Self.viewport.minY) + let without = Column(above: 306, below: 106, minimum: 180, scroll: 200) + #expect(without.content.minY == Self.viewport.minY) + #expect(Self.withoutNote(with, minimum: 180) == without.window) + } + + /// Pulled down past the top while the note made the column scroll: without the note the + /// column fits and can't be pulled, so the camera is where it settles. + @Test func aPullPastTheTopIsLeftOut() { + var with = Column(above: 306, below: 106, minimum: 180, note: 292) + let settled = with.window + let pulled = 40 as CGFloat + let estimate = OpenCameraWithoutNote.frame( + window: settled.offsetBy(dx: 0, dy: pulled), content: with.content.offsetBy(dx: 0, dy: pulled), + viewport: Self.viewport, noteRoom: with.note, minimumHeight: 180) + with.note = 0 + #expect(estimate == with.window) + } + + /// A column that overflows even without the note keeps the least camera and its scroll. + @Test func aColumnThatStillOverflowsKeepsTheLeastCameraAndTheScroll() { + let with = Column(above: 560, below: 106, minimum: 180, note: 120, scroll: 60) + let without = Column(above: 560, below: 106, minimum: 180, scroll: 60) + #expect(without.content.minY == Self.viewport.minY - 60) + let estimate = Self.withoutNote(with, minimum: 180) + #expect(estimate == without.window) + #expect(estimate.height == 180) + } + + /// A screen that lets the note take its room from the least camera (`CameraChrome`) lays the + /// note out over a shorter minimum. The answer is still the layout with the usual minimum. + @Test(arguments: [0, 60, 180] as [CGFloat]) + func theNoteLayoutsOwnMinimumDoesNotMatter(minimumWithNote: CGFloat) { + let with = Column(above: 420, below: 106, minimum: minimumWithNote, note: 292) + let without = Column(above: 420, below: 106, minimum: 180) + #expect(Self.withoutNote(with, minimum: 180) == without.window) + } + + /// Whatever the note, the scroll and the column, the answer is the layout without the note: + /// the note's own height never decides what the camera holds. + @Test(arguments: [0, 150, 360, 520] as [CGFloat], [0, 40, 180, 400] as [CGFloat]) + func theAnswerIsAlwaysTheLayoutWithoutTheNote(above: CGFloat, scroll: CGFloat) { + for note: CGFloat in [1, 52, 180, 292, 600] { + let with = Column(above: above, below: 106, minimum: 180, note: note, scroll: scroll) + let without = Column(above: above, below: 106, minimum: 180, scroll: with.appliedScroll) + #expect(Self.withoutNote(with, minimum: 180) == without.window, "note \(note)") + } + } +} diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/PlacementResultTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/PlacementResultTests.swift index ef0647716..b0b195041 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/PlacementResultTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/PlacementResultTests.swift @@ -71,6 +71,22 @@ import simd try Self.sample(replacing: #""key": "sample_window_clearance_ft""#, with: #""key": "sample_window_clearance_ft", "bogus": 1"#)) } + /// Review of #53: a mark the server set aside past an unexplored end is decoded, so the + /// result can say it wasn't counted. Absent, the list is empty, as in the sample. + @Test func decodesObjectsNotUsed() throws { + #expect(try PlacementResult.decode(Self.sampleData()).objectsNotUsed.isEmpty) + let message = "The window marked 12 ft left of the meter is past the left end of the scan, where the wall may turn, so it was not used." + let data = try Self.sample( + replacing: #""schema_version": "1.0","#, + with: #""schema_version": "1.0", "objects_not_used": [{"object": "objects[0] window", "side": "left", "message": "\#(message)"}],"#) + #expect(try SceneSchemas.result().validate(data) == []) + let result = try PlacementResult.decode(data) + #expect(result.objectsNotUsed == [PlacementObjectNotUsed(object: "objects[0] window", side: "left", message: message)]) + let encoded = try JSONEncoder().encode(result) + #expect(try SceneSchemas.result().validate(encoded) == []) + #expect(try PlacementResult.decode(encoded) == result) + } + /// A real answer from the hosted server (deployed from origin/t3/server at 737bf75) to the /// synthetic replay's scene, carrying fields newer than the first published schema. @Test func decodesARealServerAnswer() throws { diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/RebuildViewCacheTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/RebuildViewCacheTests.swift index 183165da0..c5af48054 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/RebuildViewCacheTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/RebuildViewCacheTests.swift @@ -246,9 +246,9 @@ import Testing #expect(after.reused > before.reused) // An end changes the basis: every view is worked out again. twin.step("right end") { $0.setEnd(.right, at: 3) } - twin.setSurface("far wall") #expect(twin.cached.rebuildViewCounts.computed == after.computed + 2 * frames) #expect(twin.cached.rebuildViewCounts.reused == after.reused) + twin.setSurface("far wall") } /// Seeded runs of every change the map takes, in random order. diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/ReplayPlanningTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/ReplayPlanningTests.swift index bb8a1efae..e247f555d 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/ReplayPlanningTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/ReplayPlanningTests.swift @@ -1,4 +1,4 @@ -import HouseScanKit +@testable import HouseScanKit import simd import Testing @@ -111,3 +111,45 @@ import Testing #expect(map.coveredFraction(.ground, in: -4...4) == 1) } } + +/// Review of #169: the "Can't get there" flow's ends, planned, are where the engine's own +/// chooser (`WalkedEnd.choose`) puts them from the farthest frame each way, not that frame's +/// place. +@Suite struct WalkedEndsPlanningTests { + static func frame(s: Float, out: Float = 2, trackingNormal: Bool = true) -> PlannedFrame { + PlannedFrame(camera: makeCamera(at: SIMD3(s, 1.2, out), forward: SIMD3(0, 0, -1), right: SIMD3(1, 0, 0)), timestamp: 0, trackingNormal: trackingNormal) + } + + @Test func normalTrackingWithinTheKeptViewsEndsAtThePhone() throws { + let wall = standardWall() + let stood = [Self.frame(s: -3), Self.frame(s: 0), Self.frame(s: 3)] + let ends = try #require(ReplayPlanning.walkedEnds(stood: stood, walked: stood.map(\.camera.position), seen: nil, wall: wall)) + #expect(nearlyEqual(ends.lowerBound, -3)) + #expect(nearlyEqual(ends.upperBound, 3)) + } + + @Test func limitedTrackingAtTheFarthestFrameKeepsTheEndToTheKeptViews() throws { + let wall = standardWall() + let stood = [Self.frame(s: -3), Self.frame(s: 0), Self.frame(s: 2), Self.frame(s: 4, trackingNormal: false)] + let kept = stood.prefix(3).map(\.camera.position) + let ends = try #require(ReplayPlanning.walkedEnds(stood: stood, walked: kept, seen: nil, wall: wall)) + #expect(nearlyEqual(ends.upperBound, 2), "limited tracking caps the end at the farthest kept view") + #expect(nearlyEqual(ends.lowerBound, -3)) + } + + @Test func aPhoneFarOutFromTheWallKeepsTheEndToTheKeptViews() throws { + let wall = standardWall() + let stood = [Self.frame(s: -3), Self.frame(s: 1), Self.frame(s: 4, out: WalkedEnd.phoneOutLimit + 1)] + let kept = stood.prefix(2).map(\.camera.position) + let ends = try #require(ReplayPlanning.walkedEnds(stood: stood, walked: kept, seen: nil, wall: wall)) + #expect(nearlyEqual(ends.upperBound, 1)) + } + + @Test func aPhoneWellPastTheEvidenceStopsMaxPastEvidenceOn() throws { + let wall = standardWall() + let stood = [Self.frame(s: -3), Self.frame(s: 1), Self.frame(s: 6)] + let kept = stood.prefix(2).map(\.camera.position) + let ends = try #require(ReplayPlanning.walkedEnds(stood: stood, walked: kept, seen: nil, wall: wall)) + #expect(nearlyEqual(ends.upperBound, 1 + WalkedEnd.maxPastEvidence)) + } +} diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/SlowReleaseTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/SlowReleaseTests.swift new file mode 100644 index 000000000..12a034b55 --- /dev/null +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/SlowReleaseTests.swift @@ -0,0 +1,31 @@ +import Testing +@testable import HouseScanKit + +struct SlowReleaseTests { + @Test func showsTheFirstReadingAndHoldsItWhileFound() { + var latch = SlowRelease(release: 2) + #expect(latch.update(nil, time: 0) == nil) + #expect(latch.update(5, time: 1) == 5) + #expect(latch.update(7, time: 1.1) == 5, "a later reading must not move the line") + } + + /// Review of #168: a plane ARKit moves or removes must not leave the line up for good. + @Test func clearsOnlyAfterNothingIsFoundForTheReleaseTime() { + var latch = SlowRelease(release: 2) + latch.update(5, time: 0) + #expect(latch.update(nil, time: 1) == 5) + #expect(latch.update(nil, time: 2.9) == 5) + #expect(latch.update(nil, time: 3) == nil) + #expect(latch.update(8, time: 3.1) == 8, "found again, the new reading shows") + } + + @Test func aReadingInsideTheReleaseTimeRestartsIt() { + var latch = SlowRelease(release: 2) + latch.update(5, time: 0) + latch.update(nil, time: 1) + #expect(latch.update(6, time: 2.5) == 5) + #expect(latch.update(nil, time: 3) == 5) + #expect(latch.update(nil, time: 4.9) == 5, "the absence counts from 3 s, not 1 s") + #expect(latch.update(nil, time: 5) == nil) + } +} diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/UpperWallFixtureTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/UpperWallFixtureTests.swift index 18a01e8fb..e37e22f74 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/UpperWallFixtureTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/UpperWallFixtureTests.swift @@ -73,6 +73,8 @@ import Testing rest.removeSubrange(held.frames) let stood = rest.map { wall.wallPoint($0.camera.position).s } let ends = (stood.min() ?? 0)...(stood.max() ?? 0) + // On this replay the farthest frames track normally and stand inside the kept views' + // reach, so the engine's chooser puts each end at the phone. #expect(held.ends == ends) #expect(ends.lowerBound + 1 <= held.gap.span.lowerBound && held.gap.span.upperBound <= ends.upperBound - 1) var walked = ReplayPlanning.simulateWalk(rest, wall: wall) diff --git a/ios/HouseScanUITests/PastEndReplayTests.swift b/ios/HouseScanUITests/PastEndReplayTests.swift new file mode 100644 index 000000000..d35d91eec --- /dev/null +++ b/ios/HouseScanUITests/PastEndReplayTests.swift @@ -0,0 +1,13 @@ +import XCTest + +final class PastEndReplayTests: XCTestCase { + @MainActor func testPastEndReplaysOldCoverageButStillNeedsAFreshPhoto() { + let app = XCUIApplication() + app.launchArguments = ["-enginePastEndReplayCheck"] + app.launch() + let result = app.staticTexts["engine.check"] + XCTAssertTrue(result.waitForExistence(timeout: 5)) + XCTAssertEqual(result.label, "passed") + app.terminate() + } +} diff --git a/ios/HouseScanUITests/ResultCardUITests.swift b/ios/HouseScanUITests/ResultCardUITests.swift index ebfc196b6..f69b38408 100644 --- a/ios/HouseScanUITests/ResultCardUITests.swift +++ b/ios/HouseScanUITests/ResultCardUITests.swift @@ -136,6 +136,30 @@ final class ResultCardUITests: XCTestCase { attach(app, name: "\(name)-details") } + /// Review of #53 (#42): a mark the server set aside past an unexplored end is listed under + /// Details as not counted, in the server's words. + @MainActor + func testAMarkTheServerSetAsideIsListedAsNotCounted() throws { + continueAfterFailure = false + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "result", "-uiDemoMarkPastEnd"] + app.launch() + XCTAssertTrue(element(app, "screen.result").waitForExistence(timeout: 15)) + let details = element(app, "result.details") + scrollIntoView(details, in: app) + details.tap() + let mark = element(app, "result.markNotUsed") + XCTAssertTrue(mark.waitForExistence(timeout: 10), "the set-aside mark is missing under Details") + XCTAssertTrue(mark.label.contains("The window marked 12 ft left of the meter"), mark.label) + XCTAssertTrue(mark.label.contains("so it was not used"), mark.label) + scrollIntoView(mark, in: app) + let hint = app.staticTexts["These marks are outside the part of the wall used for this answer."] + scrollIntoView(hint, in: app) + XCTAssertTrue(hint.exists) + XCTAssertFalse(app.staticTexts["To count them, scan again and walk on past that end of the wall."].exists) + attach(app, name: "result-marksNotUsed") + } + // MARK: Helpers @MainActor diff --git a/ios/HouseScanUITests/ScreenStatesUITests.swift b/ios/HouseScanUITests/ScreenStatesUITests.swift index 0f08148a0..827092d3f 100644 --- a/ios/HouseScanUITests/ScreenStatesUITests.swift +++ b/ios/HouseScanUITests/ScreenStatesUITests.swift @@ -51,6 +51,7 @@ final class ScreenStatesUITests: XCTestCase { ("markFeatures-lostPlace", ["-uiDemoPhase", "markFeatures", "-uiDemoCoaching", "relocalizing"], "markFeatures"), ("markFeatures-groundQuestion", ["-uiDemoGroundQuestion"], "markFeatures"), ("markFeatures-groundAnswered", ["-uiDemoGroundAnswer", "mulch"], "markFeatures"), + ("markFeatures-pastEnd", ["-uiDemoPhase", "markFeatures", "-uiDemoMarkPastEnd"], "markFeatures"), ("gapRequest", ["-uiDemoPhase", "gapRequest"], "gapRequest"), ("gapRequest-groundOut", ["-uiDemoPhase", "gapRequest", "-uiDemoGap", "groundOut"], "gapRequest"), ("gapRequest-walkOut", ["-uiDemoPhase", "gapRequest", "-uiDemoGap", "walkOut"], "gapRequest"), @@ -116,6 +117,10 @@ final class ScreenStatesUITests: XCTestCase { "markFeatures-groundQuestion": [(nil, "What's on the ground along this wall?")], "markFeatures-groundAnswered": [("ground.answered", "Mulch")], "markFeatures-lostPlace": [("review.lostPlace", "Your phone lost its place")], + // Review of #53: a mark wholly past a marked end says so on its row. + "markFeatures-pastEnd": [("review.featurePastEnd", "Window"), + ("review.featurePastEnd", "About 12 ft left of your meter"), + ("review.featurePastEnd", "Past the end of your scan")], // #80, #26: the gate's coaching rides on the task card. "wallWalk-slowDown": [("instruction", "Walk slowly to your right")], "wallWalk-tooDark": [("instruction", "It's dark here")], @@ -686,6 +691,480 @@ final class ScreenStatesUITests: XCTestCase { XCTAssertTrue(element(app, "screen.gapRequest").waitForExistence(timeout: 10), "the second tap must send anyway") } + /// Review of #131: once every question has an answer the gate re-arms. A window added after + /// that is pointed to by the next "Looks complete", not sent past. + @MainActor + func testAWindowAddedAfterEveryAnswerIsPointedToAgain() throws { + continueAfterFailure = false + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoPhase", "markFeatures"] + app.launch() + XCTAssertTrue(element(app, "screen.markFeatures").waitForExistence(timeout: 15)) + tap(app, "action.confirmFeatures") + XCTAssertTrue(element(app, "review.unanswered").waitForExistence(timeout: 5)) + tap(app, "ground.answer.gravel") + tap(app, "window.opens.notSure") + XCTAssertTrue(element(app, "review.unanswered").waitForNonExistence(timeout: 5)) + + // A window takes two marks: bottom-left corner, then top-right. + tap(app, "feature.window") + tap(app, "action.markPoint", timeout: 5) + tap(app, "action.markPoint", timeout: 5) + XCTAssertTrue(element(app, "action.confirmFeatures").waitForExistence(timeout: 5), "the review must come back after the mark") + XCTAssertFalse(element(app, "review.unanswered").exists, "the warning must wait for a tap") + + tap(app, "action.confirmFeatures") + XCTAssertTrue(element(app, "review.unanswered").waitForExistence(timeout: 5), "the new window's question must be pointed to") + XCTAssertFalse(element(app, "screen.gapRequest").exists, "the first tap after re-arming must not send") + tap(app, "action.confirmFeatures") + XCTAssertTrue(element(app, "screen.gapRequest").waitForExistence(timeout: 10), "the second tap must still send") + } + + /// Review of #120: the card names what the overlay draws. The engine's angle to the target + /// and the open camera between the card and the actions can disagree; planted here as the + /// engine's hint, the card must still follow the ring or the arrow on screen. + @MainActor + func testTheAimCardNamesWhatIsDrawn() throws { + continueAfterFailure = false + let app = XCUIApplication() + // A drawn ring, though the engine's angle says the target is above the view. + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "wallWalk", "-uiDemoAim", + "-uiDemoAimHint", "above", "-uiDemoAimHintLater", "onScreen", + "-UIPreferredContentSizeCategoryName", "UICTContentSizeCategoryAccessibilityM"] + app.launch() + XCTAssertTrue(element(app, "screen.wallWalk").waitForExistence(timeout: 15)) + // AX1 folds the legend into Details, leaving room for a ring at the optical center. + // Move the projection there while retaining the planted "above" engine hint. + CFNotificationCenterPostNotification( + CFNotificationCenterGetDarwinNotifyCenter(), CFNotificationName("dev.housescanning.uiDemo.aimHintLater" as CFString), + nil, nil, true) + XCTAssertTrue(element(app, "aim.ring").waitForExistence(timeout: 15)) + let card = element(app, "instruction") + XCTAssertTrue(card.label.contains("Put the ring on the strip"), "with the ring drawn the card reads \(card.label)") + attach(app, name: "wallWalk-aimHint-ring") + app.terminate() + + // An arrow, though the engine's angle says the target is on screen. + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "wallWalk", "-uiDemoAimOffScreen", "-uiDemoAimHint", "onScreen"] + app.launch() + XCTAssertTrue(element(app, "screen.wallWalk").waitForExistence(timeout: 15)) + XCTAssertFalse(element(app, "aim.ring").exists, "off screen, the arrow stands in for the ring") + let offCard = element(app, "instruction") + XCTAssertTrue(offCard.waitForExistence(timeout: 5)) + XCTAssertFalse(offCard.label.contains("ring"), "with an arrow drawn the card must not name the ring: \(offCard.label)") + attach(app, name: "wallWalk-aimHint-arrow") + app.terminate() + + // The engine already calls the target off screen to the left, but the tall card puts + // its drawn arrow above. The card must follow the arrow, not that engine classification. + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "wallWalk", + "-uiDemoAim", "-uiDemoAimHint", "left", "-uiDemoAimHintLater", "above"] + Self.largestText + app.launch() + XCTAssertTrue(element(app, "instruction.details").waitForExistence(timeout: 15)) + // Expanded Details fills the camera at AX5. Keep the tall card collapsed so its + // covered target has an open lane in which an arrow can actually be drawn. + CFNotificationCenterPostNotification( + CFNotificationCenterGetDarwinNotifyCenter(), CFNotificationName("dev.housescanning.uiDemo.aimHintLater" as CFString), + nil, nil, true) + let arrow = element(app, "aim.arrow") + XCTAssertTrue(arrow.waitForExistence(timeout: 5)) + XCTAssertTrue(arrow.label.contains("above"), arrow.label) + let aboveCard = app.descendants(matching: .any) + .matching(NSPredicate(format: "identifier == 'instruction' AND label CONTAINS %@", "Tilt up a little")).firstMatch + XCTAssertTrue(aboveCard.waitForExistence(timeout: 5), "the card ignored an arrow that disagrees with an off-screen hint") + attach(app, name: "wallWalk-aimHint-offScreenMismatch-AX5") + app.terminate() + + // With Details open at AX5, no camera lane remains. The card must name the step, + // not a ring that cannot be drawn, and say where the camera view went. + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "wallWalk", + "-uiDemoAim", "-uiDemoAimHint", "onScreen"] + Self.largestText + app.launch() + XCTAssertTrue(element(app, "instruction.details").waitForExistence(timeout: 15)) + let coveredCard = app.descendants(matching: .any) + .matching(NSPredicate(format: "identifier == 'instruction' AND label CONTAINS %@", "The camera view is below")).firstMatch + XCTAssertFalse(coveredCard.exists, "closed Details must not ask the homeowner to close it") + tap(app, "instruction.details") + XCTAssertTrue(coveredCard.waitForExistence(timeout: 5), "covered camera instructions must remain visible") + XCTAssertFalse(element(app, "aim.ring").exists) + XCTAssertFalse(element(app, "aim.arrow").exists) + XCTAssertFalse(coveredCard.label.contains("Put the ring"), coveredCard.label) + XCTAssertTrue(coveredCard.label.contains("Tilt down"), coveredCard.label) + attach(app, name: "wallWalk-aimHint-covered-AX5") + tap(app, "instruction.details") + XCTAssertTrue(coveredCard.waitForNonExistence(timeout: 5), "closing Details must remove its camera obstruction advice") + } + + /// At AX5, opening Details hides the arrow. An engine hint of above must not keep the + /// marker-specific title up after the overlay reports covered. + @MainActor + func testAimCardDropsDirectionWhenDetailsCoversMarker() throws { + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "wallWalk", + "-uiDemoAim", "-uiDemoAimHint", "above", "-uiDemoAimHintLater", "above"] + Self.largestText + app.launch() + defer { app.terminate() } + XCTAssertTrue(element(app, "instruction.details").waitForExistence(timeout: 15)) + CFNotificationCenterPostNotification( + CFNotificationCenterGetDarwinNotifyCenter(), CFNotificationName("dev.housescanning.uiDemo.aimHintLater" as CFString), + nil, nil, true) + let arrow = element(app, "aim.arrow") + XCTAssertTrue(arrow.waitForExistence(timeout: 5)) + XCTAssertTrue(arrow.label.contains("above"), arrow.label) + let aboveCard = app.descendants(matching: .any) + .matching(NSPredicate(format: "identifier == 'instruction' AND label CONTAINS %@", "Tilt up a little")).firstMatch + XCTAssertTrue(aboveCard.waitForExistence(timeout: 5)) + + tap(app, "instruction.details") + let coveredCard = app.descendants(matching: .any) + .matching(NSPredicate(format: "identifier == 'instruction' AND label CONTAINS %@", "The camera view is below")).firstMatch + XCTAssertTrue(coveredCard.waitForExistence(timeout: 5)) + XCTAssertTrue(arrow.waitForNonExistence(timeout: 5)) + XCTAssertFalse(element(app, "aim.ring").exists) + XCTAssertTrue(coveredCard.label.contains("Tilt down to show the ground"), coveredCard.label) + XCTAssertFalse(coveredCard.label.contains("Tilt up a little"), coveredCard.label) + attach(app, name: "wallWalk-aimHint-covered-direction-AX5") + + tap(app, "instruction.details") + XCTAssertTrue(coveredCard.waitForNonExistence(timeout: 5)) + XCTAssertTrue(arrow.waitForExistence(timeout: 5)) + XCTAssertTrue(aboveCard.waitForExistence(timeout: 5), "closing Details must restore the drawn arrow's direction") + } + + /// A missing marker carries no actionable direction. The engine hint is deliberately planted + /// so this catches the card retaining a direction after the overlay reports no marker. + @MainActor + func testAimCardDropsDirectionWhenMarkerIsMissing() throws { + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "wallWalk", + "-uiDemoAimOffScreen", "-uiDemoAimHint", "above", "-uiDemoAimNoMarker"] + app.launch() + defer { app.terminate() } + let card = element(app, "instruction") + XCTAssertTrue(card.waitForExistence(timeout: 15)) + XCTAssertFalse(element(app, "aim.ring").exists, "a missing target must not draw a ring") + XCTAssertFalse(element(app, "aim.arrow").exists, "a missing target must not draw an arrow") + XCTAssertTrue(card.label.contains("Tilt down to show the ground"), card.label) + XCTAssertFalse(card.label.contains("Tilt up a little"), card.label) + } + + /// Marker memory belongs to the exact target and step, not the lifetime of the overlay. + @MainActor + func testAimHysteresisBelongsToTheExactTargetAndStep() throws { + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoAimHysteresis"] + app.launch() + defer { app.terminate() } + XCTAssertTrue(element(app, "demo.nextPose").waitForExistence(timeout: 15)) + let expected = ["left", "left", "above", "ring", "left", "left", "ring", "left", "left", "above"] + for (stage, name) in expected.enumerated() { + let marker = app.staticTexts.matching(NSPredicate(format: "identifier == 'demo.marker' AND label == %@", name)).firstMatch + XCTAssertTrue(marker.waitForExistence(timeout: 5), + "pose \(stage): expected \(name), got \(element(app, "demo.marker").label)") + if name == "ring" { + XCTAssertTrue(element(app, "aim.ring").exists, "pose \(stage): the reported ring must be drawn") + } else { + let arrow = element(app, "aim.arrow") + XCTAssertTrue(arrow.exists, "pose \(stage): the reported arrow must be drawn") + XCTAssertTrue(arrow.label.contains(name), + "pose \(stage): VoiceOver direction disagreed with the card: \(arrow.label)") + } + if stage < expected.count - 1 { tap(app, "demo.nextPose") } + } + } + + /// Losing a pose must not count as seeing the spot. A real in-view pose must clear memory, + /// including when it is the first valid pose after tracking recovers. + @MainActor + func testSpotHysteresisSurvivesUnavailableTrackingButClearsInView() throws { + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoSpotHysteresis"] + app.launch() + defer { app.terminate() } + XCTAssertTrue(element(app, "demo.nextPose").waitForExistence(timeout: 15)) + let direction = element(app, "ar.spotDirection") + let expected = [true, false, true, false, true, false, false, true, false, false, false] + for (stage, offScreen) in expected.enumerated() { + if offScreen { + XCTAssertTrue(direction.waitForExistence(timeout: 5), "pose \(stage): off-screen memory was lost") + if direction.exists { XCTAssertTrue(direction.label.contains("Turn the phone left"), direction.label) } + } else { + XCTAssertTrue(direction.waitForNonExistence(timeout: 5), "pose \(stage): unavailable or visible spot must not draw a direction") + } + if stage < expected.count - 1 { tap(app, "demo.nextPose") } + } + } + + /// With Details closed and no room in the open camera for the chevron and its words, the way + /// to turn is on the card, never folded under Details. At AX5 the sample's three-line title + /// leaves too little camera for the chevron's words; at AX3 the chevron fits, and since the + /// card no longer repeats a drawn chevron, AX3 can't show this state (review of #241). + @MainActor + func testTheOffScreenSpotInstructionStaysVisibleWithDetailsClosedAtAX5() throws { + continueAfterFailure = false + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "resultAR", + "-uiDemoSpotOffScreen"] + Self.largestText + app.launch() + defer { app.terminate() } + XCTAssertTrue(element(app, "instruction.details").waitForExistence(timeout: 15)) + let note = app.descendants(matching: .any) + .matching(NSPredicate(format: "identifier == 'instruction' AND label CONTAINS %@", "The proposed spot is off screen")).firstMatch + XCTAssertTrue(note.waitForExistence(timeout: 5), "the off-screen direction must not be folded into Details") + XCTAssertTrue(note.label.contains("Turn the phone right"), note.label) + XCTAssertFalse(element(app, "instruction.detail").exists, "Details must remain closed") + XCTAssertFalse(element(app, "ar.spotDirection").exists, "fixture must leave no room for a chevron and label") + XCTAssertTrue(app.frame.contains(note.frame), "the full instruction must be on the visible screen: \(note.frame)") + XCTAssertTrue(app.buttons["action.closeAR"].isHittable, "the note takes its room from the camera, not from Done") + attach(app, name: "resultAR-offScreen-note-AX5") + } + + /// At AX3 with Details closed, an off-screen spot always has its direction: the chevron, or + /// the card's line when the chevron doesn't fit. Records which one, since the card no longer + /// repeats a drawn chevron below AX5. + @MainActor + func testTheOffScreenSpotAlwaysHasADirectionAtAX3() throws { + continueAfterFailure = false + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "resultAR", + "-uiDemoSpotOffScreen", "-UIPreferredContentSizeCategoryName", "UICTContentSizeCategoryAccessibilityXL"] + app.launch() + defer { app.terminate() } + XCTAssertTrue(element(app, "instruction.details").waitForExistence(timeout: 15)) + let chevron = element(app, "ar.spotDirection") + let note = app.descendants(matching: .any) + .matching(NSPredicate(format: "identifier == 'instruction' AND label CONTAINS %@", "The proposed spot is off screen")).firstMatch + let either = XCTNSPredicateExpectation(predicate: NSPredicate { _, _ in chevron.exists || note.exists }, object: nil) + XCTAssertEqual(XCTWaiter().wait(for: [either], timeout: 5), .completed, "an off-screen spot had neither a chevron nor a line") + XCTAssertFalse(element(app, "instruction.detail").exists, "Details must remain closed") + let done = app.buttons["action.closeAR"] + XCTAssertTrue(done.isHittable, "the chevron must not enlarge the screen and push Done away") + XCTAssertTrue(app.frame.contains(done.frame), "Done must stay inside the screen") + if chevron.exists { + XCTAssertTrue(chevron.label.contains("Turn the phone right"), "VoiceOver must hear the way to turn: \(chevron.label)") + } else { + XCTAssertTrue(note.label.contains("Turn the phone right"), note.label) + } + let shown = XCTAttachment(string: chevron.exists ? "chevron: \(chevron.label)" : "note: \(note.label)") + shown.name = "resultAR-offScreen-direction-AX3" + shown.lifetime = .keepAlways + add(shown) + attach(app, name: "resultAR-offScreen-direction-AX3") + } + + /// At AX5 the folded card leaves the spot low in the open camera (`-uiDemoSpotLow`; the + /// default pose puts it under the sample's three-line title), and opening Details covers it. + /// At d5d137ed the card then said "Tilt the phone up" and kept saying it after Details + /// closed: the five-line note made the card cover the spot itself, and Done went below the + /// screen (CI run of #241, ProvisionalPlacementUITests). Closed again, the card must be as it + /// was, and open Details gives no direction to turn. + @MainActor + func testTheOffScreenNoteDoesNotKeepItselfUpAfterDetailsCloseAtAX5() throws { + continueAfterFailure = false + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "resultAR", "-uiDemoSpotLow"] + Self.largestText + app.launch() + defer { app.terminate() } + let card = element(app, "instruction") + let details = element(app, "instruction.details") + let done = app.buttons["action.closeAR"] + func cardSaying(_ words: String) -> XCUIElement { + app.descendants(matching: .any).matching(NSPredicate(format: "identifier == 'instruction' AND label CONTAINS %@", words)).firstMatch + } + XCTAssertTrue(details.waitForExistence(timeout: 15)) + XCTAssertFalse(cardSaying("off screen").waitForExistence(timeout: 2), "fixture: the folded card must leave the spot in view") + XCTAssertFalse(element(app, "ar.spotDirection").exists, "fixture: the spot must be in the open camera") + let folded = card.label + XCTAssertTrue(done.isHittable) + + tap(app, "instruction.details") + XCTAssertTrue(element(app, "instruction.detail").waitForExistence(timeout: 5)) + // Read once the card has had time to react to the covered camera; checked at the end, so + // the run before the fix fails on the note that stays. Open Details says the camera is + // below whenever the spot's chevron can't be drawn, not only when under 52 pt of camera + // is left: so the line doesn't depend on where the open card ends above the home + // indicator. + _ = cardSaying("The camera view is below").waitForExistence(timeout: 3) + let opened = card.label + attach(app, name: "resultAR-detailsOpen-AX5") + for _ in 0..<4 where !done.isHittable { app.swipeUp(velocity: .slow) } + XCTAssertTrue(done.isHittable, "with Details open, Done must be in reach after scrolling") + for _ in 0..<4 where !details.isHittable { app.swipeDown(velocity: .slow) } + + tap(app, "instruction.details") + XCTAssertTrue(element(app, "instruction.detail").waitForNonExistence(timeout: 5), "Details didn't close") + XCTAssertTrue(cardSaying("off screen").waitForNonExistence(timeout: 5), + "the off-screen note outlasted open Details: \(card.label)") + let refolded = app.descendants(matching: .any).matching(NSPredicate(format: "identifier == 'instruction' AND label == %@", folded)).firstMatch + XCTAssertTrue(refolded.waitForExistence(timeout: 5), "closed again, the card must read as before: \(card.label)") + XCTAssertFalse(cardSaying("off screen").waitForExistence(timeout: 2), "the off-screen note came back on its own") + XCTAssertTrue(done.isHittable, "closing Details must leave Done in reach") + attach(app, name: "resultAR-detailsClosedAgain-AX5") + + XCTAssertTrue(opened.contains("The camera view is below. Close Details or scroll."), "open Details covered the camera: \(opened)") + XCTAssertFalse(opened.contains("off screen"), "open Details must not give a direction to turn: \(opened)") + } + + /// Review of #241: an aim card's title follows the marker the overlay draws, and the overlay + /// draws within the camera the card leaves open. If a title changed the card's height, a + /// target standing at the card's edge could flip the marker, then the title, then the marker + /// back, for as long as the phone stayed there. At the largest text the titles wrap to + /// different numbers of lines, so that is where the card must keep one height + /// (`Instruction.titleReserve`). + /// + /// Two frozen aim states whose cards carry different titles must lay the card out alike: its + /// Details row starts at one place. (The `instruction` element's own frame covers only the + /// words shown, not the reserved room.) Then a stationary target at the largest text, where + /// the card reaches down to the ring, must keep one title and one marker for several seconds. + /// Without the reserve this state never draws a frame: the app's main thread spins between + /// the overlay and the card, and the first query times out. + @MainActor + func testTheAimCardKeepsItsHeightSoAStillTargetDoesNotFlip() throws { + continueAfterFailure = false + let app = XCUIApplication() + func card(_ arguments: [String]) -> (title: String, detailsTop: CGFloat) { + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "wallWalk"] + arguments + Self.largestText + app.launch() + let instruction = element(app, "instruction") + XCTAssertTrue(instruction.waitForExistence(timeout: 15)) + XCTAssertTrue(element(app, "instruction.details").waitForExistence(timeout: 5), "at the largest text an aim card folds its detail") + // Let the overlay report its marker and the card follow it. + Thread.sleep(forTimeInterval: 1.5) + let title = ElementRead.snapshot(instruction)?.label ?? "" + return (title, ElementRead.snapshot(element(app, "instruction.details"))?.frame.minY ?? -1) + } + // The ring or an under-card arrow ("Put the ring on the strip" or "Tilt up a little")... + let near = card(["-uiDemoAim", "-uiDemoAimHint", "above"]) + attach(app, name: "wallWalk-aimCard-near-AX5") + app.terminate() + // ...against a left edge arrow, which keeps the step's own title. Every live aim step + // has a hint, and with it the reserved titles. + let away = card(["-uiDemoAimOffScreen", "-uiDemoAimHint", "left"]) + attach(app, name: "wallWalk-aimCard-away-AX5") + app.terminate() + XCTAssertNotEqual(near.title, away.title, "the two states must show different titles for the check to mean anything") + XCTAssertEqual(near.detailsTop, away.detailsTop, accuracy: 0.5, "the aim card's height changed with its title: \(near) vs \(away)") + + // AX5 expanded Details fills the camera. At AX1 an arrow still fits beside the + // expanded card, so this checks an actual marker-driven title swap for one target. + // A left arrow becomes an above arrow after the camera moves; the engine stays left. + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "wallWalk", "-uiDemoAimOffScreen", + "-uiDemoAimHint", "left", "-uiDemoAimHintLater", "above", + "-UIPreferredContentSizeCategoryName", "UICTContentSizeCategoryAccessibilityM"] + app.launch() + let instruction = element(app, "instruction") + XCTAssertTrue(instruction.waitForExistence(timeout: 15)) + tap(app, "instruction.details") + XCTAssertTrue(element(app, "instruction.detail").waitForExistence(timeout: 5), "Details must open") + Thread.sleep(forTimeInterval: 1) + func sample() -> (title: String, line: String) { + let title = ElementRead.snapshot(instruction)?.label ?? "?" + let ring = element(app, "aim.ring").exists + let open = element(app, "instruction.detail").exists + let details = ElementRead.snapshot(element(app, "instruction.details"))?.frame.minY ?? -1 + return (title, "ring \(ring) | Details open \(open) at \(Int(details)) pt") + } + var before: [String] = [] + for _ in 0..<10 { + let (title, line) = sample() + before.append("\(title) | \(line)") + Thread.sleep(forTimeInterval: 0.4) + } + XCTAssertEqual(Set(before).count, 1, "a still target flipped with Details open: \(before)") + let held = sample() + XCTAssertTrue(element(app, "aim.arrow").label.contains("left"), "the baseline needs a drawn left arrow") + XCTAssertTrue(held.line.contains("Details open true"), "Details closed by itself: \(held)") + + // `DemoEngine.aimHintNotification`. + CFNotificationCenterPostNotification( + CFNotificationCenterGetDarwinNotifyCenter(), CFNotificationName("dev.housescanning.uiDemo.aimHintLater" as CFString), + nil, nil, true) + let swapped = app.descendants(matching: .any) + .matching(NSPredicate(format: "identifier == 'instruction' AND label != %@", held.title)).firstMatch + XCTAssertTrue(swapped.waitForExistence(timeout: 10), "the drawn marker's new direction never reached the card") + XCTAssertTrue(element(app, "aim.arrow").label.contains("above"), "the same target needs a drawn above arrow") + Thread.sleep(forTimeInterval: 1) + var after: [String] = [] + for _ in 0..<5 { + let (title, line) = sample() + after.append("\(title) | \(line)") + Thread.sleep(forTimeInterval: 0.4) + } + XCTAssertEqual(Set(after).count, 1, "the card kept changing after the swap: \(after)") + XCTAssertNotEqual(sample().title, held.title) + XCTAssertEqual(sample().line, held.line, "a swap between reserved titles closed Details or moved the card") + attach(app, name: "wallWalk-aimCard-detailsOpen-AX1") + } + + @MainActor + func testFoldedAimCardKeepsItsHeightAcrossMarkerTitlesAtAX5() throws { + continueAfterFailure = false + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoAimCardTitles"] + Self.largestText + app.launch() + let card = element(app, "demo.aimCard") + XCTAssertTrue(card.waitForExistence(timeout: 15)) + XCTAssertTrue(element(app, "instruction.details").waitForExistence(timeout: 5)) + let height = card.frame.height + XCTAssertGreaterThan(height, 0) + let titles = ["Put the ring on the strip", "Tilt up a little", "Tilt down to show the ground", "Put the ring on the strip"] + for (index, title) in titles.enumerated() { + let instruction = app.descendants(matching: .any) + .matching(NSPredicate(format: "identifier == 'instruction' AND label CONTAINS %@", title)).firstMatch + XCTAssertTrue(instruction.waitForExistence(timeout: 5), "expected marker title: \(title)") + XCTAssertEqual(card.frame.height, height, accuracy: 0.5, "folded card moved for \(title)") + if index < titles.count - 1 { tap(app, "demo.nextAimTitle") } + } + attach(app, name: "foldedAim-cardHeight-AX5") + } + + @MainActor + func testSecondPositionAdviceKeepsTheSameAimCardHeightAtAccessibilitySize() throws { + continueAfterFailure = false + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoAimCardTitles", "-uiDemoAimSecondPositionToggle", + "-UIPreferredContentSizeCategoryName", "UICTContentSizeCategoryAccessibilityM"] + app.launch() + defer { app.terminate() } + let card = element(app, "demo.aimCard") + XCTAssertTrue(card.waitForExistence(timeout: 15)) + XCTAssertTrue(element(app, "instruction.details").waitForExistence(timeout: 5)) + let height = card.frame.height + XCTAssertGreaterThan(height, 0) + for title in ["Step sideways, look again", "Put the ring on the strip", "Step sideways, look again"] { + tap(app, "demo.toggleAimPosition") + let instruction = app.descendants(matching: .any) + .matching(NSPredicate(format: "identifier == 'instruction' AND label CONTAINS %@", title)).firstMatch + XCTAssertTrue(instruction.waitForExistence(timeout: 5), "missing position advice: \(title)") + XCTAssertEqual(card.frame.height, height, accuracy: 0.5, "same-target card moved for \(title)") + } + attach(app, name: "foldedAim-secondPosition-cardHeight-AX1") + } + + /// A new target closes Details even when it shares the old target's reserved titles or + /// rounds to the same displayed distance. The notification advances the actual demo aim. + @MainActor + func testANewAimTargetClosesDetailsEvenWithReservedTitles() throws { + continueAfterFailure = false + for target in ["-2.4", "0.401"] { + let app = XCUIApplication() + app.launchArguments = ["-practiceMeter", "NO", "-uiDemo", "-uiDemoFreeze", "-uiDemoPhase", "wallWalk", "-uiDemoAim", + "-uiDemoAimHint", "above", "-uiDemoAimTargetLater", target] + Self.largestText + app.launch() + XCTAssertTrue(element(app, "instruction.details").waitForExistence(timeout: 15)) + tap(app, "instruction.details") + let detail = element(app, "instruction.detail") + XCTAssertTrue(detail.waitForExistence(timeout: 5), "Details must open before the task changes") + CFNotificationCenterPostNotification( + CFNotificationCenterGetDarwinNotifyCenter(), CFNotificationName("dev.housescanning.uiDemo.aimHintLater" as CFString), + nil, nil, true) + XCTAssertTrue(detail.waitForNonExistence(timeout: 5), "Details stayed open for the new aim at \(target)") + XCTAssertTrue(element(app, "instruction.details").exists, "the disclosure itself must remain available") + attach(app, name: "wallWalk-newAim-detailsClosed-\(target)-AX5") + app.terminate() + } + } + /// #81: the first aim ring comes with a line under the card saying what it is for, clear of /// the card at every text size, and reads its progress to VoiceOver. Off screen, the edge /// arrow stands in for the ring, and neither shows. @@ -827,7 +1306,9 @@ final class ScreenStatesUITests: XCTestCase { XCTAssertTrue(reply.isHittable, "the card's reply must be reachable without scrolling") let cardBottom = max(card.frame.maxY, details.frame.maxY, reply.frame.maxY) let openCamera = CGRect(x: window.minX, y: cardBottom, width: window.width, height: mark.frame.minY - cardBottom) - XCTAssertGreaterThanOrEqual(openCamera.height, 150, "the camera must stay open between the card and the actions: \(openCamera)") + let visibleCamera = openCamera.intersection(window) + print("AX5 aim: cardBottom=\(cardBottom), openCamera=\(openCamera), visibleCamera=\(visibleCamera)") + XCTAssertGreaterThanOrEqual(visibleCamera.height, 150, "the camera must stay visibly open between the card and the actions: \(visibleCamera)") let ring = element(app, "aim.ring") let arrow = element(app, "aim.arrow")