From d9fc36676767cf9314e982de502b423eb5641c51 Mon Sep 17 00:00:00 2001 From: Mohamed Afifi Date: Sat, 6 Apr 2024 19:37:32 -0400 Subject: [PATCH] Enhance word frame processor to not have overlaps --- .../Sources/ImageDataService.swift | 9 ++- .../ImageService/Tests/WordFrameTests.swift | 47 ++++++++++++++++ .../testGettingImageAtPage3.2.json | 6 +- .../testGettingImageAtPage604.2.json | 12 ++-- .../WordFrameService/WordFrameProcessor.swift | 38 ++++++++----- .../WordFrameScale+Extension.swift | 56 +++++++++++++------ 6 files changed, 127 insertions(+), 41 deletions(-) create mode 100644 Domain/ImageService/Tests/WordFrameTests.swift diff --git a/Domain/ImageService/Sources/ImageDataService.swift b/Domain/ImageService/Sources/ImageDataService.swift index fc71545d..17655b79 100644 --- a/Domain/ImageService/Sources/ImageDataService.swift +++ b/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 diff --git a/Domain/ImageService/Tests/WordFrameTests.swift b/Domain/ImageService/Tests/WordFrameTests.swift new file mode 100644 index 00000000..9839522a --- /dev/null +++ b/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") +} diff --git a/Domain/ImageService/Tests/__Snapshots__/ImageDataServiceTests/testGettingImageAtPage3.2.json b/Domain/ImageService/Tests/__Snapshots__/ImageDataServiceTests/testGettingImageAtPage3.2.json index 15bdccd9..ba1dc784 100644 --- a/Domain/ImageService/Tests/__Snapshots__/ImageDataServiceTests/testGettingImageAtPage3.2.json +++ b/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 ] ], diff --git a/Domain/ImageService/Tests/__Snapshots__/ImageDataServiceTests/testGettingImageAtPage604.2.json b/Domain/ImageService/Tests/__Snapshots__/ImageDataServiceTests/testGettingImageAtPage604.2.json index 88a63341..2a491000 100644 --- a/Domain/ImageService/Tests/__Snapshots__/ImageDataServiceTests/testGettingImageAtPage604.2.json +++ b/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 ] ], diff --git a/Domain/WordFrameService/WordFrameProcessor.swift b/Domain/WordFrameService/WordFrameProcessor.swift index 4e91fa49..bb77c8c9 100644 --- a/Domain/WordFrameService/WordFrameProcessor.swift +++ b/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 } } } diff --git a/Domain/WordFrameService/WordFrameScale+Extension.swift b/Domain/WordFrameService/WordFrameScale+Extension.swift index 204870bd..d6fc1d79 100644 --- a/Domain/WordFrameService/WordFrameScale+Extension.swift +++ b/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 } }