gizak / termui

Golang terminal dashboard
MIT License
13.2k stars 786 forks source link

ui.TermWidth() returns incorrect value if ui.Render() has not being called first #43

Closed Ghoughpteighbteau closed 9 years ago

Ghoughpteighbteau commented 9 years ago

I was updating grid.go so that it didn't use 100% CPU, and I ran across this bug.

    go func() {
        for _ = range time.Tick(time.Second * 4) {
            update <- true
        }
    }()
    ui.Render(ui.Body)
    for {
        select {
        case e := <-evt:
            if e.Type == ui.EventKey && e.Ch == 'q' {
                return
            }
            if e.Type == ui.EventResize {
                ui.Body.Width = ui.TermWidth()
                ui.Body.Align()
                ui.Render(ui.Body)
            }
        case <-done:
            return
        case <-update:
            ui.Render(ui.Body)
        }
    }

here's the updated redraw code and you can pretty much immediately see the problem if you try this.

However, if you write it like this:

    for {
        select {
        case e := <-evt:
            if e.Type == ui.EventKey && e.Ch == 'q' {
                return
            }
            if e.Type == ui.EventResize {
                ui.Render(ui.Body)
                ui.Body.Width = ui.TermWidth()
                ui.Body.Align()
                ui.Render(ui.Body)
            }
        case <-done:
            return
        case <-update:
            ui.Render(ui.Body)
        }
    }

then it works fine. I expect ui.TermWidth() to return the current terminals width. Not the width of the terminal when ui.Render() last ran, so this was very confusing!

gizak commented 9 years ago

Weird... ui.TermWidth should return current width value. Looking into that...

Ghoughpteighbteau commented 9 years ago

It might actually be a bug with my terminal emulator incorrectly reporting its size? I should of considered that, I'll re-run the test on other emulators.

edit: so I've tested this on lxterminal and tilda, and it manifests in both of them. Manually scrolling obviouslly lessens the bug a fair big, by only making very small adjustments then re-rendering. While doing large transformations like snapping the terminal to full screen and back cause problems.

http://imgur.com/wiWR2w1,pcQnDUq,V48aGru

First image is on start with split screen. Second image is full screen. Third is back to split screen again.

edit2: you know, I just now looked through your code, are you interfacing with termbox correctly? Or is termbox at fault here?

gizak commented 9 years ago

It happens in my terminal too, resize always lags one step behind.

The Render should not be called parallel, so I used this instead:

if e.Type == ui.EventResize {
    ui.Body.Width = ui.TermWidth()
    ui.Body.Align()
    go func() { update <- true }()
}

While this did not solve the problem. Now I am checking termbox to see if there is anything I missed.