Enhance word frame processor to not have overlaps

Domain/ImageService/Sources/ImageDataService.swift

@@ -44,9 +44,16 @@ public struct ImageDataService {
         let unloadedImage: UIImage = image
         let preloadedImage = preloadImage(unloadedImage, cropInsets: cropInsets)
 
+        let wordFrames = try await wordFrames(page)
+        return ImagePage(image: preloadedImage, wordFrames: wordFrames, startAyah: page.firstVerse)
+    }
+
+    // MARK: Internal
+
+    func wordFrames(_ page: Page) async throws -> WordFrameCollection {
         let plainWordFrames = try await persistence.wordFrameCollectionForPage(page)
         let wordFrames = processor.processWordFrames(plainWordFrames, cropInsets: cropInsets)
-        return ImagePage(image: preloadedImage, wordFrames: wordFrames, startAyah: page.firstVerse)
+        return wordFrames
     }
 
     // MARK: Private

Domain/ImageService/Tests/WordFrameTests.swift

@@ -0,0 +1,47 @@
+//
+//  WordFrameTests.swift
+//
+//
+//  Created by Mohamed Afifi on 2024-04-05.
+//
+
+import QuranGeometry
+import QuranKit
+import XCTest
+@testable import WordFrameService
+
+@MainActor
+class WordFrameTests: XCTestCase {
+    let word1 = Word(verse: Quran.hafsMadani1405.firstVerse, wordNumber: 1)
+    let word2 = Word(verse: Quran.hafsMadani1405.firstVerse, wordNumber: 2)
+
+    func testNonOverlappingFrames() {
+        // Case 1: Non-overlapping frames
+        // Before: [leftFrame]     [rightFrame]
+        var leftFrame = WordFrame(line: 1, word: word1, minX: 0, maxX: 10, minY: 0, maxY: 10)
+        var rightFrame = WordFrame(line: 1, word: word2, minX: 20, maxX: 30, minY: 0, maxY: 10)
+        WordFrame.unionHorizontally(leftFrame: &leftFrame, rightFrame: &rightFrame)
+        // After:  [leftFrame][rightFrame]
+
+        assertFrame(leftFrame, minX: 0, maxX: 15)
+        assertFrame(rightFrame, minX: 15, maxX: 30)
+    }
+
+    func testOverlappingFrames() {
+        // Case 2: Overlapping frames with non-overlapping parts
+        // Before: [leftFrame overlaps]
+        //                   [overlaps rightFrame]
+        var leftFrame = WordFrame(line: 1, word: word1, minX: 0, maxX: 20, minY: 0, maxY: 10)
+        var rightFrame = WordFrame(line: 1, word: word2, minX: 15, maxX: 35, minY: 0, maxY: 10)
+        WordFrame.unionHorizontally(leftFrame: &leftFrame, rightFrame: &rightFrame)
+        // After:  [leftFrame][rightFrame]
+
+        assertFrame(leftFrame, minX: 0, maxX: 15)
+        assertFrame(rightFrame, minX: 15, maxX: 35)
+    }
+}
+
+func assertFrame(_ frame: WordFrame, minX: Int, maxX: Int, file: StaticString = #filePath, line: UInt = #line) {
+    XCTAssertEqual(frame.minX, minX, "minX")
+    XCTAssertEqual(frame.maxX, maxX, "minX")
+}

Domain/ImageService/Tests/__Snapshots__/ImageDataServiceTests/testGettingImageAtPage3.2.json

@@ -211,11 +211,11 @@
   {
     "frame" : [
       [
-        853,
+        852,
         143
       ],
       [
-        104,
+        105,
         119
       ]
     ],
@@ -234,7 +234,7 @@
         143
       ],
       [
-        112,
+        111,
         119
       ]
     ],

Domain/ImageService/Tests/__Snapshots__/ImageDataServiceTests/testGettingImageAtPage604.2.json

@@ -477,11 +477,11 @@
   {
     "frame" : [
       [
-        439,
+        438,
         812
       ],
       [
-        79,
+        80,
         124
       ]
     ],
@@ -500,7 +500,7 @@
         812
       ],
       [
-        39,
+        38,
         124
       ]
     ],
@@ -800,11 +800,11 @@
   {
     "frame" : [
       [
-        584,
+        581,
         1071
       ],
       [
-        105,
+        108,
         117
       ]
     ],
@@ -823,7 +823,7 @@
         1071
       ],
       [
-        140,
+        137,
         117
       ]
     ],

Domain/WordFrameService/WordFrameProcessor.swift

