search

AudioKit / AudioKitUI

Controls and Visualization for AudioKit apps
MIT License
187 stars 52 forks source link

Flat Spectrogram View #82

Closed aure closed 3 months ago

mahal commented 3 months ago

Thanks for getting onto that Spectrogram. I'm sorry about the many hound findings, didn't know about SwiftLint. Added it to my project so my future PRs won't suffer from colon violations and the like.

aure commented 3 months ago

If you want to give me write privileges to your repo, I can fix things up there and this PR will be updated, or else I can fork yours and make a new pr off of it. Best to address these things prior to merging.

mahal commented 3 months ago

Sure, added you as collaborator.

mahal commented 3 months ago

Hey @aure I have fixed all but one hound linter warning. Will read more about protocol Identifiable, maybe use an lint exception.

mahal commented 3 months ago

@maksutovic thank you so much for the thorough review and the totally legit questions. Will work on it and push it into the fork.

mahal commented 3 months ago

Added swiftlint directive to disable the warning from Hound about the var called id. With recent versions of swiftlint, the newly added lines will show up as a warning: Superflous Disable Command :-)

mahal commented 3 months ago

@maksutovic I've added more detail at the top of the class describing the dataflow and history of the class.

Drawing is done using UIGraphicsImageRenderer which is Core Graphics, with context.fill primitives. These then are cached as UImage and layouted onto the view.

I was struggling and couldn't find a good example on how to animate a Canvas from right to left as well as drawing async on it.

I did my fair share of Core Animation development with raskinformac.com but I'm pretty new to SwiftUI. So it didn’t took me long to figure out the bottleneck of the existing SpectrogramView using instruments and stuff, but also couldn't implement what I had in mind using ImageRenderer objectwillchange or an animated Canvas. Or all the other possibilites listed.

mahal commented 3 months ago

@aure @maksutovic You may have another look at this PR. I made the suggested changes as well as added some comments to the code and this PR.