juliusHuelsmann / st

Repatch repository of developed 'Vim Browse' and 'Alpha Focus Highlight' patches for simple terminal (st).
https://st.suckless.org/patches/alpha_focus_highlight/
MIT License
28 stars 11 forks source link

background flickering issue on st v0.8.4 while using the keyboard to focus (intermittently on previous versions, including 0.8.3) #31

Closed vide0hanz closed 4 years ago

vide0hanz commented 4 years ago

Hey there, I've been using this patch across the past several versions of ST and just wanted to say first off that it is one of my favorite patches available, but I've always had an issue with intermittent flickering of the background color when ST has focus -- but only appears to flicker when moving through the stack with the keyboard to give a window focus; using the mouse to hover over and focus a window does not reproduce the flicker.

Today I tested it out on the newset version of ST 0.8.4 to see if that somehow improves things, but its a lot worse than in 0.8.3, I can't seem to nail it down as to why. The issue is acknowledged for the previous version on i3wm and appears to describe exactly what I'm experiencing in DWM:

In i3WM, the focus event is triggered twice for one specific window on a workspace (could be the root); hence the alpha values are applied twice which appears as blinking.

I still had my old build based on ST 0.8.3 and the flickering is noticeable but significantly reduced, and appears at random. It seems to affect larger terminal windows more.

I've tried adjusting my picom.conf file as described in the one you have provied: https://github.com/juliusHuelsmann/Config/blob/master/.config/picom/picom.conf -- but this doesn't appear to have any effect

Interestingly, with the same build of both dwm and st on my laptop using the same picom configuration the issue is fine - my laptop has a 60hz panel. I only notice this flickering on my desktop which is connected to a 165hz panel -- not sure if that has anything to do with it, but I have a hunch it could be related.

Do you have any ideas about why the flickering is present more on 0.8.4 over 0.8.3? Again, I still have the flickering on 0.8.3 but it is intermittent and much less frequent. On 0.8.4 it is almost constant - but only when focusing windows with my keyboard rather than the mouse.

Hoping you have some insights! Please let me know if there is any other information you need from me.

juliusHuelsmann commented 4 years ago

Hi!

Thanks! I haven't tried out v0.8.4 yet, so I wasn't aware that the flickering issue intensifies with the update. This problem does not occur a lot at my machine, so I didn't look into that extensively.

I think you're right that larger terminals are affected more often; my guess is that it takes more time to redraw the terminal, and therefore the flickering is visible more often. I currently do not know why the flickering appears more with v0.8.4, but I think this commit might be related as it changes the way chunks of the terminal are redrawn. Either way, the flickering issue is not an issue of st, it is either solvable in my patch or a bug of window managers that evoke focus events more than once.

Approaches for solving the issue

  1. there is a manual redraw in the focus function, maybe changing that to tfulldirt helps
  2. I think when I first investigated this issue, I found out that the focus function is called multiple times (switching on and off the focus multiple times when one event would suffice); finding out why that happens might be the key to fixing this issue.

I'm currently busy, and won't be able to have a look at this in the very near future. However, I just pushed a version which performs what I proposed in (1) here; on my machine I did not notice flickering with the patch, but the problem did not occur a lot at my machine in the previous version aswell, so I don't know if this changes anything. It would be awesome if you could check whether that reduces flickering in the new version of your build by applying the following patch:

patch -p1 < mightReduceFlickering.patch.txt

vide0hanz commented 4 years ago

Yeah, happy to help!

Could you double check that .diff you linked me? I manually checked it against my patched build which includes alpha, and the focus patch applied on top of it and the diff does not seem to change anything. Did you mean to remove all instances of tfulldirt or redraw? The diff suggests removing tfulldirt in all areas where there already isn't any instance of it.

Just want to be clear before I start axing things.

EDIT: Hey, I think that fixed it. The diff you linked was indeed backwards, but I was able to work it out based on your actual commits - tested on 0.8.4 and I do not appear to get any flickering at all. I tested it with various terminal apps open as well, all draw the background without any flickering whatsoever on my end. This even seems to improve the drawing of the focused bg as well, before I could occasionally see it drawing the background (probably only noticeable on a high refresh display, and you really have to be looking for it to tell).

I don't think this change should affect any other patches, will just have to use this build for a while to see but I think you can mark this one closed! Thanks again!

juliusHuelsmann commented 4 years ago

Oups sorry; yeah I made an error while generating the patch, it is indeed backwards.

Cool, I'm happy to hear that it seems to work, thanks a lot for testing! I will release a new version of the focus patch with this fix applied when I have some time on my hands and then close this issue.