@@ -21,16 +21,18 @@ public struct WordFrameProcessor {
         _ wordFrames: [WordFrame],
         cropInsets: UIEdgeInsets
     ) -> WordFrameCollection {
+        guard !wordFrames.isEmpty else {
+            return WordFrameCollection(lines: [])
+        }
         let frames = wordFrames.map { $0.withCropInsets(cropInsets) }
 
         // group by line
         let framesByLines = Dictionary(grouping: frames, by: { $0.line })
         var sortedLines = framesByLines
             .sorted { $0.key < $1.key }
-            .map { line, wordFrames in
-                wordFrames.sorted { $0.word < $1.word }
-            }
+            .map { _, wordFrames in wordFrames }
 
+        normalize(&sortedLines)
         alignFramesVerticallyInEachLine(&sortedLines)
         unionLinesVertically(&sortedLines)
         unionFramesHorizontallyInEachLine(&sortedLines)
@@ -41,6 +43,14 @@ public struct WordFrameProcessor {
 
     // MARK: Private
 
+    private func normalize(_ lines: inout [[WordFrame]]) {
+        for i in 0 ..< lines.count {
+            for j in 0 ..< lines[i].count {
+                lines[i][j].normalize()
+            }
+        }
+    }
+
     private func alignFramesVerticallyInEachLine(_ lines: inout [[WordFrame]]) {
         // align vertically each line
         for i in 0 ..< lines.count {
@@ -64,18 +74,20 @@ public struct WordFrameProcessor {
     }
 
     private func unionFramesHorizontallyInEachLine(_ lines: inout [[WordFrame]]) {
-        // union each position with its neighbors
-        for i in 0 ..< lines.count {
-            for j in 0 ..< lines[i].count - 1 {
-                // Create temporary copies
-                var left = lines[i][j]
-                var right = lines[i][j + 1]
+        for lineIndex in 0 ..< lines.count {
+            // Sort frames in the current line based on minX to ensure they are in decreasing order
+            lines[lineIndex].sort(by: { $0.minX > $1.minX })
+
+            for frameIndex in 0 ..< lines[lineIndex].count - 1 {
+                var leftFrame = lines[lineIndex][frameIndex + 1]
+                var rightFrame = lines[lineIndex][frameIndex + 0]
 
-                left.unionHorizontally(left: &right)
+                // Ensure the frames touch each other without gaps or overlaps
+                WordFrame.unionHorizontally(leftFrame: &leftFrame, rightFrame: &rightFrame)
 
-                // Assign the modified copies back to the original array
-                lines[i][j] = left
-                lines[i][j + 1] = right
+                // Update the frames in the current line
+                lines[lineIndex][frameIndex + 1] = leftFrame
+                lines[lineIndex][frameIndex + 0] = rightFrame
             }
         }
     }

Domain/WordFrameService/WordFrameScale+Extension.swift

@@ -8,6 +8,7 @@
 import QuranGeometry
 import QuranKit
 import UIKit
+import VLogging
 
 extension WordFrameCollection {
     public func wordAtLocation(_ location: CGPoint, imageScale: WordFrameScale) -> Word? {
@@ -23,9 +24,21 @@ extension WordFrameCollection {
 }
 
 extension WordFrame {
+    mutating func normalize() {
+        // Ensure minX is less than or equal to maxX
+        if minX > maxX {
+            swap(&minX, &maxX)
+        }
+
+        // Ensure minY is less than or equal to maxY
+        if minY > maxY {
+            swap(&minY, &maxY)
+        }
+    }
+
     static func alignedVertically(_ list: [WordFrame]) -> [WordFrame] {
-        let minY = list.map(\.minY).min()!
-        let maxY = list.map(\.maxY).max()!
+        let minY = list.map(\.minY).min() ?? 0
+        let maxY = list.map(\.maxY).max() ?? 0
         var result: [WordFrame] = []
         for var frame in list {
             frame.minY = minY
@@ -35,32 +48,39 @@ extension WordFrame {
         return result
     }
 
-    mutating func unionHorizontally(left: inout WordFrame) {
-        let distance = Int(ceil((CGFloat(minX) - CGFloat(left.maxX)) / 2))
-        left.maxX += distance
-        minX -= distance
-        left.maxX = minX
+    static func unionHorizontally(leftFrame: inout WordFrame, rightFrame: inout WordFrame) {
+        if leftFrame.maxX < rightFrame.minX {
+            // If there's a gap, middleX is halfway between the left frame's maxX and the right frame's minX
+            let middleX = (leftFrame.maxX + rightFrame.minX) / 2
+            rightFrame.minX = middleX
+            leftFrame.maxX = middleX
+        } else {
+            // If there's an overlap or the frames are touching, leftFrame.maxX is set to rightFrame.minX
+            leftFrame.maxX = rightFrame.minX
+        }
     }
 
+    /// Adjusts the top and bottom arrays of WordFrame instances to meet vertically with an equal gap between them,
+    /// but only if they belong to the same sura.
+    ///
+    /// - Parameters:
+    ///   - top: An array of WordFrame instances representing the top line, to be adjusted downwards.
+    ///   - bottom: An array of WordFrame instances representing the bottom line, to be adjusted upwards.
     static func unionVertically(top: inout [WordFrame], bottom: inout [WordFrame]) {
-        // If not continuous lines (different suras).
-        guard top.last!.word.verse.sura == bottom.first!.word.verse.sura else {
+        // Early return if not continuous lines (different suras).
+        guard top.last?.word.verse.sura == bottom.first?.word.verse.sura else {
             return
         }
 
-        var topMaxY = top.map(\.maxY).max()!
-        var bottomMinY = bottom.map(\.minY).min()!
-
-        let distance = Int(ceil((CGFloat(bottomMinY) - CGFloat(topMaxY)) / 2))
-        topMaxY += distance
-        bottomMinY -= distance
-        topMaxY = bottomMinY
+        let topMaxY = top.map(\.maxY).max() ?? 0
+        let bottomMinY = bottom.map(\.minY).min() ?? 0
+        let middleY = (topMaxY + bottomMinY) / 2
 
         for i in 0 ..< top.count {
-            top[i].maxY = topMaxY
+            top[i].maxY = middleY
         }
         for i in 0 ..< bottom.count {
-            bottom[i].minY = bottomMinY
+            bottom[i].minY = middleY
         }
     }