ii8 / havoc

minimal terminal emulator for wayland
Other
108 stars 18 forks source link

Allocate & update buffer based on full window size #6

Closed MichaelMackus closed 5 years ago

MichaelMackus commented 5 years ago

This seems to solve #5 for the top and left sides of the window, but not for the bottom or right. It might be as simple to just blank out the remainder of the screen - from the input prompt (last row) until the bottom of the window, and from the very last drawn column to the right window edge.

ii8 commented 5 years ago

Hey, thanks for the detailed report and patch! The behavior you're observing is actually a combination of two issues:

The ugly border in tiling layouts: This happens because havoc may not adhere exactly to the size that the compositor configures it to be which is sometimes desirable: for example if you have a floating wm or tiling with gaps and no borders you might want the terminal width/height to be an exact multiple of the cell width/height so you don't get a gap between the actual content within the terminal and the edge of the window.

In your case, where window edges touch directly, you probably want the margin to be there so that the terminal window size is exactly what the compositor expects. So the correct solution is to add a config option to turn the margin on and off.

Unfortunately you cannot simply change the size of the buffer, the margin has to be rendered to fill the extra space around the cells, and the cells have to be rendered with an offset so they end up centered in the window.

Text not rendering correctly on the bottom and right edge: This, I believe, is caused by incorrect damage handling in sway, I've not opened an issue over there because I've not confirmed this yet. It looks like sway applies damage relative to the tile container instead of the surface buffer, so this only happens if the buffer size is different from smaller than the configured size.

ii8 commented 5 years ago

I tried to verify whether the 2nd issue lies with havoc or with sway by testing weston-terminal(which also snaps to multiples of cell size), but it just makes itself larger than the tile container:

ss

lol

ii8 commented 5 years ago

https://github.com/swaywm/sway/pull/4252

MichaelMackus commented 5 years ago

Interesting, I have not seen that behavior in sway.

I was under the impression the math for allocating the buffer may be wrong. The term.width variable is the result of an implicitly cast integer division between the window width and the glyph (font) size (see line 939 of main.c). This is the same for the height. Therefore, the buffer is usually smaller than the actual size of the window. Thus, in this PR I just allocate the buffer to the entire window size and it "fixes" the visual glitch. This is similar to how wterm seems to draw the window, from what I can tell.

I'm not too familiar with how wayland handles this, though. Is this defined behavior in the wayland spec? It seems to me, when you mark the window as damaged, you mark from 0,0 to the term.width*term.height - which (as noted above) could be less than the entire size of the window.

MichaelMackus commented 5 years ago

See my latest commit. This seems to solve my original issue & fully draws the background of the entire window. I'm going to test in weston to ensure the default window sizes isn't messed up by this.

MichaelMackus commented 5 years ago

Havoc won't launch for me in weston (complaining about something related to xdg shell), but the newest commit works fine when adjusting the default window size in the havoc.cfg - in sway floating windows still spawn by default with the configured window size.

ii8 commented 5 years ago

Maybe your weston is out of date, the latest weston should have the stable xdg shell I think.

When applying damage in wayland it's applied to a buffer and is unrelated to the term.confwidth/term.confheight variables(or at least should be), those are just hints from the compositor.

term.width is intentionally smaller than term.confwidth, this is allowed in the wayland protocol. It's done here to avoid a gap between the cells in the terminal and the edge of the surface. For example, this: pic ...can look like this instead in havoc: shot (no gap below the blue status line at the bottom)

This means the gap is on the outside of the buffer instead, if you have tile containers/borders like you do then it becomes noticeable.

So we need a config option to choose whether we should render an extra margin(like your commit does) or not.

ii8 commented 5 years ago

Also, the margin should be on all sides equally, not just bottom and right; and it doesn't need to be rendered on every redraw, only once when the buffer is created.

MichaelMackus commented 5 years ago

Thank you, that makes a lot more sense! My PR is more of a PoC, but if I find more time I will try to make the updates you mentioned.

On Fri, Jun 14, 2019, 5:12 PM Murray notifications@github.com wrote:

Also, the margin should be on all sides equally, not just bottom and right; and it doesn't need to be rendered on every redraw, only once when the buffer is created.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ii8/havoc/pull/6?email_source=notifications&email_token=AAFT7CVQVV2P57PT2PRQ25LP2QXVXA5CNFSM4HXWBCQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXYLYSA#issuecomment-502316104, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFT7CTBO6CWNCSQAH4K36DP2QXVXANCNFSM4HXWBCQA .

ii8 commented 5 years ago

@MichaelMackus do you still want to do this one? If not, I'll give it a go.

MichaelMackus commented 5 years ago

My hands are currently full with other wayland stuff, feel free!

ii8 commented 5 years ago

2dfb08bb68bb5a0a8bc5a0fea7d3b1804eefd90c

Also, forgot to say this; be aware while working on your wayland endeavors that you should not use the values from the toplevel configure event(term.confwidth/heigth in this code) until configure has been called on the surface as well. See the xdg-shell protocol for details.