From 58e1a0212d0ef048106be46d5804f182b7e6e2b1 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 3 Aug 2026 22:55:03 +0300 Subject: [PATCH] Indent blank lines and address avatar crop review feedback Indent the new method's blank lines to satisfy SwiftFormat's indent-blank-lines rule, which failed the Tests CI jobs. Also address review findings on the crop itself: - Request a 256pt thumbnail box at both donation sites so cropping a scaled (aspect-fit) thumbnail to its minor axis keeps enough resolution for the share sheet's circle. - Round the draw offsets to whole pixels so odd-sized crops blit instead of resampling. - Log when fetched avatar data can't be decoded rather than silently swapping in the placeholder, and document the non-obvious format.scale pinning. --- ElementX/Sources/Other/Avatars.swift | 17 +++++++++++------ .../TimelineController/TimelineController.swift | 11 ++++++++--- NSE/Sources/NotificationContentBuilder.swift | 5 ++++- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/ElementX/Sources/Other/Avatars.swift b/ElementX/Sources/Other/Avatars.swift index 39f4857d8..f90e3cdfc 100644 --- a/ElementX/Sources/Other/Avatars.swift +++ b/ElementX/Sources/Other/Avatars.swift @@ -44,23 +44,28 @@ enum Avatars { /// images to fill its circle, so centre-crop to a square before donating them. nonisolated static func squareAvatarImageData(from data: Data) -> Data? { guard let image = UIImage(data: data) else { return nil } - + let side = min(image.size.width, image.size.height) guard side > 0 else { return nil } - + if image.size.width == image.size.height { return data } - + let format = UIGraphicsImageRendererFormat() + // UIImage(data:) decodes at scale 1; pin the renderer to that so the + // output isn't silently multiplied by the device's screen scale. format.scale = image.scale - + return UIGraphicsImageRenderer(size: CGSize(width: side, height: side), format: format) .pngData { _ in - image.draw(at: CGPoint(x: (side - image.size.width) / 2, y: (side - image.size.height) / 2)) + // Round the offsets to pixel boundaries so the crop is a straight + // copy rather than resampling (and softening) the whole image. + image.draw(at: CGPoint(x: ((side - image.size.width) / 2).rounded(), + y: ((side - image.size.height) / 2).rounded())) } } - + @MainActor static func generatePlaceholderAvatarImageData(name: String, id: String, size: CGSize) -> Data? { let image = PlaceholderAvatarImage(name: name, contentID: id) diff --git a/ElementX/Sources/Services/Timeline/TimelineController/TimelineController.swift b/ElementX/Sources/Services/Timeline/TimelineController/TimelineController.swift index c22dc19ae..7e6058515 100644 --- a/ElementX/Sources/Services/Timeline/TimelineController/TimelineController.swift +++ b/ElementX/Sources/Services/Timeline/TimelineController/TimelineController.swift @@ -650,9 +650,14 @@ class TimelineController: TimelineControllerProtocol { } if let avatarURL, let mediaSource = try? MediaSourceProxy(url: avatarURL, mimeType: nil) { - if case let .success(avatarData) = await mediaProvider.loadThumbnailForSource(source: mediaSource, size: .init(width: 100, height: 100)), - let squareAvatarData = Avatars.squareAvatarImageData(from: avatarData) { - sendMessageIntent.setImage(INImage(imageData: squareAvatarData), forParameterNamed: \.speakableGroupName) + // Request a generous size so the square crop of a non-square avatar keeps enough resolution. + if case let .success(avatarData) = await mediaProvider.loadThumbnailForSource(source: mediaSource, size: .init(width: 256, height: 256)) { + if let squareAvatarData = Avatars.squareAvatarImageData(from: avatarData) { + sendMessageIntent.setImage(INImage(imageData: squareAvatarData), forParameterNamed: \.speakableGroupName) + } else { + MXLog.error("Failed processing the room avatar for the send message intent, using a placeholder.") + addPlacehoder() + } } else { addPlacehoder() } diff --git a/NSE/Sources/NotificationContentBuilder.swift b/NSE/Sources/NotificationContentBuilder.swift index 7bb218394..dc0b79e40 100644 --- a/NSE/Sources/NotificationContentBuilder.swift +++ b/NSE/Sources/NotificationContentBuilder.swift @@ -257,10 +257,13 @@ nonisolated struct NotificationContentBuilder { let image: INImage if !forcePlaceholder, let mediaSource = icon.mediaSource { - switch await mediaProvider.loadThumbnailForSource(source: mediaSource, size: .init(width: 100, height: 100)) { + // Request a generous size so the square crop of a non-square avatar keeps enough resolution. + switch await mediaProvider.loadThumbnailForSource(source: mediaSource, size: .init(width: 256, height: 256)) { case .success(let data): if let squareData = Avatars.squareAvatarImageData(from: data) { fetchedImage = INImage(imageData: squareData) + } else { + MXLog.error("Couldn't decode the fetched sender icon, using a placeholder.") } case .failure(let error): MXLog.error("Couldn't add sender icon: \(error)")