-
Notifications
You must be signed in to change notification settings - Fork 1
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
커스텀 앨범에서 이미지 편집 뷰로 이동 #44
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!! 👍 👍 👍
options: nil | ||
) { image, _ in | ||
// TODO: - 이미지 편집 뷰 로 이동 | ||
guard let asset = viewModel.photoAsset?[indexPath.item - 1] else { return } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3. 뷰모델의 속성에 직접 접근하는 것 대신 함수를 만들어주면 어떨까요?
func photo(at indexPath: indexPath) -> 사진 느낌?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 부분도 고민을 많이 한 지점이었는데 cell의 delegate나 dataSource 관련된 함수 내부에서 ViewModel과 어떻게 바인딩을 해야할지를 모르겠어서 저렇게 직접 접근하는 방식을 사용했던 것 같습니다..
데이터의 단방향 flow를 만들고 싶어서 ViewModelInput이라는 열거형을 만들고 Input와 output을 나눠 관리하려했는데 메서드를 넣으면 훔... 그래도 확실히 뷰모델의 속성에 직접 접근하는게 어색해보일 수 있을 것 같네요.. 일단 조금 더 고민해보겠습니다 .ᐟ.ᐟ
final class CustomAlbumViewModel { | ||
// MARK: - Properties | ||
@Published private(set) var photoAsset: PHFetchResult<PHAsset>? | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3. 선택의 문제긴 한데 지금처럼 photoAsset을 구독하게 할지, CustomAlbumViewModel을 observableObject로 만들고 viewModel을 구독하게 할지 고민이네요....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
옹 이 부분은 제가 observableObject로 구현해본적이 없어서 관성처럼 나온 코드이긴합니다.. 이 부분에 대해서는 같이 생각해봐도 좋을 것 같네용 .ᐟ.ᐟ
추가로 이 CustomAlbumViewModel이 필요한 것인지도 솔직히 살짝 의문이 들긴합니다.. 최대한 view와 로직을 분리해야겠다는 생각으로 이렇게 나눠보긴했는데 ViewModel이 하는 일이 해당 뷰가 보여지기 시작했을 때 Asset만 불러오는 정도가 다라서.... 고민이 됩니다..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(개인적 의견) ObservableObject는 뭔가 스유 느낌이라 저는 지금 구조가 눈에 익기는 합니다..!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[대 영 현]
고생하셨습니다 !!
enum CustomAlbumViewModelInput { | ||
case viewDidLoad | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
단순 궁금증) 영현님은 언제 nested type을 적용하는지 궁금합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
같은 네임 스페이스를 공유할 때 ..? 뭔가 포함관계가 있다 싶으면 사용하는 것 같습니당
오 근데 이런 기준이라면 저 Input도 nested로 하는게 맞을 것 같네용
private func fetchPhotoAssets() { | ||
let fetchOptions = PHFetchOptions() | ||
fetchOptions.predicate = NSPredicate(format: "mediaType = %d", PHAssetMediaType.image.rawValue) | ||
fetchOptions.sortDescriptors = [NSSortDescriptor(key: "creationDate", ascending: false)] | ||
|
||
photoAsset = PHAsset.fetchAssets(with: fetchOptions) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사진앱에서 사진을 가져오는 코드인가요? 그렇다면 영상과 함께 가져와야하는지 고민되네요! 아니라면 넘어가주세요! ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
옹 24번째 줄 옵션을 넣어서 사진만 가져오게끔 만들었습니다 ! 아무래도 해당 화면은 이미지를 넣을 목적으로 만든 앨범 화면이라 사진만 불러오게 하는 것이 적절할 것 같다고 판단해서 저렇게 만들었습니다 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오홍 나중에 비디오 자체를 가져오기 전에 썸네일만 가져오도록 하는 방식으로도 최적화를 꾀할 수 있을 것같긴 합니다. (나중이야기)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저거 Asset의 mediaType을 대충 비디오로 해주면 알아서 썸네일만 가져옵니당 .ᐟ.ᐟ
let closeAction = UIAction { [weak self] _ in | ||
guard let self else { return } | ||
self.navigationController?.popViewController(animated: true) | ||
} | ||
let leftBarButton = UIBarButtonItem(title: "닫기", primaryAction: closeAction) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2
비동기 작업 전 optional unwrapping을 하시면 weak의 효과가 사라지는 것으로 알고 있습니다.
https://medium.com/@sunghyun_kim/swift-task에서-weak-self는-언제-쓸까요-f7ccf78e22bb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
옹 해당 부분에 대해서 저도 이전 학습 스프린트때 학습한 적이 있는데 weak의 효과가 사라지는 경우는 Task 코드블럭 내부에서만 해당하는 것으로 알고있습니다 ! 위 클로저는 UIAction으로 Task 블럭이 아니기 때문에 self를 언래핑해도 weak 참조가 가능한 것으로 알고있습니다.
추가로 위 코드는 비동기 작업을 해당 클로저 내부에서 하고있지 않기 때문에 괜찮지 않을까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제 생각에는 윤철님 말씀이 맞는 것같습니다. 다만, weak self 목적이 강한순환참조를 방지하려는 목적이기 때문에, 저렇게 사용하는 것자체는 동의합니다. Task나 저 Action이나 모두 escaping closure이기 때문에 weak self로 캡쳐한 후 guard let self
를 쓰면 해당 guard문이 실행될 때부터 강하게 붙잡습니다.(weak의 효과가 사라짐)
참고자료
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다..!
뷰모델이 들어가서 관련해서 궁금한 사항들 코멘트 남겨두었습니당
import Photos | ||
|
||
final class LocalPhotoManager { | ||
nonisolated(unsafe) static let shared = LocalPhotoManager() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
싱글톤 사용 시 Swift 6에서 nonisolated
안 붙이면 에러 뜨나요 ?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵.ᐟ.ᐟ nonisolated를 해주거나 해당 클래스 자체를 Sendable 하게 만들어줘야합니다 .ᐟ.ᐟ 싱글톤이라는 것 자체가 여러 곳에서 하나의 객체를 공유하기 때문에 data race가 발생하기 쉬운 구조라 저런 장치를 해줘야합니당
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오호 swift 6 도전기 시작이군요..
감사합니다..!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data race가 발생하기 쉬운 구조인데 저렇게 하면 에러만 안나게 하는 것아닌가요? data race가 염려되는 구조면 actor를 사용하는 것은 어떤가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
허거거걱 그렇네요 .ᐟ.ᐟ 역시 우리팀 기술 고-문.. 👍👍👍
final class CustomAlbumViewModel { | ||
// MARK: - Properties | ||
@Published private(set) var photoAsset: PHFetchResult<PHAsset>? | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(개인적 의견) ObservableObject는 뭔가 스유 느낌이라 저는 지금 구조가 눈에 익기는 합니다..!
enum CustomAlbumViewModelInput { | ||
case viewDidLoad | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저희 뷰모델은 Action으로 처리 하나용 ??
제가 이렇게 안 해보긴 했는데, 뷰모델에서 할 일들을 case로 관리해주고
사용하는 쪽은 action 메소드만 호출하여 내부에서 switch-case로 각 case에 맞는 메소드 호출해주는 걸까요 ??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 정확합니다 .ᐟ.ᐟ
아직 해당 부분에 대해 저희 팀 내부적으로 정한 건 없어서 이야기를 해봐도 좋을 것 같습니다 !
위에도 적혀있듯 이러한 구조가 제게 제일 익숙한 구조라 관성적으로 이렇게 구현해주었습니다.. 개인적으로 저런 구조가 데이터의 단방향 흐름을 가장 확실하게 (?) 만들어 줄 수 있는 구조라고 생각하긴합니당
다른분들은 어떤식으로 바인딩을 하는지(?) 궁금하네용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 뷰모델에서 해야할 것들을 각각 메소드로 만들어 놓되,
enum과 action으로 정의 안 해두고 썼습니다!
지금 구조가 더 명확해보이네요 좋습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
10 / 10 드립니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
너무 좋읍니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이제 안좋게 되었습니다..
private func fetchPhotoAssets() { | ||
let fetchOptions = PHFetchOptions() | ||
fetchOptions.predicate = NSPredicate(format: "mediaType = %d", PHAssetMediaType.image.rawValue) | ||
fetchOptions.sortDescriptors = [NSSortDescriptor(key: "creationDate", ascending: false)] | ||
|
||
photoAsset = PHAsset.fetchAssets(with: fetchOptions) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오홍 나중에 비디오 자체를 가져오기 전에 썸네일만 가져오도록 하는 방식으로도 최적화를 꾀할 수 있을 것같긴 합니다. (나중이야기)
- EditPhotoViewController @mainactor 키워드 추가 - CustomAlbumViewController 내부 async 함수 처리를 위한 Task 코드 블럭 추가 - CustomAlbumCollectionViewCell의 이미지 설정 메서드 async 처리
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Swift 6 에러 재밌게 잘 봤습니다
앞으로의 우리.. 화이팅 !
@@ -1,13 +1,20 @@ | |||
import UIKit | |||
|
|||
@MainActor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니당 MainActor의 첫 등장이네요,
찾아보니 Main 스레드에서 항상 실행되는 걸 보장하는 거라 하는데,
이 키워드가 없으면 백그라운드에서 돌아갈 가능성,
즉 다른 스레드로 교체될 가능성이 있다? 때문에 에러가 떴던게 아닐까 조심스레 추측해봅니다..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
한밤중 고생한 흔적들이 많이 보이네요... ㅠㅠㅠ 고생하셨습니다...
@@ -1,13 +1,20 @@ | |||
import UIKit | |||
|
|||
@MainActor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let closeAction = UIAction { [weak self] _ in | ||
guard let self else { return } | ||
self.navigationController?.popViewController(animated: true) | ||
} | ||
let leftBarButton = UIBarButtonItem(title: "닫기", primaryAction: closeAction) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제 생각에는 윤철님 말씀이 맞는 것같습니다. 다만, weak self 목적이 강한순환참조를 방지하려는 목적이기 때문에, 저렇게 사용하는 것자체는 동의합니다. Task나 저 Action이나 모두 escaping closure이기 때문에 weak self로 캡쳐한 후 guard let self
를 쓰면 해당 guard문이 실행될 때부터 강하게 붙잡습니다.(weak의 효과가 사라짐)
참고자료
@@ -43,7 +43,7 @@ final class CustomAlbumCollectionViewCell: UICollectionViewCell { | |||
} | |||
|
|||
// MARK: - Set Cell Image | |||
func setPhoto(_ photo: UIImage?) { | |||
func setPhoto(_ photo: UIImage?) async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1. UI관련 작업을 비동기로 하는 것이 옳은지 의문입니다...
func requestImage(with asset: PHAsset?, cellSize: CGSize = .zero, completion: @escaping (UIImage?) -> Void) { | ||
guard let asset else { return } | ||
|
||
imageManager.requestImage( | ||
for: asset, | ||
targetSize: cellSize, | ||
contentMode: .aspectFill, | ||
options: imageRequestOptions, | ||
resultHandler: { image, _ in | ||
completion(image) | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3. 혹시 위에서의 어떤 문제가 생겼는지는 모르겠지만.... MainThread문제라면 아래와같이 시도해 볼 수 있을 것같아요.
func requestImage(with asset: PHAsset?, cellSize: CGSize = .zero, completion: @MainActor (UIImage?) -> Void) {
guard let asset else { return }
imageManager.requestImage(
for: asset,
targetSize: cellSize,
contentMode: .aspectFill,
options: imageRequestOptions,
resultHandler: { image, _ in
Task { await completion(image) }
})
}
- EditPhotoViewController의 @mainactor 키워드 삭제 - LocalPhotoManager의 requestImage escaping 클로저는 @mainactor 처리 - CustomAlbumCollectionViewCell의 setPhoto 함수 async 삭제
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
죻읍니다! 너무 고생하셨어요 !!!! 👍
#️⃣ 연관된 이슈
📝 작업 내용
⚽️ 트러블 슈팅
ImageRequest가 2번 불러지는 이슈
앨범에서 image를 클릭했을때, 이미지 편집 뷰가 2번 연달아 보여지는 이슈가 있었습니다. 왜 이런지 이유를 찾아봤는데 이미지를 불러오기 위해 사용되는
PHImageManager
의 문제였습니다.지금 로직을 간략히 설명해보자면
PHAsset
이라는 이미지와 동영상에 대한 메타데이터만 가지고 있는 친구를 사용하고, 각각의 cell이 생성될 때, 그리고 cell이 선택될 때, cell의 index값을 활용하여 알맞을 asset을 찾아내고PHImageManager
의 메서드인requestImage
를 사용해 사용해 실제 이미지를 불러오게 됩니다.여기서
requestImage
의 애플 공식문서를 읽어보면 아래와 같은 내용이 나옵니다.그래서 어떻게 해결하나?
없는거 빼고 다있는 우리의 스오플이 답을 알려줬습니다.
간단합니다. 그냥 requestImage의 파라미터 중 제가 가뿐히 사뿐히 무시만 options라는 매개변수를 설정해주면 됩니다.
저 options 매개변수에 들어가는 타입은
PHImageRequestOptions
라는 타입으로 여러 설정들을 해줄 수 있습니다. 그 중 저희에게 필요한 옵션은PHImageRequestOptionsDeliveryMode
라는 옵션입니다. (이름 왤케 길죠 ;;)이 옵션은 위와 같이 3가지 선택지가 있는데 �기본 default값은
opportunistic
였습니다.저희는 두 번 이상 이미지가 불리는게 문제였음으로
highQualityFormat
혹은fastFormat
으로 설정해주면 됩니다.조금 시간이 걸리더라도 저화질의 사진보다는 고화질의 사진을 보여주는 것이 적절할 것이라고 판단하여
highQualityFormat
옵션으로 설정해주었습니다 !📸 스크린샷