WIP rough AppKit support.

[jverkoey]

Note: this currently removes UIKit support. An ideal change would introduce some amount of build-time conditionality to swap the different bits of code.

[dmrschmidt]

Great thanks, if these are all the minimum necessary changes I think it should be relatively quick to get a macOS compatible build.

I'm already working on an approach right now that typealiases UIColor & NSColor respectively in the core libs and using extensions to support the different image rendering approach. I'll see if I can shoehorn this onto your PR here for better attribution. I'll see how git will let me do this :) Will try to push some first draft early if possible.

[dmrschmidt]

😆 oh my and a huge thank you for the sponsorship!! Truly means a lot to me!! Thank you 🙏

[jverkoey]

Def! Thank you for making a library that works so well! I'm trying to build my dream version of iTunes and being able to hook in SoundCloud-like waveform visualization for scrubbing is 💯💯💯 for auditioning tracks :D

[dmrschmidt]

Alright... So I think this is pretty much what's needed.

There's one issue though that I'm struggling with, and that's - I think - configuring the new target correctly.

There's a new macOS example app now, where I want to import the new DSWaveformImage_macos framework (name gets suggested that way by Xcode autocomplete) but then I just get No such module 'DSWaveformImage_macOS' from the compiler.

So I guess something isn't configured quite right yet. When it comes to build settings I'm always just kinda tapping in the dark tbh. If you've got a clue what's missing here, that would be wonderful :)

[dmrschmidt]

I just had an idea how to "fix this". Will just stick with one framework target instead, that supports both. Tomorrow. Should require minimal changes actually.

[dmrschmidt]

That was really straightforward actually :) Would you mind trying this out if it works for your use case now?

I'll then merge the PR, add a bit of README stuff around and push this as a new major release. It should all be backwards compatible but I rather play it safe here.

[jverkoey]

The change you made got really close to the right thing, but unfortunately due to the fact that the SwiftUI APIs were included in the library I ran into the limitation of not being able to link two versions of SwiftUI into a single binary (the macOS SwiftUI version that gets pulled into my AppKit target via your library, and the iOS SwiftUI version from my Catalyst app). It's complicated :(

The cleanest thing I could think to do here involves somewhat of a breaking change proposal. Concretely, I'm proposing in the commit I just pushed that there be two products / libraries in your package now:

1. DSWaveformImage: contains only the core, platform agnostic audio processing and image rendering APIs.
2. DSWaveformImageViews: contains convenience wrappers around DSWaveformImage that handle image rendering in UIKit and SwiftUI.

As part of this proposal, I've also made the following changes in order to facilitate the overall refactoring:

1. Upgraded your swift-tools-version to 5.7. This allows .macOS(.v11) to be specified as the minimum supported macOS. We could lower this minimum OS, but we'd need to conditionalize some of the code in the library as there are usages of macOS v11+ APIs.
2. Moved the source files to match Swift package manager convention of using Sources/<#target#> directories. This simplifies the package specification and makes it easier to align the target names with the product names.
3. Introduced a new package, DSWaveformImageViews. This package depends on DSWaveformImage, which is now just the core image rendering tech.
4. Removed the .framework target in the example project and replaced it with a standard Swift package manager dependency. I'm not sure if you were intentionally supporting a .framework, so keep in mind this is a breaking change.

Change 3 and 4 are the breaking changes. The migration steps for clients for 3 are:

Before:

import DSWaveformImage

After:

import DSWaveformImage
import DSWaveformImageViews

I found that in most cases, both targets needed to be imported because there are some direct references to APIs from DSWaveformImage in the examples.

The migration step for 4 is to use Swift package manager instead of importing the .framework.

I tested the most recent commit of this PR with your example on both iOS, macOS, and with my project which is a combo of Catalyst and AppKit.

All of the above is a proposal, and it def won't break my heart if you feel this is too far off-base from your intended structuring of this project! Happy to discuss further, and am curious what you think about this proposal ❤️

[dmrschmidt]

Thanks for the detailed write-up!

