DSWaveformImage
DSWaveformImage copied to clipboard
dmrschmidt
macOS / AppKit support
I'm working on a personal music player app on macOS and I'm hoping to use the underlying waveform analysis code you've written. Everything in the package is currently bundled into a single target that's compatible only with iOS. Are you open to potentially splitting the package into a few targets, with one containing the platform-agnostic "Waveform*" classes?
Hey @jverkoey, thanks a lot for the PR 🙏 Very much appreciated :)
I was just yesterday thinking about it after I restructured the files more logically into those separate folders. In the end I didn't go into that direction for these thoughts I had:
- Given the small overall size, I subjectively felt the actual benefit would likely be dubious at best
- I wasn't sure what one would gain in terms of reduced size footprint if one only needed one target but assumed it would be minimal
- It would be "more complicated" to use for most people, as most would at least need "Core" and either "SwiftUI" or "UIKit" targets
- The added build time for now needing two targets probably outweighs the organizational benefit
- For most people now needing two targets, they'd probably increase their final binary size
Those are mostly gut feeling based thoughts / conclusions, so I'd generally be open to being swayed into that direction after all. I'd need to better understand the exact benefits of going this direction vs keeping it as is in a more "founded" pro vs con comparison though.
Please feel free to share your thoughts on the reasons here.
Ah wait, or are we talking slightly different things? You actually mean creating two separate macOS / iOS targets?
If that's the case: doesn't the current target actually work already for macOS? Or does it not, because it's "Catalyst only"? Tbh I haven't done any macOS development myself yet, so I'm a little unfamiliar here. But if there's issues with macOS I'll be more than happy to make the library work better for all platforms.
You actually mean creating two separate macOS / iOS targets?
Yep! I'm working on a macCatalyst app, but (prepare for semi-long story 😅) I'm placing the waveform visualizers in the toolbar and, unfortunately, catalyst toolbars don't support custom views. Sooo I'm using a hack from @steventroughtonsmith (context: https://twitter.com/featherless/status/1569144875769303042) that allows you to bridge an AppKit framework into a Catalyst app so that you can build custom views. Unfortunately, this means they have to be written in AppKit! I could transport the rendered images over from the Catalyst portion of the app, but it was conceptually more immediately obvious to pass the URL over the framework bridge and let the player handle the async behaviors.
Now that I explain all of that though, I suppose I could just try rendering the image in the catalyst portion of the app and pass it over the framework bridge as a CGImageRef or something 😅.
Alas, I already got it working by forking your code and updating all of the UI- prefixes to NS-prefixes and one small logical change in the image rendering :)
Happy to propose a pull request if supporting AppKit is something you'd be open to doing!
Great track in the image btw 😄
So yeah, I've just started playing around with macOS and realized a few culprits around shared classes around UIColor, UIScreen etc. Now that I understand what the issue is, I definitely think it would be sensible to either have a dedicated macOS target OR just have the one target work on both platforms via preprocessor directives etc.
Not sure what would be best tbh, but I think I'm leaning towards your splitting suggestion and adding a new DSWaveformImage-macOS framework target which contains the core classes (those would use compiler directives to swap out UI* for NS* classes) and the SwiftUI views.
I think I have some energy at the moment to do get this done relatively soon, possibly even today already. Given there are no unforeseen issues that I don't know about yet.
What are those logical changes in the image rendering about you're referring to? Is this just something that you need custom for your app or is this also platform related? If it's the former, I'd be curious to see if this could be achieved by making the library more flexible to be extensible without the need of forking.
^ Pull request. Note I took the straightest path to getting things building 😅 I'd be happy to clean it up and get it good for merging if you like the direction it's going.