Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intermittent screen flashes #576

Closed
palerdot opened this issue Nov 5, 2022 · 10 comments
Closed

Intermittent screen flashes #576

palerdot opened this issue Nov 5, 2022 · 10 comments

Comments

@palerdot
Copy link

palerdot commented Nov 5, 2022

Firstly, I would like to convey my thanks for the tcell library.

I'm facing an issue, where I'm seeing intermittent tiny screen flashes during repaint/Sync call. I have built a web like terminal wordle, where I have to animate the guess letter colors. When animating the letter colors of the guess word, I'm seeing intermittent screen flashes. I have attached a gif screenshot at the end of this issue. In the screenshot, you can see dark blue flashes when the word is animated. The dark blue is my actual terminal background color. I would like to know if there is something I should do when calling s.Sync() to prevent these flashes.

The relevant part of code can be seen here - https://github.com/palerdot/wordl/blob/master/screen/screen.go#L115
Full source code can be seen here - https://github.com/palerdot/wordl

I tried looking for documentation but unable to find them. tcell is my only dependency, and I thought of seeking help here. It would be great if someone from tcell team can clarify if anything can be done to prevent these intermittent flashes.

wordl-screen-flash

@n-peugnet
Copy link

n-peugnet commented Nov 16, 2022

We had a similar problem in gomuks. We temporarily fixed it by reverting 62f5502.

Maybe you can first try tulir's tcell fork to see if the issue persist (this can easily be done with a "replace" in go.mod).

As gomuks' code was dependant on some recent commits of tcell, we didn't do a real git bisect to find the source of this issue, I only tried to revert some commits. So if you code is simpler, it may be a good opportunity to bisect tcell on your end (replace can also be used to use a local version of a lib), to find since which commit this issue appears.

@gdamore
Copy link
Owner

gdamore commented Nov 16, 2022

Looking at that fork it looks like they have changed to address something about Clear. If you call that a lot it could be bad.

Also Sync is probably not what you want to do. Sync should only be used to repair screen damage because it assumes everything is messed up and it clears the screen.

Show() should be used instead.

@n-peugnet
Copy link

@gdamore:

Looking at that fork it looks like they have changed to address something about Clear.

Whoops, you are right I did read this issue too quickly.

If you call that a lot it could be bad.

We may indeed call Clear() too often in gomuks, but with a previous version of tcell we cannot notice any screen flashes (and with said commit reverted neither). Should I open another issue for this specific problem?

@liweiyi88
Copy link
Contributor

I have a similar issue with my snake game. Calling .Clear() in every time tick to re-draw the board and state. I didn't have the "Intermittent screen flashes" until recently upgrade the package to the latest version

@liweiyi88
Copy link
Contributor

Also can confirm, this can be temporarily fixed by commenting the code

func (t *tScreen) Clear() {
	t.Fill(' ', t.style)
	// t.Lock()
	// t.clear = true
	// w, h := t.cells.Size()
	// // because we are going to clear (see t.clear) in the next cycle,
	// // let's also unmark the dirty bit so that we don't waste cycles
	// // drawing things that are already dealt with via the clear escape sequence.
	// for row := 0; row < h; row++ {
	// 	for col := 0; col < w; col++ {
	// 		t.cells.SetDirty(col, row, false)
	// 	}
	// }
	// t.Unlock()
}

@gdamore
Copy link
Owner

gdamore commented Dec 30, 2022

So the theory was that clearing the screen was faster, but it does force a redraw and can be visually disconcerting.

Applications that use Clear() to reset the double buffered state, but don't actually meant to clear the screen are using this API probably in error, but it's not unreasonable that they might do so.

Probably we need to enhance the API or make a new one so that applications can indicate whether they want a "hard clear" or a "soft clear".

@gdamore
Copy link
Owner

gdamore commented Dec 30, 2022

Actually, Apps should just use t.Fill(' ', style).

@gdamore
Copy link
Owner

gdamore commented Dec 30, 2022

I think what you want is

screen.Fill(' ', tcell.StyleDefault)

I can make an API that does just that, but it really would be nothing more than a very small convenience function.

@gdamore
Copy link
Owner

gdamore commented Dec 30, 2022

I will push a document fix for this though.

@gdamore
Copy link
Owner

gdamore commented Dec 30, 2022

Actually, now that I think about it with, double buffering in play, I think the entire screen clearing logic is misguided.

The goal was to make the initial redraw faster, and certainly that is the case, but it's both jarring in some use cases, and in cases where the performance will be impacted it should not be called often. Essentially the only real use case for this is during initial program startup where we want to discard the entire contents of the screen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants