migueldeicaza / SwiftTerm

Xterm/VT100 Terminal emulator in Swift
MIT License
978 stars 145 forks source link

TerminalView does not respect frame height on macOS Sonoma #330

Closed nkleemann closed 5 months ago

nkleemann commented 1 year ago

As you can see from the screen recording, the LocalProcessTerminalView exceeds the frame height when it's displayed text exceeds the vertical height of the frame.

I'm targeting macOS Sonoma, using SwiftUI. I can also confirm that the same code worked an hour ago on macOS Ventura.

Small GIF showing the bug: Screen Recording

How I'm creating the View:

struct SuperColliderProcessView: NSViewRepresentable {

    @EnvironmentObject var superColliderProcessManager: SuperColliderProcessManager

    func makeCoordinator() -> Coordinator {
        Coordinator(self)
    }

    func makeNSView(context: Context) -> LocalProcessTerminalView {
        let terminalView = LocalProcessTerminalView(frame: CGRect(x: 0, y: 0, width: 300, height: 90))        
        return terminalView
    }

    func updateNSView(_ nsView: LocalProcessTerminalView, context: Context) {}

    class Coordinator: NSObject {
        var parent: SuperColliderProcessView
        init(_ parent: SuperColliderProcessView) {
            self.parent = parent
        }
    }
}

// From SwiftUI

SuperColliderProcessView()
    // even with these two, on or the other - the terminalview doesnt respect the set height
    .frame(width: 300, height: 90)
    .frame(maxHeight: 90)

Edit: Another angle on this bug:

Screen Recording

Maybe it's linked to some Scroll Behaviour changes in Sonoma?

nkleemann commented 9 months ago

@migueldeicaza I found the commit that introduces this Bug in Sonoma, its the deadlock fix here: https://github.com/migueldeicaza/SwiftTerm/commit/e2c374952c77f5f43db7765f5e9f0cf47604c7b0

Maybe a problem related to SwiftUI's drawing system, in relation to background threads?

Here is the self-contained example to recreate this bug. On macOS Sonoma, run this on any SwiftTerm version 1.2.2 and up:

import SwiftUI
import SwiftTerm

struct ContentView: View {

    @EnvironmentObject var replViewManager: REPLViewManager
    @State var redraw: Bool = false

    var body: some View {

        VStack {

            Text("VStack Top Text")

            REPLView()

            Button {
                self.redraw.toggle()
                replViewManager.sendTextToREPL()
            } label: {
                Text("feed text to terminal")
            }
        }
        .padding()
    }
}

class REPLViewManager: ObservableObject {

    private var replView: LocalProcessTerminalView?

    func setupREPLView(_ view: LocalProcessTerminalView) {
        view.font = .monospacedSystemFont(ofSize: 10, weight: .regular)
        replView = view
        replView?.startProcess(executable: Constants.python3Path)
    }

    func sendTextToREPL() {
        guard let repl = replView else {
            print("REPLView is not initialized")
            return
        }

        repl.send(txt: Constants.blockContent)
    }
}

struct REPLView: NSViewRepresentable {

    @EnvironmentObject var replViewManager: REPLViewManager

    func makeCoordinator() -> Coordinator {
        Coordinator(self)
    }

    func makeNSView(context: Context) -> LocalProcessTerminalView {

        let view = LocalProcessTerminalView(frame: .init(x: 0, y: 0, width: 200, height: 200))

        replViewManager.setupREPLView(view)
        return view
    }

    func updateNSView(_ nsView: LocalProcessTerminalView, context: Context) {}

    class Coordinator: NSObject {
        var parent: REPLView

        init(_ parent: REPLView) {
            self.parent = parent
        }
    }
}

struct Constants {
    static let python3Path = "/usr/bin/python3"
    // dummy python code to send into repl
    static let blockContent = """
    import time

    def is_prime(n):
        if n <= 1:
            return False
        elif n <= 3:
            return True
        elif n % 2 == 0 or n % 3 == 0:
            return False
        i = 5
        while i * i <= n:
            if n % i == 0 or n % (i + 2) == 0:
                return False
            i += 6
        return True

    def print_first_n_primes(n):
        count = 0
        num = 2
        while count < n:
            if is_prime(num):
                print(num)
                count += 1
            num += 1

    start = time.time()
    print_first_n_primes(10000)
    end = time.time()
    print(f'Time taken: {end - start} seconds')
    """
}

@main
struct FreezingREPLApp: App {

    @StateObject var replViewManager = REPLViewManager()

    var body: some Scene {
        WindowGroup {
            ContentView()
                .environmentObject(replViewManager)
        }
    }
}

After clicking the button, the LocalProcessTerminalView expands out of the Frame given by SwiftUI:

Screenshot 2024-01-26 at 08 55 31

migueldeicaza commented 9 months ago

Thank you for putting together a test case, I will investigate this weekend

lkamil commented 6 months ago

Any updates on this? I'm having the same issue

nkleemann commented 5 months ago

I fixed it with this PR, it's just a simple flag that needs to be set.