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: Add support for remaining asset types #21664

Merged
merged 11 commits into from
Oct 5, 2023
35 changes: 25 additions & 10 deletions WordPress/Classes/Services/MediaImageService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ final class MediaImageService: NSObject {
return try? await Task.detached {
let imageURL = try self.getCachedThumbnailURL(for: mediaID, size: size)
let data = try Data(contentsOf: imageURL)
return try decompressedImage(from: data)
return try makeImage(from: data)
}.value
}

Expand Down Expand Up @@ -116,7 +116,7 @@ final class MediaImageService: NSObject {

let image = try? await Task.detached {
let data = try Data(contentsOf: export.url)
return try decompressedImage(from: data)
return try makeImage(from: data)
}.value

// The order is important to ensure `export.url` still exists when creating an image
Expand All @@ -143,15 +143,18 @@ final class MediaImageService: NSObject {
guard !Task.isCancelled else {
throw CancellationError()
}
let (data, _) = try await session.data(for: request)

let (data, response) = try await session.data(for: request)
guard let statusCode = (response as? HTTPURLResponse)?.statusCode,
(200..<400).contains(statusCode) else {
throw URLError(.unknown)
}
let image = try await Task.detached {
try makeImage(from: data)
}.value
saveThumbnail(for: media.objectID, size: size) { targetURL in
try data.write(to: targetURL)
}

return try await Task.detached {
try decompressedImage(from: data)
}.value
return image
}

// MARK: - Stubs
Expand Down Expand Up @@ -254,7 +257,13 @@ private extension Media {
}
// Download a non-retina version for GIFs: makes a massive difference
// in terms of size. Example: 2.4 MB -> 350 KB.
let targetSize = remoteURL.isGif ? targetSize.scaled(by: 1.0 / UIScreen.main.scale) : targetSize
let scale = UIScreen.main.scale
var targetSize = targetSize
if remoteURL.isGif {
targetSize = targetSize
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downscaling from 3x to 1x turned out to be a bit too much.

.scaled(by: 1.0 / scale)
.scaled(by: min(2, scale))
}
if !isEligibleForPhoton {
return WPImageURLHelper.imageURLWithSize(targetSize, forImageURL: remoteURL)
} else {
Expand All @@ -275,10 +284,13 @@ private extension Media {

// Forces decompression (or bitmapping) to happen in the background.
// It's very expensive for some image formats, such as JPEG.
private func decompressedImage(from data: Data) throws -> UIImage {
private func makeImage(from data: Data) throws -> UIImage {
guard let image = UIImage(data: data) else {
throw URLError(.cannotDecodeContentData)
}
if data.isMatchingMagicNumbers(Data.gifMagicNumbers) {
return AnimatedImageWrapper(gifData: data) ?? image
}
guard isDecompressionNeeded(for: data) else {
return image
}
Expand All @@ -299,6 +311,9 @@ private extension Data {
// JPEG magic numbers https://en.wikipedia.org/wiki/JPEG
static let jpegMagicNumbers: [UInt8] = [0xFF, 0xD8, 0xFF]

// GIF magic numbers https://en.wikipedia.org/wiki/GIF
static let gifMagicNumbers: [UInt8] = [0x47, 0x49, 0x46]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL


func isMatchingMagicNumbers(_ numbers: [UInt8?]) -> Bool {
guard self.count >= numbers.count else {
return false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ final class SiteMediaCollectionViewController: UIViewController, NSFetchedResult
private var syncError: Error?
private var pendingChanges: [(UICollectionView) -> Void] = []
private var selection = NSMutableOrderedSet() // `Media`
private var viewModels: [NSManagedObjectID: MediaCollectionCellViewModel] = [:]
private var viewModels: [NSManagedObjectID: SiteMediaCollectionCellViewModel] = [:]
private let blog: Blog
private let filter: Set<MediaType>?
private let isShowingPendingUploads: Bool
Expand Down Expand Up @@ -95,7 +95,7 @@ final class SiteMediaCollectionViewController: UIViewController, NSFetchedResult
}

private func configureCollectionView() {
collectionView.register(MediaCollectionCell.self, forCellWithReuseIdentifier: Constants.cellID)
collectionView.register(cell: SiteMediaCollectionCell.self)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to do

collectionView.register(SiteMediaCollectionCell.self, forCellWithReuseIdentifier: SiteMediaCollectioinCell.defaultReuseID)

Looks like you're using a extension method defined in Kanvas

    func register<T: UICollectionViewCell>(cell cellType: T.Type) {
        register(cellType, forCellWithReuseIdentifier: cellType.identifier)
    }

This works, because UICollectionReusableView conforms to Kanvas.Identifiable. Just wanted to point out that it's not leveraging Reusable, but Kanvas.Identifiable instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, Kanvas.Identifiable is nice, but we should probably not rely on it. I want to get this merged before the trunk gets too far ahead. Let me address it separately together with the filename fix.


view.addSubview(collectionView)
collectionView.translatesAutoresizingMaskIntoConstraints = false
Expand Down Expand Up @@ -260,6 +260,13 @@ final class SiteMediaCollectionViewController: UIViewController, NSFetchedResult
pendingChanges.append({ $0.deleteItems(at: [indexPath]) })
if let media = anObject as? Media {
setSelect(false, for: media)

if let viewController = navigationController?.topViewController,
viewController !== self,
let detailsViewController = viewController as? MediaItemViewController,
detailsViewController.media.objectID == media.objectID {
navigationController?.popViewController(animated: true)
}
} else {
assertionFailure("Invalid object: \(anObject)")
}
Expand Down Expand Up @@ -303,7 +310,7 @@ final class SiteMediaCollectionViewController: UIViewController, NSFetchedResult
}

func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
let cell = collectionView.dequeueReusableCell(withReuseIdentifier: Constants.cellID, for: indexPath) as! MediaCollectionCell
let cell = collectionView.dequeue(cell: SiteMediaCollectionCell.self, for: indexPath)!
let media = fetchController.object(at: indexPath)
let viewModel = getViewModel(for: media)
cell.configure(viewModel: viewModel)
Expand Down Expand Up @@ -392,11 +399,11 @@ final class SiteMediaCollectionViewController: UIViewController, NSFetchedResult
// MARK: - Helpers

// Create ViewModel lazily to avoid fetching more managed objects than needed.
private func getViewModel(for media: Media) -> MediaCollectionCellViewModel {
private func getViewModel(for media: Media) -> SiteMediaCollectionCellViewModel {
if let viewModel = viewModels[media.objectID] {
return viewModel
}
let viewModel = MediaCollectionCellViewModel(media: media)
let viewModel = SiteMediaCollectionCellViewModel(media: media)
viewModels[media.objectID] = viewModel
return viewModel
}
Expand Down Expand Up @@ -452,10 +459,6 @@ extension SiteMediaCollectionViewController: NoResultsViewHost {
}
}

private enum Constants {
static let cellID = "cellID"
}

private enum Strings {
static let syncFailed = NSLocalizedString("media.syncFailed", value: "Unable to sync media", comment: "Title of error prompt shown when a sync fails.")
static let retryMenuRetry = NSLocalizedString("mediaLibrary.retryOptionsAlert.retry", value: "Retry Upload", comment: "User action to retry media upload.")
Expand Down

This file was deleted.

Loading