Improve image generation prints

Sources/Generator/Files/FileSystem.swift

@@ -6,36 +6,18 @@ final class FileSystem {
 
     private static let tempFileName = "temp.bin"
 
-    private static let hashesFileName = "hashes.json"
-
     private let input: URL
 
     private let output: URL
 
-    private let source = "FileChangeMonitor"
+    private let source = "FileSystem"
 
-    private var hashesFile: URL {
+    private let images: ImageGenerator
-        input.appendingPathComponent(FileSystem.hashesFileName)
-    }
 
     private var tempFile: URL {
         input.appendingPathComponent(FileSystem.tempFileName)
     }
 
-    /**
-     The hashes of all accessed files from the previous run
-
-     The key is the relative path to the file from the source
-     */
-    private var previousFiles: [String : Data] = [:]
-
-    /**
-     The paths of all files which were accessed, with their new hashes
-
-     This list is used to check if a file was modified, and to write all accessed files back to disk
-     */
-    private var accessedFiles: [String : Data] = [:]
-
     /**
      All files which should be copied to the output folder
      */
@@ -87,30 +69,8 @@ final class FileSystem {
     init(in input: URL, to output: URL) {
         self.input = input
         self.output = output
+        self.images = .init(input: input, output: output)
 
-        guard exists(hashesFile) else {
-            log.add(info: "No file hashes loaded, regarding all content as new", source: source)
-            return
-        }
-        let data: Data
-        do {
-            data = try Data(contentsOf: hashesFile)
-        } catch {
-            log.add(
-                warning: "File hashes could not be read, regarding all content as new",
-                source: source,
-                error: error)
-            return
-        }
-        do {
-            self.previousFiles = try JSONDecoder().decode(from: data)
-        } catch {
-            log.add(
-                warning: "File hashes could not be decoded, regarding all content as new",
-                source: source,
-                error: error)
-            return
-        }
     }
 
     func urlInOutputFolder(_ path: String) -> URL {
@@ -121,15 +81,6 @@ final class FileSystem {
         input.appendingPathComponent(path)
     }
 
-    /**
-     Get the current hash of file data at a path.
-
-     If the hash has been computed previously during the current run, then this function directly returns it.
-     */
-    private func hash(_ data: Data, at path: String) -> Data {
-        accessedFiles[path] ?? SHA256.hash(data: data).data
-    }
-
     private func exists(_ url: URL) -> Bool {
         FileManager.default.fileExists(atPath: url.path)
     }
@@ -180,178 +131,19 @@ final class FileSystem {
         }
     }
 
-    private func getData(atPath path: String) -> (data: Data, didChange: Bool)? {
+    func writeDetectedFileChangesToDisk() {
-        let url = input.appendingPathComponent(path)
+        images.writeDetectedFileChangesToDisk()
-        guard exists(url) else {
-            return nil
-        }
-        let data: Data
-        do {
-            data = try Data(contentsOf: url)
-        } catch {
-            log.add(error: "Failed to read data at \(path)", source: source, error: error)
-            return nil
-        }
-        let newHash = hash(data, at: path)
-        defer {
-            accessedFiles[path] = newHash
-        }
-        guard let oldHash = previousFiles[path] else {
-            return (data: data, didChange: true)
-        }
-        return (data: data, didChange: oldHash != newHash)
     }
-
+    
-    func writeHashes() {
-        do {
-            let encoder = JSONEncoder()
-            encoder.outputFormatting = .prettyPrinted
-            let data = try encoder.encode(accessedFiles)
-            try data.write(to: hashesFile)
-        } catch {
-            log.add(warning: "Failed to save file hashes", source: source, error: error)
-        }
-    }
-
     // MARK: Images
 
-    private func loadImage(atPath path: String) -> (image: NSImage, changed: Bool)? {
-        guard let (data, changed) = getData(atPath: path) else {
-            log.add(error: "Failed to load file", source: path)
-            return nil
-        }
-        guard let image = NSImage(data: data) else {
-            log.add(error: "Failed to read image", source: path)
-            return nil
-        }
-        return (image, changed)
-    }
-
     @discardableResult
-        let height = desiredHeight.unwrapped(CGFloat.init)
-        let sourceUrl = input.appendingPathComponent(source)
-        let image = ImageOutput(source: source, width: width, desiredHeight: desiredHeight)
-
-        let standardSize = NSSize(width: CGFloat(width), height: height ?? CGFloat(width) / 16 * 9)
-        guard sourceUrl.exists else {
-            log.add(error: "Missing file with size (\(width),\(desiredHeight ?? -1))",
-                    source: source)
-            return standardSize
-        }
-        guard let imageSize = loadImage(atPath: image.source)?.image.size else {
-            log.add(error: "Unreadable image with size (\(width),\(desiredHeight ?? -1))",
-                    source: source)
-            return standardSize
-        }
-        let scaledSize = imageSize.scaledDown(to: CGFloat(width))
-
-        guard let existing = imageTasks[destination] else {
-            imageTasks[destination] = image
-            return scaledSize
-        }
-        guard existing.source == source else {
-            log.add(error: "Multiple sources (\(existing.source),\(source))",
-                    source: destination)
-            return scaledSize
-        }
-        guard existing.hasSimilarRatio(as: image) else {
-            log.add(error: "Multiple ratios (\(existing.ratio!),\(image.ratio!))",
-                    source: destination)
-            return scaledSize
-        }
-        if image.width > existing.width {
-            log.add(info: "Increasing size from \(existing.width) to \(width)",
-                    source: destination)
-            imageTasks[destination] = image
-        }
-        return scaledSize
     func requireImage(source: String, destination: String, requiredBy path: String, width: Int, desiredHeight: Int? = nil)  -> NSSize {
+        images.requireImage(at: destination, generatedFrom: source, requiredBy: path, width: width, height: desiredHeight)
     }
 
     func createImages() {
-        for (destination, image) in imageTasks.sorted(by: { $0.key < $1.key }) {
+        images.createImages()
-            createImageIfNeeded(image, for: destination)
-        }
-    }
-
-    private func createImageIfNeeded(_ image: ImageOutput, for destination: String) {
-        guard let (sourceImageData, sourceImageChanged) = getData(atPath: image.source) else {
-            log.add(error: "Failed to open file", source: image.source)
-            return
-        }
-
-        let destinationUrl = output.appendingPathComponent(destination)
-
-        // Check if image needs to be updated
-        guard !destinationUrl.exists || sourceImageChanged else {
-            return
-        }
-
-        // Ensure that image file is supported
-        let ext = destinationUrl.pathExtension.lowercased()
-        guard ImageType(fileExtension: ext) != nil else {
-            // TODO: This should never be reached, since extensions are checked before
-            log.add(info: "Copying image", source: image.source)
-            do {
-                let sourceUrl = input.appendingPathComponent(image.source)
-                try destinationUrl.ensureParentFolderExistence()
-                try sourceUrl.copy(to: destinationUrl)
-            } catch {
-                log.add(error: "Failed to copy image", source: destination)
-            }
-            return
-        }
-        guard let sourceImage = NSImage(data: sourceImageData) else {
-            log.add(error: "Failed to read file", source: image.source)
-            return
-        }
-
-        let desiredWidth = CGFloat(image.width)
-        let desiredHeight = image.desiredHeight.unwrapped(CGFloat.init)
-
-        let destinationSize = sourceImage.size.scaledDown(to: desiredWidth)
-        let scaledImage = sourceImage.scaledDown(to: destinationSize)
-        let scaledSize = scaledImage.size
-
-        if abs(scaledSize.width - desiredWidth) > 2 {
-            log.add(warning: "Desired width \(desiredWidth), got \(scaledSize.width)", source: destination)
-        }
-        if abs(destinationSize.height - scaledImage.size.height) > 2 {
-            log.add(warning: "Desired height \(destinationSize.height), got \(scaledSize.height)", source: destination)
-        }
-        if let desiredHeight = desiredHeight {
-            let desiredRatio = desiredHeight / desiredWidth
-            let adjustedDesiredHeight = scaledSize.width * desiredRatio
-            if abs(adjustedDesiredHeight - scaledSize.height) > 5 {
-                log.add(warning: "Desired height \(desiredHeight), got \(scaledSize.height)", source: destination)
-                return
-            }
-        }
-        if scaledSize.width > desiredWidth {
-            log.add(warning:" Desired width \(desiredWidth), got \(scaledSize.width)", source: destination)
-        }
-
-        let destinationExtension = destinationUrl.pathExtension.lowercased()
-        guard let type = ImageType(fileExtension: destinationExtension)?.fileType else {
-            log.add(error: "No image type for extension \(destinationExtension)",
-                    source: destination)
-            return
-        }
-        guard let tiff = scaledImage.tiffRepresentation, let tiffData = NSBitmapImageRep(data: tiff) else {
-            log.add(error: "Failed to get data", source: image.source)
-            return
-        }
-
-        guard let data = tiffData.representation(using: type, properties: [.compressionFactor: NSNumber(0.7)]) else {
-            log.add(error: "Failed to get data", source: image.source)
-            return
-        }
-        do {
-            try data.createFolderAndWrite(to: destinationUrl)
-        } catch {
-            log.add(error: "Failed to write image \(destination)", source: "Image Processor", error: error)
-            return
-        }
     }
 
     // MARK: File copying

Sources/Generator/Files/FileUpdateChecker.swift

@@ -0,0 +1,88 @@
+import Foundation
+import CryptoKit
+
+final class FileUpdateChecker {
+
+    private static let hashesFileName = "hashes.json"
+
+    private let input: URL
+
+    private var hashesFile: URL {
+        input.appendingPathComponent(FileUpdateChecker.hashesFileName)
+    }
+    
+    /**
+     The hashes of all accessed files from the previous run
+
+     The key is the relative path to the file from the source
+     */
+    private var previousFiles: [String : Data] = [:]
+
+    /**
+     The paths of all files which were accessed, with their new hashes
+
+     This list is used to check if a file was modified, and to write all accessed files back to disk
+     */
+    private var accessedFiles: [String : Data] = [:]
+
+    private var source: String {
+        "FileUpdateChecker"
+    }
+
+    init(input: URL) {
+        self.input = input
+        guard hashesFile.exists else {
+            log.add(info: "No file hashes loaded, regarding all content as new", source: source)
+            return
+        }
+        let data: Data
+        do {
+            data = try Data(contentsOf: hashesFile)
+        } catch {
+            log.add(
+                warning: "File hashes could not be read, regarding all content as new",
+                source: source,
+                error: error)
+            return
+        }
+        do {
+            self.previousFiles = try JSONDecoder().decode(from: data)
+        } catch {
+            log.add(
+                warning: "File hashes could not be decoded, regarding all content as new",
+                source: source,
+                error: error)
+            return
+        }
+    }
+
+    func fileHasChanged(at path: String) -> Bool {
+        guard let oldHash = previousFiles[path] else {
+            // Image wasn't used last time, so treat as new
+            return true
+        }
+        guard let newHash = accessedFiles[path] else {
+            // Each image should have been loaded once
+            // before using this function
+            fatalError()
+        }
+        return oldHash != newHash
+    }
+
+    func didLoad(_ data: Data, at path: String) {
+        accessedFiles[path] = SHA256.hash(data: data).data
+    }
+
+
+    func writeDetectedFileChangesToDisk() {
+        do {
+            let encoder = JSONEncoder()
+            encoder.outputFormatting = .prettyPrinted
+            let data = try encoder.encode(accessedFiles)
+            try data.write(to: hashesFile)
+        } catch {
+            log.add(warning: "Failed to save file hashes", source: source, error: error)
+        }
+    }
+
+}

Sources/Generator/Files/ImageGenerator.swift

@@ -0,0 +1,262 @@
+import Foundation
+import AppKit
+import CryptoKit
+
+private struct ImageJob {
+
+    let destination: String
+
+    let width: Int
+
+    let path: String
+}
+
+final class ImageGenerator {
+
+    /**
+     The path to the input folder.
+     */
+    private let input: URL
+
+    /**
+     The path to the output folder
+     */
+    private let output: URL
+
+    /**
+     The images to generate.
+
+     The key is the image source path relative to the input folder, and the values are the destination path (relative to the output folder) and the required image width.
+     */
+    private var imageJobs: [String : [ImageJob]] = [:]
+
+    /**
+     The images which could not be found, but are required for the site.
+
+     The key is the image path, and the value is the page that requires it.
+     */
+    private var missingImages: [String : String] = [:]
+
+    /**
+     All warnings produced for images during generation
+     */
+    private var imageWarnings: Set<String> = []
+
+    /**
+     All images required by the site.
+
+     The values are the destination paths of the images, relative to the output folder
+     */
+    private var requiredImages: Set<String> = []
+
+    /**
+     All images modified or created during this generator run.
+     */
+    private var generatedImages: Set<String> = []
+
+    /**
+     A cache to get the size of source images, so that files don't have to be loaded multiple times.
+
+     The key is the absolute source path, and the value is the image size
+     */
+    private var imageSizeCache: [String : NSSize] = [:]
+
+    private var fileUpdates: FileUpdateChecker
+
+    init(input: URL, output: URL) {
+        self.fileUpdates = FileUpdateChecker(input: input)
+        self.input = input
+        self.output = output
+    }
+
+    func writeDetectedFileChangesToDisk() {
+        fileUpdates.writeDetectedFileChangesToDisk()
+    }
+
+    private func getImageSize(atPath path: String) -> NSSize? {
+        if let size = imageSizeCache[path] {
+            return size
+        }
+        guard let image = getImage(atPath: path) else {
+            return nil
+        }
+        let size = image.size
+        imageSizeCache[path] = size
+        return size
+    }
+
+    private func getImage(atPath path: String) -> NSImage? {
+        guard let data = getData(atPath: path) else {
+            log.add(error: "Failed to load file", source: path)
+            return nil
+        }
+        guard let image = NSImage(data: data) else {
+            log.add(error: "Failed to read image", source: path)
+            return nil
+        }
+        return image
+    }
+
+    private func getData(atPath path: String) -> Data? {
+        let url = input.appendingPathComponent(path)
+        guard url.exists else {
+            return nil
+        }
+        do {
+            let data = try Data(contentsOf: url)
+            fileUpdates.didLoad(data, at: path)
+            return data
+        } catch {
+            log.add(error: "Failed to read data", source: path, error: error)
+            return nil
+        }
+    }
+
+    func requireImage(at destination: String, generatedFrom source: String, requiredBy path: String, width: Int, height: Int?) -> NSSize {
+        let height = height.unwrapped(CGFloat.init)
+        let sourceUrl = input.appendingPathComponent(source)
+        guard sourceUrl.exists else {
+            missingImages[source] = path
+            return .zero
+        }
+
+        guard let imageSize = getImageSize(atPath: source) else {
+            missingImages[source] = path
+            return .zero
+        }
+        let scaledSize = imageSize.scaledDown(to: CGFloat(width))
+
+        // Check desired height, then we can forget about it
+        if let height = height {
+            let expectedHeight = scaledSize.width / CGFloat(width) * height
+            if abs(expectedHeight - scaledSize.height) > 2 {
+                addWarning("Invalid height (\(scaledSize.height) instead of \(expectedHeight))", destination: destination, path: path)
+            }
+        }
+
+        let job = ImageJob(destination: destination, width: width, path: path)
+
+        guard let existingSource = imageJobs[source] else {
+            imageJobs[source] = [job]
+            return scaledSize
+        }
+
+        guard let existingJob = existingSource.first(where: { $0.destination == destination}) else {
+            imageJobs[source] = existingSource + [job]
+            return scaledSize
+        }
+
+        if existingJob.width != width {
+            addWarning("Multiple image widths (\(existingJob.width) and \(width))", destination: destination, path: "\(existingJob.path) and \(path)")
+        }
+        return scaledSize
+    }
+
+    func createImages() {
+        for (source, jobs) in imageJobs.sorted(by: { $0.key < $1.key }) {
+            create(images: jobs, from: source)
+        }
+        printMissingImages()
+        printImageWarnings()
+    }
+
+    private func printMissingImages() {
+        guard !missingImages.isEmpty else {
+            return
+        }
+        print("\(missingImages.count) missing images:")
+        for (source, path) in missingImages {
+            print("  \(source) (required by \(path))")
+        }
+    }
+
+    private func printImageWarnings() {
+        guard !imageWarnings.isEmpty else {
+            return
+        }
+        print("\(imageWarnings.count) image warnings:")
+        for imageWarning in imageWarnings {
+            print(imageWarning)
+        }
+    }
+
+    private func addWarning(_ message: String, destination: String, path: String) {
+        let warning = "  \(destination): \(message) required by \(path)"
+        imageWarnings.insert(warning)
+    }
+
+    private func addWarning(_ message: String, job: ImageJob) {
+        addWarning(message, destination: job.destination, path: job.path)
+    }
+
+    private func isMissing(_ job: ImageJob) -> Bool {
+        !output.appendingPathComponent(job.destination).exists
+    }
+
+    private func create(images: [ImageJob], from source: String) {
+        // Only load image if required
+        let imageHasChanged = fileUpdates.fileHasChanged(at: source)
+        guard imageHasChanged || images.contains(where: isMissing) else {
+            return
+        }
+
+        guard let image = getImage(atPath: source) else {
+            missingImages[source] = images.first?.path
+            return
+        }
+        if imageHasChanged {
+            // Update all images
+            images.forEach { create(job: $0, from: image, source: source) }
+        } else {
+            // Update only missing images
+            images
+                .filter(isMissing)
+                .forEach { create(job: $0, from: image, source: source) }
+        }
+    }
+
+    private func create(job: ImageJob, from image: NSImage, source: String) {
+        let destinationUrl = output.appendingPathComponent(job.destination)
+
+        // Ensure that image file is supported
+        let ext = destinationUrl.pathExtension.lowercased()
+        guard ImageType(fileExtension: ext) != nil else {
+            fatalError()
+        }
+
+        let desiredWidth = CGFloat(image.size.width)
+
+        let destinationSize = image.size.scaledDown(to: desiredWidth)
+        let scaledImage = image.scaledDown(to: destinationSize)
+        let scaledSize = scaledImage.size
+
+        if abs(scaledSize.width - desiredWidth) > 2 {
+            addWarning("Invalid width (\(scaledSize.width) instead of \(desiredWidth))", job: job)
+        }
+
+        if scaledSize.width > desiredWidth {
+            addWarning("Invalid width (\(scaledSize.width) instead of \(desiredWidth))", job: job)
+        }
+
+        let destinationExtension = destinationUrl.pathExtension.lowercased()
+        guard let type = ImageType(fileExtension: destinationExtension)?.fileType else {
+            addWarning("Invalid image extension \(destinationExtension)", job: job)
+            return
+        }
+        guard let tiff = scaledImage.tiffRepresentation, let tiffData = NSBitmapImageRep(data: tiff) else {
+            addWarning("Failed to get data", job: job)
+            return
+        }
+
+        guard let data = tiffData.representation(using: type, properties: [.compressionFactor: NSNumber(0.7)]) else {
+            addWarning("Failed to get data", job: job)
+            return
+        }
+        do {
+            try data.createFolderAndWrite(to: destinationUrl)
+        } catch {
+            addWarning("Failed to write image (\(error))", job: job)
+            return
+        }
+    }
+}

Sources/Generator/run.swift

@@ -36,8 +36,7 @@ private func generate(configPath: String) throws {
     files.printDraftPages()
 
     files.createImages()
-    print("Images generated")
     files.copyRequiredFiles()
     files.printExternalFiles()
-    files.writeHashes()
+    files.writeDetectedFileChangesToDisk()
 }