So just generally speaking, it's by no means unheard of to see libraries split into a "Core" and a "UI" counterpart (that's probably what I'd prefer naming-wise, but that's just a teeny tiny detail). Given the overall rather small footprint of this lib however, I'd like to avoid any split as much as possible. Not really much of a very string reason behind it, other than "perceived overhead and complexity". I feel that for 90% of potential users, having two libs to import makes this whole thing "feel" a lot more chunky than it actually is and may deter people. It certainly would make me think twice.

That said, if this is what is required to truly enable it being used in a macOS application, then I guess that outweighs this concern.

Your points 1) and 2) I think are perfectly fine changes. And so is 4) - there was no actual reason I didn't already use SwiftPM in the examples app. It was just more convenient during dev, being able to reference the lib without needing to push it first. I'll need to have a closer look and actually run your changes to see whether this still works. But even if not, going full SwiftPM here is maybe anyway better still, as it shows the intended use case more accurately. Which is the sole purpose of the example app.

What I'm not quite sure I understand is why this

[...] not being able to link two versions of SwiftUI into a single binary (the macOS SwiftUI version [...]

doesn't work. It feels to me that there should be some way to configure some things differently and just make it work™? Let me play around a little with your PR so that I can understand the issue better. As I mentioned, when it comes to the build process, I'm usually a bit lost and I always try to avoid going too deep in the woods here 😅 But I'll do my best to understand this a little better.

[jverkoey]

Ah! Keeping everything in a single library makes sense as a constraint. https://twitter.com/stroughtonsmith/status/1569147911795007489?s=20&t=5ZFESfVcqG29BIaz_y4vnA is the context for the linker error I'm running into when my AppKit framework depends on any file that imports SwiftUI. This situation is admittedly an edge case of edge cases, so I think it's super fair not to break this library into smaller pieces just because of that.

I'll keep pondering as to whether there might be another way to both keep everything in one library while also making it possible to link into an AppKit/Catalyst hybrid app.

[dmrschmidt]

Hmm ok, so if I understand this correctly, then embedding the lib in a "normal" aka AppKit mac app (which is what the newly included mac example app I think is doing, then there won't be issues with the bundled SwiftUI version. So the problem really only arises in these edge case scenarios.

I guess one other possible approach that I'd feel a bit better with could be to sill offer two targets, but have one stay the same that it is currently, and then another one that has only the core parts. So for the 90% cases there's still just one import needed, but for any edge case like yours, there's a supported way to only get the core parts and implement custom drawing logic.

If there was an approach though with smarter compiler directives, that would still be my preferred approach I think.

[dmrschmidt]

I just had a look at the changes a bit more in depth and have a few more thoughts :)

1. Interestingly enough, it made me realize that in my main app where I use the library myself, I am actually not using any of the view classes and instead just the raw rendering logic. So even my own app I guess could benefit from having this split up 😅

2. I remembered that the framework target was still there for maintaining carthage support. While I haven't used carthage myself anymore (and last time I tested that it actually still works is almost as long ago), I'm not sure whether I'd want to drop support for it officially. As far as I know the library still works just fine with carthage. There was no hard reason why you removed it other than it didn't seem necessary, right? So bringing it back would not cause any problems I assume?

3. You mentioned the example app. When I check out the code, I cannot run the example app anymore. I get the error Missing package product 'DSWaveformImageViews'. I see it references under Framework, Libraries and Embedded Content in the target's General tab, but I'm not sure how it should know how to build it? Where is it referencing the swift package?

4. When I open the project in Xcode via the Package.swift file instead of the xcproject, I see a new scheme called DSWaveformImage-Package. What is this for? I think I was only expecting to see DSWaveformImage and DSWaveformImageViews there.

[dmrschmidt]

Alright, merged this.

I've decided to drop carthage support from here on out. Both can live together side by side, so any projects still requiring carthage can either keep using 10.0 perfectly for a whole while or switch over to using SPM for this lib.

I've also went with your split of the two targets.

I did just quickly re-arrange the example code to clean that up a little, everything else should pretty much be as you've edited it.

Let me know if there's anything still missing. I've released this now as 11.0.0.

Thanks a lot for your contribution!