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

Fixes some issues with GIF images in the reader. #13906

Merged
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
6 changes: 3 additions & 3 deletions WordPress/Classes/Utility/Media/GIFPlaybackStrategy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,18 @@ extension GIFPlaybackStrategy {

class SmallGIFPlaybackStrategy: GIFPlaybackStrategy {
var maxSize = 8_000_000 // in MB
var frameBufferCount = 25
var frameBufferCount = 50
var gifStrategy: GIFStrategy = .smallGIFs
}

class MediumGIFPlaybackStrategy: GIFPlaybackStrategy {
var maxSize = 20_000_000 // in MB
var frameBufferCount = 50
var frameBufferCount = 150
var gifStrategy: GIFStrategy = .mediumGIFs
}

class LargeGIFPlaybackStrategy: GIFPlaybackStrategy {
var maxSize = 50_000_000 // in MB
var frameBufferCount = 60
var frameBufferCount = 300
var gifStrategy: GIFStrategy = .largeGIFs
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ import WordPressShared
///
class WPRichContentView: UITextView {

/// Used to keep references to image attachments.
///
var mediaArray = [RichMedia]()

/// Manages the layout and positioning of text attachments.
///
@objc lazy var attachmentManager: WPTextAttachmentManager = {
Expand Down Expand Up @@ -339,30 +335,22 @@ extension WPRichContentView: WPTextAttachmentManagerDelegate {
attachment.maxSize = CGSize(width: finalSize.width, height: finalSize.height)
}

let index = mediaArray.count
let indexPath = IndexPath(row: index, section: 1)
weak var weakImage = image

image.loadImage(from: mediaHost, preferedSize: finalSize, indexPath: indexPath, onSuccess: { [weak self] indexPath in
guard
let richMedia = self?.mediaArray[indexPath.row],
let img = weakImage
else {
image.loadImage(from: mediaHost, preferedSize: finalSize, onSuccess: { [weak self] in
guard let img = weakImage else {
return
}

richMedia.attachment.maxSize = img.contentSize()
attachment.maxSize = img.contentSize()

if isUsingTemporaryLayoutDimensions {
self?.layoutAttachmentViews()
}
}, onError: { (indexPath, error) in
}, onError: { (error) in
DDLogError("\(String(describing: error))")
})

let media = RichMedia(image: image, attachment: attachment)
mediaArray.append(media)

return image
}

Expand Down Expand Up @@ -415,12 +403,13 @@ extension WPRichContentView: WPTextAttachmentManagerDelegate {
/// - Returns: An NSRange optional.
///
func attachmentRangeForRichTextImage(_ richTextImage: WPRichTextImage) -> NSRange? {
for item in mediaArray {
if item.image == richTextImage {
return rangeOfAttachment(item.attachment)
}
guard let match = attachmentManager.attachmentViews.first( where: { (_, value) -> Bool in
return value.view == richTextImage
}) else {
return nil
}
return nil

return rangeOfAttachment(match.key)
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ open class WPRichTextImage: UIControl, WPRichTextMediaAttachment {
@objc fileprivate(set) var imageView: CachedAnimatedImageView

fileprivate lazy var imageLoader: ImageLoader = {
let imageLoader = ImageLoader(imageView: imageView, gifStrategy: .smallGIFs)
let imageLoader = ImageLoader(imageView: imageView, gifStrategy: .largeGIFs)
imageLoader.photonQuality = Constants.readerPhotonQuality
return imageLoader
}()
Expand Down Expand Up @@ -73,25 +73,23 @@ open class WPRichTextImage: UIControl, WPRichTextMediaAttachment {
/// - Parameters:
/// - host: The host for the media.
/// - preferedSize: The prefered size of the image to load.
/// - indexPath: The IndexPath where this view is located — returned as a param in success and error blocks.
/// - onSuccess: A closure to be called if the image was loaded successfully.
/// - onError: A closure to be called if there was an error loading the image.
func loadImage(from host: MediaHost,
preferedSize size: CGSize = .zero,
indexPath: IndexPath,
onSuccess: ((IndexPath) -> Void)?,
onError: ((IndexPath, Error?) -> Void)?) {
onSuccess: (() -> Void)?,
onError: ((Error?) -> Void)?) {
guard let contentURL = self.contentURL else {
onError?(indexPath, nil)
onError?(nil)
return
}

let successHandler: (() -> Void)? = {
onSuccess?(indexPath)
onSuccess?()
}

let errorHandler: ((Error?) -> Void)? = { error in
onError?(indexPath, error)
onError?(error)
}

imageLoader.loadImage(with: contentURL, from: host, preferredSize: size, placeholder: nil, success: successHandler, error: errorHandler)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import UIKit
///
@objc open class WPTextAttachmentManager: NSObject {
@objc open var attachments = [WPTextAttachment]()
var attachmentViews = [String: WPTextAttachmentView]()
var attachmentViews = [WPTextAttachment: WPTextAttachmentView]()
@objc open weak var delegate: WPTextAttachmentManagerDelegate?
@objc fileprivate(set) open weak var textView: UITextView?
@objc let layoutManager: NSLayoutManager
Expand Down Expand Up @@ -40,7 +40,7 @@ import UIKit
/// - Returns: A UIView optional
///
@objc open func viewForAttachment(_ attachment: WPTextAttachment) -> UIView? {
return attachmentViews[attachment.identifier]?.view
return attachmentViews[attachment]?.view
}


Expand Down Expand Up @@ -88,7 +88,7 @@ import UIKit
// Make sure attachments are correctly laid out.
attachmentsAndRanges.forEach { (attachment, range) in

guard let attachmentView = attachmentViews[attachment.identifier] else {
guard let attachmentView = attachmentViews[attachment] else {
return
}

Expand Down Expand Up @@ -125,7 +125,7 @@ import UIKit
self.attachments.append(attachment)

if let view = self.delegate?.attachmentManager(self, viewForAttachment: attachment) {
self.attachmentViews[attachment.identifier] = WPTextAttachmentView(view: view, identifier: attachment.identifier, exclusionPath: nil)
self.attachmentViews[attachment] = WPTextAttachmentView(view: view, identifier: attachment.identifier, exclusionPath: nil)
self.textView?.addSubview(view)
}
})
Expand Down