Implemented the generateThumbHash() function on iOS.
Used code from the thumbhash repository: https://github.com/evanw/thumbhash
Depends on D7778
Details
Details
One easy way is to hook this to expo-image-picker result (e.g. avatar picker) and provide picked uri to the function. It should usually return a 43 char long string (sometimes longer if there is alpha channel).
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| native/expo-modules/thumbhash/ios/Thumbhash.swift | ||
|---|---|---|
| 1–5 ↗ | (On Diff #26340) | This file is copy-pasted from the original repo (only removed unused code) - you can skip it |
| 20–31 ↗ | (On Diff #26340) | cc @atul, you might be interested in this ;) Btw. Wondering if the Accelerate framework you mentioned in https://linear.app/comm/issue/ENG-3380#comment-93c468f6 can be used for thumbhash too |
| native/expo-modules/thumbhash/ios/Thumbhash.swift | ||
|---|---|---|
| 20–31 ↗ | (On Diff #26340) | Interesting! I think this could be optimized via "higher level" Accelerate::vDSP (https://developer.apple.com/documentation/accelerate/vdsp) and Accelerate::vImage (https://developer.apple.com/documentation/accelerate/vimage). Not sure whether there would be any gains in practice, but could be a fun project |
| native/expo-modules/thumbhash/ios/Thumbhash.swift | ||
|---|---|---|
| 20–31 ↗ | (On Diff #26340) | Looks like it's performant enough and they're valuing readability over marginal increase in performance:  |
Comment Actions
Looks good to me!
Adding @ashoat as blocking since the code from https://github.com/evanw/thumbhash is effectively a "new dependency" + to get sign off on license/attribution stuff.
| native/expo-modules/thumbhash/ios/ThumbhashModule.swift | ||
|---|---|---|
| 14–15 ↗ | (On Diff #26433) | Instead of loading photoURI into memory and then passing to UIImage(data: ) initializer, could we use UIImage(contentsOfFile path: String) and let UIImage handle loading of data? Think it would looks something like: guard let image = UIImage(contentsOfFile: photoURI) else { throw LoadingFailureException(photoURI.absoluteString) } ? Probably doesn't make a huge difference, but keep things simpler? |
| 24–28 ↗ | (On Diff #26433) | Was a little confused reading this because it wasn't clear where param came from. It looks like it comes from GenericException<String>:  Would something like the following be clearer?  (Totally defer to you on this one since I have no idea) |
| native/expo-modules/thumbhash/ios/ThumbhashModule.swift | ||
|---|---|---|
| 14–15 ↗ | (On Diff #26433) | I used URL on purpose to keep parity with Android and also to defer the conversion and validation to expo-modules (JS string is auto-converted to Swift URL). The contentsOfFile would need to extract the path from URL. Another advantage of my approach is that when try Data(...) fails, the exception message will be nicely handled by expo-modules and will be more explicit than the one from UIImage failure. |
| 24–28 ↗ | (On Diff #26433) | GenericException's purpose is to avoid writing such long exception classes. It's a typical pattern in expo-modules codebase. It's a shame that expo modules documentation doesn't mention it even though it's widely used. |