dmrschmidt / DSWaveformImage

Generate waveform images from audio files on iOS, macOS & visionOS in Swift. Native SwiftUI & UIKit views.
MIT License
978 stars 109 forks source link

Configuration given in WaveformLiveView initialiser not used in sampleLayer #58

Closed leberwurstsaft closed 1 year ago

leberwurstsaft commented 1 year ago

Hi,

unfortunately, didSet isn't being triggered during an initialiser. Therefore the sampleLayer of WaveformLiveView never gets the message about the initial configuration given via

public init(configuration: Waveform.Configuration = defaultConfiguration, renderer: WaveformRenderer = LinearWaveformRenderer())

The sample applies to the renderer.

It can be worked around by setting these properties later. It could be fixed by calling these property setters within a defer { } block in the initialiser. But only if they have a default value.

https://github.com/dmrschmidt/DSWaveformImage/blob/b3a955b03320ec54b89dc023664aef907cd92b89/Sources/DSWaveformImageViews/UIKit/WaveformLiveView.swift#L22

dmrschmidt commented 1 year ago

Hey @leberwurstsaft, love the name fyi 😂 (did I say this before already? 🤔)

What's the concrete issue that results from what you've described. You're right, those didSet aren't called but they should be. Makes sense. I'm wondering why I haven't observed any bugs from that so far.

So essentially this is the fix you're proposing?

public init(configuration: Waveform.Configuration = defaultConfiguration, renderer: WaveformRenderer = LinearWaveformRenderer()) {
    defer { // trigger didSet to propagate to sampleLayer
        self.configuration = configuration
        self.renderer = renderer
    }

    // set required properties initially
    self.configuration = configuration
    self.renderer = renderer
    super.init(frame: .zero)
    self.contentMode = .redraw
}
leberwurstsaft commented 1 year ago

Sorry, forgot to reply. The concrete issue was that I wanted to use the striped style, and got the normal linear style instead.

There was a confusing typo in the original message:

The sample applies to the renderer.

should be

The same applies to the renderer.

The fix above seems about right.

dmrschmidt commented 1 year ago

I totally slept on this one... Took way too long, sorry about that.

I've just pushed 12.0.1.