Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Media: Thumbnails #21615

Merged
merged 10 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions WordPress/Classes/Extensions/Math.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,14 @@ extension CGSize {
func clamp(min minValue: Int, max maxValue: Int) -> CGSize {
return clamp(min: CGFloat(minValue), max: CGFloat(maxValue))
}

func scaled(by scale: CGFloat) -> CGSize {
CGSize(width: width * scale, height: height * scale)
}

func rounded() -> CGSize {
CGSize(width: width.rounded(), height: height.rounded())
}
}

extension CGFloat {
Expand Down
6 changes: 2 additions & 4 deletions WordPress/Classes/Models/Media.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,8 @@ typedef NS_ENUM(NSUInteger, MediaType) {
@property (nonatomic, strong, nullable) NSURL *absoluteLocalURL;

/**
Local file URL for a preprocessed thumbnail of the Media's asset. This may be nil if the
thumbnail has been deleted from the cache directory.

Note: it is recommended to instead use MediaService to generate thumbnails with a preferred size.
Local file URL for a preprocessed **large** thumbnail that can be used for
a full-screen presentation.
*/
@property (nonatomic, strong, nullable) NSURL *absoluteThumbnailLocalURL;

Expand Down
333 changes: 227 additions & 106 deletions WordPress/Classes/Services/MediaImageService.swift

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions WordPress/Classes/Services/MediaImportService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,7 @@ class MediaImportService: NSObject {
/// using `absoluteThumbnailLocalURL`.
func exportPlaceholderThumbnail(for media: Media, completion: ((URL?) -> Void)?) {
let thumbnailService = MediaThumbnailService(coreDataStack: coreDataStack)
let targetSize: CGSize = FeatureFlag.mediaModernization.enabled ? MediaImageService.preferredThumbnailPointSize : .zero
thumbnailService.thumbnailURL(forMedia: media, preferredSize: targetSize) { url in
thumbnailService.thumbnailURL(forMedia: media, preferredSize: .zero) { url in
self.coreDataStack.performAndSave({ context in
let mediaInContext = try context.existingObject(with: media.objectID) as! Media
// Set the absoluteThumbnailLocalURL with the generated thumbnail's URL.
Expand Down
1 change: 1 addition & 0 deletions WordPress/Classes/System/WordPressAppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ class WordPressAppDelegate: UIResponder, UIApplicationDelegate {
window = UIWindow(frame: UIScreen.main.bounds)
AppAppearance.overrideAppearance()
MemoryCache.shared.register()
MediaImageService.migrateCacheIfNeeded()

// Start CrashLogging as soon as possible (in case a crash happens during startup)
try? loggingStack.start()
Expand Down
6 changes: 3 additions & 3 deletions WordPress/Classes/Utility/Media/MediaThumbnailExporter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class MediaThumbnailExporter: MediaExporter {
/// of the image within a layout's dimensions. If nil, the image will not be resized.
///
/// - Note: The final size may or may not match the preferred dimensions, depending
/// on the original image.
/// on the original image
///
var preferredSize: CGSize?

Expand All @@ -36,7 +36,7 @@ class MediaThumbnailExporter: MediaExporter {

/// The compression quality of the thumbnail, if the image type supports compression.
///
var compressionQuality = 0.90
var compressionQuality = 0.8

/// The target image type of the exported thumbnail images.
///
Expand All @@ -59,7 +59,7 @@ class MediaThumbnailExporter: MediaExporter {
guard let size = preferredSize else {
return nil
}
return max(size.width, size.height) * scale
return max(size.width, size.height) * min(2, scale)
}

lazy var identifier: String = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ final class MediaCollectionCellViewModel {
// No sure why but `.initial` didn't work.
self.updateOverlayState()

thumbnailObservation = media.observe(\.localThumbnailURL, options: [.new]) { [weak self] media, _ in
thumbnailObservation = media.observe(\.localURL, options: [.new]) { [weak self] media, _ in
self?.didUpdateLocalThumbnail()
}
}
Expand Down Expand Up @@ -118,7 +118,7 @@ final class MediaCollectionCellViewModel {

// Monitors thumbnails generated by `MediaImportService`.
private func didUpdateLocalThumbnail() {
guard media.remoteStatus != .sync, media.localThumbnailURL != nil else { return }
guard media.remoteStatus != .sync, media.localURL != nil else { return }
fetchThumbnailIfNeeded()
}

Expand Down
4 changes: 4 additions & 0 deletions WordPress/WordPress.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,7 @@
0CDEC40D2A2FAF0500BB3A91 /* DashboardBlazeCampaignsCardView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CDEC40B2A2FAF0500BB3A91 /* DashboardBlazeCampaignsCardView.swift */; };
0CED95602A460F4B0020F420 /* DebugFeatureFlagsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CED955F2A460F4B0020F420 /* DebugFeatureFlagsView.swift */; };
0CED95612A460F4B0020F420 /* DebugFeatureFlagsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CED955F2A460F4B0020F420 /* DebugFeatureFlagsView.swift */; };
0CF7D6C32ABB753A006D1E89 /* MediaImageServiceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CF7D6C22ABB753A006D1E89 /* MediaImageServiceTests.swift */; };
1702BBDC1CEDEA6B00766A33 /* BadgeLabel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1702BBDB1CEDEA6B00766A33 /* BadgeLabel.swift */; };
1702BBE01CF3034E00766A33 /* DomainsService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1702BBDF1CF3034E00766A33 /* DomainsService.swift */; };
17039225282E6D2800F602E9 /* ViewsVisitorsLineChartCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = DC772B0728201F5300664C02 /* ViewsVisitorsLineChartCell.swift */; };
Expand Down Expand Up @@ -6098,6 +6099,7 @@
0CD382852A4B6FCE00612173 /* DashboardBlazeCardCellViewModelTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DashboardBlazeCardCellViewModelTest.swift; sourceTree = "<group>"; };
0CDEC40B2A2FAF0500BB3A91 /* DashboardBlazeCampaignsCardView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DashboardBlazeCampaignsCardView.swift; sourceTree = "<group>"; };
0CED955F2A460F4B0020F420 /* DebugFeatureFlagsView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DebugFeatureFlagsView.swift; sourceTree = "<group>"; };
0CF7D6C22ABB753A006D1E89 /* MediaImageServiceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MediaImageServiceTests.swift; sourceTree = "<group>"; };
0CFD6C792A73E703003DD0A0 /* WordPress 152.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "WordPress 152.xcdatamodel"; sourceTree = "<group>"; };
131D0EE49695795ECEDAA446 /* Pods-WordPressTest.release-alpha.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-WordPressTest.release-alpha.xcconfig"; path = "../Pods/Target Support Files/Pods-WordPressTest/Pods-WordPressTest.release-alpha.xcconfig"; sourceTree = "<group>"; };
150B6590614A28DF9AD25491 /* Pods-Apps-Jetpack.release-alpha.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Apps-Jetpack.release-alpha.xcconfig"; path = "../Pods/Target Support Files/Pods-Apps-Jetpack/Pods-Apps-Jetpack.release-alpha.xcconfig"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -12301,6 +12303,7 @@
24A2948225D602710000A51E /* BlogTimeZoneTests.m */,
D848CC1620FF38EA00A9038F /* FormattableCommentRangeTests.swift */,
0879FC151E9301DD00E1EFC8 /* MediaTests.swift */,
0CF7D6C22ABB753A006D1E89 /* MediaImageServiceTests.swift */,
C38C5D8027F61D2C002F517E /* MenuItemTests.swift */,
D848CC1420FF33FC00A9038F /* NotificationContentRangeTests.swift */,
D826D67E211D21C700A5D8FE /* NullMockUserDefaults.swift */,
Expand Down Expand Up @@ -23371,6 +23374,7 @@
DC13DB7E293FD09F00E33561 /* StatsInsightsStoreTests.swift in Sources */,
ACACE3AE28D729FA000992F9 /* NoResultsViewControllerTests.swift in Sources */,
4A2C73E42A943DEA00ACE79E /* TaggedManagedObjectIDTests.swift in Sources */,
0CF7D6C32ABB753A006D1E89 /* MediaImageServiceTests.swift in Sources */,
8BFE36FF230F1C850061EBA8 /* AbstractPost+fixLocalMediaURLsTests.swift in Sources */,
C995C22629D30AB000ACEF43 /* WidgetUrlSourceTests.swift in Sources */,
08A2AD791CCED2A800E84454 /* PostTagServiceTests.m in Sources */,
Expand Down
161 changes: 161 additions & 0 deletions WordPress/WordPressTest/MediaImageServiceTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
import XCTest
import OHHTTPStubs
@testable import WordPress

class MediaImageServiceTests: CoreDataTestCase {
var mediaFileManager: MediaFileManager!
var sut: MediaImageService!

override func setUp() {
super.setUp()

mediaFileManager = MediaFileManager(directory: .temporary(id: UUID()))
sut = MediaImageService(
coreDataStack: contextManager,
mediaFileManager: mediaFileManager
)
}

override func tearDown() {
super.tearDown()

HTTPStubs.removeAllStubs()

if let directoryURL = try? mediaFileManager.directoryURL() {
try? FileManager.default.removeItem(at: directoryURL)
}
}

// MARK: - Local Resources

func testSmallThumbnailForLocalImage() async throws {
// GIVEN
let media = Media(context: mainContext)
media.mediaType = .image
media.width = 1024
media.height = 680
let localURL = try makeLocalURL(forResource: "test-image", fileExtension: "jpg")
media.absoluteLocalURL = localURL
try mainContext.obtainPermanentIDs(for: [media])

// WHEN
let thumbnail = try await sut.thumbnail(for: media)

// THEN a small thumbnail is created
XCTAssertEqual(thumbnail.size, MediaImageService.getThumbnailSize(for: media, size: .small))

// GIVEN local asset is deleted
try FileManager.default.removeItem(at: localURL)
sut.flush()

// WHEN
let cachedThumbnail = try await sut.thumbnail(for: media)

// THEN cached thumbnail is still available
XCTAssertEqual(cachedThumbnail.size, MediaImageService.getThumbnailSize(for: media, size: .small))
}

// MARK: - Remote Resources

func testSmallThumbnailForRemoteImage() async throws {
// GIVEN
let media = Media(context: mainContext)
media.mediaType = .image
media.width = 1024
media.height = 680
let remoteURL = try XCTUnwrap(URL(string: "https://example.files.wordpress.com/2023/09/image.jpg"))
media.remoteURL = remoteURL.absoluteString
try mainContext.obtainPermanentIDs(for: [media])

// GIVEN remote image is mocked and is resized based on the parameters
try mockRemoteImage(withResource: "test-image", fileExtension: "jpg")

// WHEN
let thumbnail = try await sut.thumbnail(for: media)

// THEN a small thumbnail is created
XCTAssertEqual(thumbnail.size, MediaImageService.getThumbnailSize(for: media, size: .small))

// GIVEN local asset is deleted
sut.flush()

// WHEN
let cachedThumbnail = try await sut.thumbnail(for: media)

// THEN cached thumbnail is still available
XCTAssertEqual(cachedThumbnail.size, MediaImageService.getThumbnailSize(for: media, size: .small))
}

// MARK: - Target Size

func testThatLandscapeImageIsResizedToFillTargetSize() {
XCTAssertEqual(
MediaImageService.targetSize(
forMediaSize: CGSize(width: 3000, height: 2000),
targetSize: CGSize(width: 200, height: 200)
),
CGSize(width: 300, height: 200)
)
}

func testThatPortraitImageIsResizedToFillTargetSize() {
XCTAssertEqual(
MediaImageService.targetSize(
forMediaSize: CGSize(width: 2000, height: 3000),
targetSize: CGSize(width: 200, height: 200)
),
CGSize(width: 200, height: 300)
)
}

func testThatPanoramaIsResizedToSaneSize() {
XCTAssertEqual(
MediaImageService.targetSize(
forMediaSize: CGSize(width: 4000, height: 400),
targetSize: CGSize(width: 200, height: 200)
),
CGSize(width: 800, height: 200)
)
}

func testThatImagesAreNotUpscaled() {
XCTAssertEqual(
MediaImageService.targetSize(
forMediaSize: CGSize(width: 30, height: 20),
targetSize: CGSize(width: 200, height: 200)
),
CGSize(width: 30, height: 20)
)
}

// MARK: - Helpers

/// `Media` is hardcoded to work with a specific direcoty URL managed by `MediaFileManager`
func makeLocalURL(forResource name: String, fileExtension: String) throws -> URL {
let sourceURL = try XCTUnwrap(Bundle.test.url(forResource: name, withExtension: fileExtension))
let mediaURL = try MediaFileManager.default.makeLocalMediaURL(withFilename: name, fileExtension: fileExtension)
try FileManager.default.copyItem(at: sourceURL, to: mediaURL)
return mediaURL
}

func mockRemoteImage(withResource name: String, fileExtension: String) throws {
let sourceURL = try XCTUnwrap(Bundle.test.url(forResource: name, withExtension: fileExtension))
let image = try XCTUnwrap(UIImage(data: try Data(contentsOf: sourceURL)))

stub(condition: { _ in
return true
}, response: { request in
guard let url = request.url,
let components = URLComponents(url: url, resolvingAgainstBaseURL: false),
let queryItems = components.queryItems,
let resize = queryItems.first(where: { $0.name == "resize" }),
let values = resize.value?.components(separatedBy: ","), values.count == 2,
let width = Int(values[0]), let height = Int(values[1]) else {
return HTTPStubsResponse(error: URLError(.unknown))
}
let resizedImage = image.resizedImage(CGSize(width: width, height: height), interpolationQuality: .default)
let responseData = resizedImage?.jpegData(compressionQuality: 0.8) ?? Data()
return HTTPStubsResponse(data: responseData, statusCode: 200, headers: nil)
})
}
}