Skip to content

Commit

Permalink
Moved diffable data sources to background queue (#1033)
Browse files Browse the repository at this point in the history
  • Loading branch information
@michalrentka
michalrentka authored Dec 4, 2024
1 parent 9dc4035 commit e046b7a
Show file tree
Hide file tree
Showing 7 changed files with 445 additions and 389 deletions.
2 changes: 1 addition & 1 deletion Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ final class ItemDetailCollectionViewHandler: NSObject {

let toReload = rowsToReload(from: oldRows, to: newRows, in: section)
if !toReload.isEmpty {
snapshot.reloadItems(toReload)
snapshot.reconfigureItems(toReload)
}

dataSource.apply(snapshot, animatingDifferences: animated)
Expand Down
194 changes: 102 additions & 92 deletions Zotero/Scenes/Detail/Lookup/Views/LookupViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class LookupViewController: UIViewController {
private unowned let remoteFileDownloader: RemoteAttachmentDownloader
private unowned let schemaController: SchemaController
private let disposeBag: DisposeBag
private let updateQueue: DispatchQueue

init(
viewModel: ViewModel<LookupActionHandler>,
Expand All @@ -65,6 +66,7 @@ class LookupViewController: UIViewController {
self.remoteFileDownloader = remoteFileDownloader
self.schemaController = schemaController
self.disposeBag = DisposeBag()
updateQueue = DispatchQueue(label: "org.zotero.LookupViewController.UpdateQueue")
super.init(nibName: "LookupViewController", bundle: nil)
self.setupAttachmentObserving(observer: remoteDownloadObserver)
}
Expand Down Expand Up @@ -100,124 +102,131 @@ class LookupViewController: UIViewController {
private func update(state: LookupState) {
switch state.lookupState {
case .failed(let error):
self.tableView.isHidden = true
self.activityIndicator.stopAnimating()
self.activityIndicator.isHidden = true
self.errorLabel.text = error.localizedDescription
self.errorLabel.isHidden = false
tableView.isHidden = true
activityIndicator.stopAnimating()
activityIndicator.isHidden = true
errorLabel.text = error.localizedDescription
errorLabel.isHidden = false

case .waitingInput:
self.tableView.isHidden = true
self.errorLabel.isHidden = true
self.activityIndicator.stopAnimating()
self.activityIndicator.isHidden = true
tableView.isHidden = true
errorLabel.isHidden = true
activityIndicator.stopAnimating()
activityIndicator.isHidden = true

case .loadingIdentifiers:
self.tableView.isHidden = true
self.errorLabel.isHidden = true
self.activityIndicator.isHidden = false
self.activityIndicator.startAnimating()
tableView.isHidden = true
errorLabel.isHidden = true
activityIndicator.isHidden = false
activityIndicator.startAnimating()

case .lookup(let data):
// It takes a little while for the `contentSize` observer notification to come, so all the content is hidden after the notification arrives, so that there is not an empty screen while
// waiting for it.
self.show(data: data) { [weak self] in
guard let self = self else { return }
self.activityIndicator.stopAnimating()
self.activityIndicator.isHidden = true
self.errorLabel.isHidden = true
self.tableView.isHidden = false

self.dataReloaded?()

self.closeAfterUpdateIfNeeded()
show(data: data) { [weak self] in
guard let self else { return }
activityIndicator.stopAnimating()
activityIndicator.isHidden = true
errorLabel.isHidden = true
tableView.isHidden = false
dataReloaded?()
closeAfterUpdateIfNeeded()
}
}
}

private func show(data: [LookupState.LookupData], completion: @escaping () -> Void) {
var snapshot = NSDiffableDataSourceSnapshot<Int, Row>()
snapshot.appendSections([0])

for lookup in data {
switch lookup.state {
case .translated(let translationData):
let title: String
if let _title = translationData.response.fields[KeyBaseKeyPair(key: FieldKeys.Item.title, baseKey: nil)] {
title = _title
} else {
let _title = translationData.response.fields.first(where: { self.schemaController.baseKey(for: translationData.response.rawType, field: $0.key.key) == FieldKeys.Item.title })?.value
title = _title ?? ""
}
let itemData = Row.Item(type: translationData.response.rawType, title: title)

snapshot.appendItems([.item(itemData)], toSection: 0)
tableView.isHidden = false

let attachments = translationData.attachments.map({ attachment -> Row in
let (progress, error) = self.remoteFileDownloader.data(for: attachment.0.key, parentKey: translationData.response.key, libraryId: attachment.0.libraryId)
let updateKind: RemoteAttachmentDownloader.Update.Kind
if error != nil {
updateKind = .failed
} else if let progress = progress {
updateKind = .progress(progress)
} else {
updateKind = .ready(attachment.0)
}
return .attachment(attachment.0, updateKind)
})
snapshot.appendItems(attachments, toSection: 0)
updateQueue.async { [weak self] in
guard let self else { return }

case .failed:
snapshot.appendItems([.identifier(identifier: lookup.identifier, state: .failed)])

case .inProgress:
snapshot.appendItems([.identifier(identifier: lookup.identifier, state: .inProgress)])
let snapshot = createSnapshot(from: data, schemaController: schemaController, remoteFileDownloader: remoteFileDownloader)
dataSource.apply(snapshot, animatingDifferences: false) { [weak self] in
guard let self, contentSizeObserver == nil else {
completion()
return
}

case .enqueued:
snapshot.appendItems([.identifier(identifier: lookup.identifier, state: .enqueued)])
// For some reason, the observer subscription has to be here, doesn't work if it's in `viewDidLoad`.
contentSizeObserver = tableView.observe(\.contentSize, options: [.new]) { [weak self] _, change in
guard let self, let value = change.newValue, value.height != tableViewHeight.constant else { return }
tableViewHeight.constant = value.height
if value.height >= self.tableView.frame.height, !tableView.isScrollEnabled {
tableView.isScrollEnabled = true
}
completion()
}
}
}

self.tableView.isHidden = false
self.dataSource.apply(snapshot, animatingDifferences: false)

guard self.contentSizeObserver == nil else {
completion()
return
}
// For some reason, the observer subscription has to be here, doesn't work if it's in `viewDidLoad`.
self.contentSizeObserver = self.tableView.observe(\.contentSize, options: [.new]) { [weak self] _, change in
guard let self = self, let value = change.newValue, value.height != self.tableViewHeight.constant else { return }
func createSnapshot(from data: [LookupState.LookupData], schemaController: SchemaController, remoteFileDownloader: RemoteAttachmentDownloader) -> NSDiffableDataSourceSnapshot<Int, Row> {
var snapshot = NSDiffableDataSourceSnapshot<Int, Row>()
snapshot.appendSections([0])

self.tableViewHeight.constant = value.height

if value.height >= self.tableView.frame.height, !self.tableView.isScrollEnabled {
self.tableView.isScrollEnabled = true
for lookup in data {
switch lookup.state {
case .translated(let translationData):
let title: String
if let _title = translationData.response.fields[KeyBaseKeyPair(key: FieldKeys.Item.title, baseKey: nil)] {
title = _title
} else {
let _title = translationData.response.fields.first(where: { schemaController.baseKey(for: translationData.response.rawType, field: $0.key.key) == FieldKeys.Item.title })?.value
title = _title ?? ""
}
let itemData = Row.Item(type: translationData.response.rawType, title: title)

snapshot.appendItems([.item(itemData)], toSection: 0)

let attachments = translationData.attachments.map({ attachment -> Row in
let (progress, error) = remoteFileDownloader.data(for: attachment.0.key, parentKey: translationData.response.key, libraryId: attachment.0.libraryId)
let updateKind: RemoteAttachmentDownloader.Update.Kind
if error != nil {
updateKind = .failed
} else if let progress = progress {
updateKind = .progress(progress)
} else {
updateKind = .ready(attachment.0)
}
return .attachment(attachment.0, updateKind)
})
snapshot.appendItems(attachments, toSection: 0)

case .failed:
snapshot.appendItems([.identifier(identifier: lookup.identifier, state: .failed)])

case .inProgress:
snapshot.appendItems([.identifier(identifier: lookup.identifier, state: .inProgress)])

case .enqueued:
snapshot.appendItems([.identifier(identifier: lookup.identifier, state: .enqueued)])
}
}

completion()
return snapshot
}
}

private func process(update: RemoteAttachmentDownloader.Update) {
guard update.download.libraryId == self.viewModel.state.libraryId, var snapshot = self.dataSource?.snapshot(), !snapshot.sectionIdentifiers.isEmpty else { return }

var rows = snapshot.itemIdentifiers(inSection: 0)
private func process(update: RemoteAttachmentDownloader.Update, completion: @escaping () -> Void) {
guard update.download.libraryId == viewModel.state.libraryId, var snapshot = dataSource?.snapshot(), !snapshot.sectionIdentifiers.isEmpty else { return }

guard let index = rows.firstIndex(where: { $0.isAttachment(withKey: update.download.key, libraryId: update.download.libraryId) }) else { return }
updateQueue.async { [weak self] in
guard let self else { return }

snapshot.deleteItems(rows)
var rows = snapshot.itemIdentifiers(inSection: 0)
guard let index = rows.firstIndex(where: { $0.isAttachment(withKey: update.download.key, libraryId: update.download.libraryId) }) else { return }
snapshot.deleteItems(rows)
let row = rows[index]
switch row {
case .attachment(let attachment, _):
rows[index] = .attachment(attachment, update.kind)

let row = rows[index]
switch row {
case .attachment(let attachment, _):
rows[index] = .attachment(attachment, update.kind)
case .item, .identifier: break
case .item, .identifier:
break
}
snapshot.appendItems(rows, toSection: 0)
dataSource.apply(snapshot, animatingDifferences: false, completion: completion)
}

snapshot.appendItems(rows, toSection: 0)

self.dataSource.apply(snapshot, animatingDifferences: false)
}

private func closeAfterUpdateIfNeeded() {
Expand Down Expand Up @@ -300,9 +309,10 @@ class LookupViewController: UIViewController {

private func setupAttachmentObserving(observer: PublishSubject<RemoteAttachmentDownloader.Update>) {
observer.observe(on: MainScheduler.instance)
.subscribe(with: self, onNext: { `self`, update in
self.process(update: update)
self.closeAfterUpdateIfNeeded()
.subscribe(onNext: { [weak self] update in
self?.process(update: update) { [weak self] in
self?.closeAfterUpdateIfNeeded()
}
})
.disposed(by: self.disposeBag)
}
Expand Down
60 changes: 36 additions & 24 deletions Zotero/Scenes/Detail/PDF/Views/PDFAnnotationsViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ protocol AnnotationsDelegate: AnyObject {
final class PDFAnnotationsViewController: UIViewController {
private static let cellId = "AnnotationCell"
private unowned let viewModel: ViewModel<PDFReaderActionHandler>
private let updateQueue: DispatchQueue
private let disposeBag: DisposeBag

private weak var emptyLabel: UILabel!
Expand All @@ -44,7 +45,8 @@ final class PDFAnnotationsViewController: UIViewController {

init(viewModel: ViewModel<PDFReaderActionHandler>) {
self.viewModel = viewModel
self.disposeBag = DisposeBag()
disposeBag = DisposeBag()
updateQueue = DispatchQueue(label: "org.zotero.PDFAnnotationsViewController.UpdateQueue")
super.init(nibName: nil, bundle: nil)
}

Expand All @@ -66,13 +68,16 @@ final class PDFAnnotationsViewController: UIViewController {
override func viewIsAppearing(_ animated: Bool) {
super.viewIsAppearing(animated)

var snapshot = NSDiffableDataSourceSnapshot<Int, PDFReaderState.AnnotationKey>()
snapshot.appendSections([0])
snapshot.appendItems(viewModel.state.sortedKeys)
tableView.setEditing(viewModel.state.sidebarEditingEnabled, animated: false)
dataSource.apply(snapshot, animatingDifferences: false)
if let key = viewModel.state.focusSidebarKey, let indexPath = dataSource.indexPath(for: key) {
tableView.selectRow(at: indexPath, animated: false, scrollPosition: .middle)
updateQueue.async { [weak self] in
guard let self else { return }
var snapshot = NSDiffableDataSourceSnapshot<Int, PDFReaderState.AnnotationKey>()
snapshot.appendSections([0])
snapshot.appendItems(viewModel.state.sortedKeys)
dataSource.apply(snapshot, animatingDifferences: false) { [weak self] in
guard let self, let key = viewModel.state.focusSidebarKey, let indexPath = dataSource.indexPath(for: key) else { return }
tableView.selectRow(at: indexPath, animated: false, scrollPosition: .middle)
}
}

viewModel.stateObservable
Expand Down Expand Up @@ -155,7 +160,7 @@ final class PDFAnnotationsViewController: UIViewController {
emptyLabel.isHidden = !tableView.isHidden
}

self.reloadIfNeeded(for: state) { [weak self] in
reloadIfNeeded(for: state) { [weak self] in
guard let self else { return }

if let keys = state.loadedPreviewImageAnnotationKeys {
Expand Down Expand Up @@ -202,33 +207,40 @@ final class PDFAnnotationsViewController: UIViewController {
let isVisible = parentDelegate?.isSidebarVisible ?? false

if state.changes.contains(.annotations) {
var snapshot = NSDiffableDataSourceSnapshot<Int, PDFReaderState.AnnotationKey>()
snapshot.appendSections([0])
snapshot.appendItems(state.sortedKeys)
if let keys = state.updatedAnnotationKeys {
snapshot.reloadItems(keys)
}

if state.changes.contains(.sidebarEditing) {
tableView.setEditing(state.sidebarEditingEnabled, animated: isVisible)
tableView.setEditing(state.sidebarEditingEnabled, animated: false)
}
updateQueue.async { [weak self] in
guard let self else { return }
var snapshot = NSDiffableDataSourceSnapshot<Int, PDFReaderState.AnnotationKey>()
snapshot.appendSections([0])
snapshot.appendItems(state.sortedKeys)
if let keys = state.updatedAnnotationKeys {
snapshot.reloadItems(keys)
}
dataSource.apply(snapshot, animatingDifferences: isVisible, completion: completion)
}
dataSource.apply(snapshot, animatingDifferences: isVisible, completion: completion)
return
}

if state.changes.contains(.interfaceStyle) {
var snapshot = dataSource.snapshot()
guard !snapshot.sectionIdentifiers.isEmpty else { return }
snapshot.reloadSections([0])
dataSource.apply(snapshot, animatingDifferences: false, completion: completion)
updateQueue.async { [weak self] in
guard let self else { return }
var snapshot = dataSource.snapshot()
guard !snapshot.sectionIdentifiers.isEmpty else { return }
snapshot.reloadSections([0])
dataSource.apply(snapshot, animatingDifferences: false, completion: completion)
}
return
}

if state.changes.contains(.selection) || state.changes.contains(.activeComment) {
if let keys = state.updatedAnnotationKeys {
var snapshot = dataSource.snapshot()
snapshot.reloadItems(keys)
dataSource.apply(snapshot, animatingDifferences: false)
updateQueue.sync { [unowned self] in
var snapshot = dataSource.snapshot()
snapshot.reloadItems(keys)
dataSource.apply(snapshot, animatingDifferences: false)
}
}

updateCellHeight()
Expand Down
Loading

0 comments on commit e046b7a

Please sign in to comment.