add fractional scale protocol support

[nglibc]

On compositors supporting the fractional scale and viewporter protocols, scales the terminal font size to reflect the fractional scale setting. On compositors not supporting these protocols, scaling is set to 1.0x. If fractional scale is changed dynamically (eg. using wlr-randr), de-init of the old font size and init of the new font size occurs in response to fractional scale events.

[ii8]

This segfaults if I change my output scale. For example if I set my output scale factor to 2 and restart my monitor, havoc crashes. At a glance I'm not sure what the cause is, but presumably a use after free with the buffers or font.

[ii8]

Why is the default preferred scale 2?

[nglibc]

Thanks for spotting this. It should be 120, I was just testing it with 240 to ensure other default scales work correctly. I will change it to 120 on the next re-submission of this pull request.

[ii8]

buffer_init already prints an error when it fails, this doesn't really add anything useful

[nglibc]

Noted, thanks for the clarification.

[ii8]

This does not return the correct coordinates because the margin is already scaled

[nglibc]

Thanks for picking up this issue, new pull request will avoid re-scaling the margin.

[ii8]

see above

[ii8]

This unnecessarily unmaps the whole font file. Only the cached glyphs need to be re-rendered.

[nglibc]

Noted, but is there a way to invalidate the cached glyphs without using "delete_cache(font.cache)" (which will access the presumably private cache field of the font struct)?

[ii8]

The way to do it is probably to add a font_resize function that will delete the old cache, change the font size values and reinitialise the cache.

Ideally the font code should be changed so that multiple fonts can be loaded at once so that for example a bold font can be used in addition to the normal font, but I suppose that's not strictly needed for this fractional scaling.

Is this here not the cause of the segfault too, because the font size gets changed but there's no guarantee that the buffers will be resized before the next frame callback.

[ii8]

This will allocate oversized buffers when the margin is disabled.

[nglibc]

I will send a new pull request which will not oversize the buffers (adds a couple of lines of extra code in complexity but avoids wasting memory on larger buffers)

[ii8]

Hmm, we might still want to support older compositors, will need to look into it.

[nglibc]

This segfaults if I change my output scale. For example if I set my output scale factor to 2 and restart my monitor, havoc crashes. At a glance I'm not sure what the cause is, but presumably a use after free with the buffers or font.

Can I check which compositor you are using? I have so far tested on dwl and labwc, and also tinywl (a compositor not supporting viewporter/fractional scale, to test the standard 1.0x scaling).

Also, I'll look into whether fs_preferred_scale should set term.configured = false - as there is a theoretical possibility a compositor can send fractional_scale events without a toplvl_configure event. (This means fs_preferred_scale can change the glyph dimensions without ensuring recalculation of rows & cols).

[ii8]

I'm on sway

[nglibc]

I'm on sway

I've just now done testing with sway 1.11, compiled using default options and changing the scale (via editing sway config file and reloading using mod + shift + c and also via wlr-randr) but can't trigger any segfaults. Which version of sway are you using, and do you have gdb?

If so, if you compile havoc-fs using -g, and run gdb within either a non-fractionalscale havoc or another terminal (so you can see gdb output), gdb should help with finding out where the segfault is occurring.

I have also added a wl_surface_commit() to the fractional scale event handler function, so this ensures that wayland will call toplvl_configure event for every change in fractional scale. I'll repost the new pull request soon.

[ii8]

fs_preferred_scale is called from toplvl_configure in the default case, and now we want to trigger toplvl_configure from fs_preferred_scale, something is confused here.

I also don't see why committing the surface here would necessarily cause a configure event.

[nglibc]

toplvl_configure calls fs_preferred_scale only in the case where a fractional scale hasn't been set yet, and this guards against the case where toplvl_configure events occurs before the 1st fractional scale event. On the other hand, fs_preferred_scale doesn't directly call toplvl_configure, it relies on the compositor to do it after a fractional scale change has occurred and after the current surface is committed. (Hence where a fractional scale change hasn't occurred there won't be an endless cycle between these two functions).

The word "trigger" in the comment is my own choice wording, but comes from my understanding of the surface lifecycle (https://wayland-book.com/surfaces-in-depth/lifecycle.html). The fractional scale event isn't covered in the Wayland Book, but the Book does state that the current scale (integer and resumably also fractional) is part of the current surface state.

My understanding is that Wayland isn't able to sending any configure events until the current surface (which was drawn or part-drawn before the fractional scale event occured) is flushed out. Once the current surface is flushed, Wayland is then able to reconfigure for the new scaling geometry, including sending out the relevant configure events.

Unfortunately, fractional_scale_v1 has very little documentation on this, and so it might be necessary to test with various compositors, to verify their behaviour against the above.

[nglibc]

To avoid any risk of non-atomic changes to font size vs dimensions, I have now changed fs_preferred_scale to store to a new variable, term.confscale. Only when toplvl_configure is called will it now take the preferred scale, resize the font and recalculate the buffer dimensions and make the new scale active. A new pull request (#3) will have this change.

[ii8]

That's not how it works, the preferred_scale event is not part of the surface configuration state managed by the configure event, so the new scale can just be used in the next committed buffer. The book is talking about the scale the client has set for a surface, which is double buffered, not the preferred scale event.

Calling wl_surface_commit will not necessarily cause the compositor to send a configure event here.

[ii8]

Why do this? We are using viewport to scale it no?

[ii8]

Why not create the correct size buffer and use the whole buffer?

[nglibc]

It is a limitation of the viewporter protocol, which is overly conservative in it's bound checking and requires the source buffer to be sized to the (ceiling) integral multiple of the destination size, even when only a subset of this buffer is used. In previous versions of havoc-fs (not committed to my fork's github repo), I did try using a fractionally sized source buffer but viewporter reports an out of bounds error. The fractional scale protocol appears to only be used by Wayland to improve image quality (but not to reduce memory usage) compared to integer scaling.

[ii8]

No, the buffer can be any size, something else must have been the cause of the error you saw, this unnecessarily uses up to 4x more memory for the buffers.

[ii8]

See above

[ii8]

Why have an integer buffer scale, can't we just create the buffer with the correct fractional size?

[nglibc]

It's a limitation of the viewporter protocol. I don't know enough about the internal implementation of viewporter to add any further detail, but it might have something to do with Wayland's history of only supporting integer scaling before then adding fractional scaling. Or it might be to prevent multiple buffer memory resizing occurring in response to the 'zoom' level changing by tiny amounts at a time (eg. in a fancy UI with zoom animations, maybe there might a use case of viewporter being used to draw the various levels of zoom - and by having an over-scaled buffer, it means that only the pixels inside the buffer need to be redrawn, and the buffer memory size itself doesn't need to change). This is all speculation on my part, as no zoom animation is done currently.

[ii8]

That's not how it works, the buffer can be any size.

[ii8]

Why scale back and forth here, isn't it all in buffer coordinates?

[nglibc]

I use confwidth & confheight to communicate window dimensions between the two configure functions. The question is then whether to pre-multiply by the buffer scale or not. This is an arbitrary choice but is done in integer arithmetic (hence is exact, with no rounding errors upon dividing the buffer scale back out).

The other choice could have been not to pre-multiply by buffer scale but I went with pre-multiplying because in nearly all cases where there is a division by m, there is also multiplication by s or division by cw or ch, so this has the effect of encapsulating the scale calculations to fewer lines of code. In contrast, if I didn't pre-multiply, then many simple assignment lines (eg. term.width = term.confwidth) became cluttered up by scaling calculations, and then almost every line of code involving dimensions ends up also needing either a 'm' or a 's' scale factor.

[ii8]

You can just use the actual size of the terminal here so it's all in buffer coordinates, you don't need confwidth. Without scaling they were equal so it didn't matter before.

[ii8]

Why is this needed?

[nglibc]

This ensures the buffer is never smaller than 2 glyphs high & wide. The choice of two is a bit arbitrary but buffer should never be smaller than 1 glyph as otherwise it gets rounded down to zero, which causes segfaults. Looking at it again, the minimum size (3rd operand of conditional operator) should be cw * 2 / s, not cw * 2. I will resumbit the pull request for this once any other changes need have all been decided. The other change to consider is whether the following in configure() is no longer needed due to the 2x2 glyph minimum:

if (col == 0 || row == 0)
	return;

[ii8]

Which version of sway are you using, and do you have gdb?

I'm on sway 1.11. gdb does not reveal a consistent source location where the segfault happens unfortunately.

I can cause the segfault by using swaymsg -t command output DP-1 scale 2 and then restarting my monitor.

[ii8]

Another thing I wonder: is there any point in supporting the wl_surface integer scaling method at all? Compositor scaling only is probably good enough for when fractional scaling is not available. Does it make sense to have 3 different scaling methods?

[nglibc]

Another thing I wonder: is there any point in supporting the wl_surface integer scaling method at all? Compositor scaling only is probably good enough for when fractional scaling is not available. Does it make sense to have 3 different scaling methods?

Actually, of the 3 options (no scaling, integer scaling by receiving wl_output events, and receiving preferred fractional scaling events), I chose to omit the middle one (wl_output scaling). wl_output scale has a different scale for each monitor, and if we were to handle dynamic re-scaling (ie. new monitor added with a different scale, or change of scale for existing monitor) it would be the most complex option (eg. consider what happens if a surface moves from one monitor to another monitor with different scale).

You could argue the above is a strawman case, and the steelman case is to calculate a single preferred integer scale to use across all monitors. But if we are doing that, then it is actually cleaner to just use the the preferred fractional scale protocol to effectively calculate a single scale for all monitors for me, rather than checking the scale of each individual monitor myself.

Also, note that integer scaling wastes just as much memory (because scale is rounded up to nearest integer) as the current inefficient/over-conservative viewporter bounds checking for fractional scaling.

[nglibc]

Which version of sway are you using, and do you have gdb?

I'm on sway 1.11. gdb does not reveal a consistent source location where the segfault happens unfortunately.

I can cause the segfault by using swaymsg -t command output DP-1 scale 2 and then restarting my monitor.

I am now running 'swaymsg -t command output eDP-1 scale 2' and it rescales immediately without needing to restart my monitor. What do you do to restart your monitor (and do you use more than 1 monitor)?

Also, does havoc rescale work if you put a line in your swayconfig like the following, start sway and then change the scale value in the config file with a text editor to a new value (eg. 2.0), and trigger a config reload with MOD + shift + c?

output * scale 1.4

Can you show the sway verbose output (using -d on starting up sway)? For example, this is what I get when I run swaymsg on X11 backend:

00:00:36.913 [sway/ipc-server.c:196] New client: fd 40
00:00:36.913 [sway/commands.c:261] Handling command 'output X11-1 scale 2'
00:00:36.913 [sway/commands.c:436] Subcommand: scale 2
00:00:36.913 [sway/config/output.c:272] Config stored for output X11-1 (enabled: -1) (-1x-1@-1.000000Hz position 2147483647,2147483647 scale 2.000000 subpixel unknown transform -1) (bg (null) (null)) (power -1) (max render time: -1) (allow tearing: -1)
00:00:36.913 [sway/config/output.c:880] Committing 1 outputs
00:00:36.913 [sway/config/output.c:643] Output state for X11-1
00:00:36.913 [sway/config/output.c:645]     enabled:       yes
00:00:36.913 [sway/config/output.c:649]     render_format: XR24
00:00:36.913 [sway/config/output.c:667]     scale:         2.000000
00:00:36.913 [sway/config/output.c:670]     subpixel:      unknown
00:00:36.913 [wlr] [types/wlr_output_swapchain_manager.c:160] Preparing test commit for 1 outputs with explicit modifiers
00:00:36.913 [wlr] [types/wlr_output_swapchain_manager.c:181] Test commit for 1 outputs succeeded
00:00:36.916 [sway/config/output.c:940] Commit of 1 outputs succeeded
00:00:36.916 [sway/config/output.c:946] Finalizing config for X11-1
00:00:36.916 [sway/desktop/layer_shell.c:96] Usable area changed, rearranging output
00:00:36.916 [sway/tree/arrange.c:274] Usable area for ws: 512x384@0,0
00:00:36.916 [sway/tree/arrange.c:308] Arranging workspace '1' at 0.000000, 0.000000
00:00:36.916 [sway/tree/arrange.c:76] Arranging 0x7ffd3b351420 horizontally
00:00:36.916 [sway/tree/arrange.c:274] Usable area for ws: 512x384@0,0
00:00:36.916 [sway/tree/arrange.c:308] Arranging workspace '1' at 0.000000, 0.000000
00:00:36.916 [sway/tree/arrange.c:76] Arranging 0x7ffd3b351460 horizontally
00:00:36.916 [sway/desktop/transaction.c:821] Transaction 0x1185ac80 committing with 2 instructions
00:00:36.916 [sway/input/seat.c:694] Applying input mapping to 0:0:x11-touch
00:00:36.916 [sway/input/seat.c:740] Mapping input device 0:0:x11-touch to output X11-1
00:00:36.916 [sway/input/seat.c:760] Mapped to output X11-1
00:00:36.916 [sway/input/seat.c:694] Applying input mapping to 0:0:x11-pointer
00:00:36.916 [sway/input/seat.c:740] Mapping input device 0:0:x11-pointer to output X11-1
00:00:36.916 [sway/input/seat.c:760] Mapped to output X11-1
00:00:36.931 [wlr] [xcursor/wlr_xcursor.c:224] Loaded cursor theme 'default' at size 48 (92 available cursors)
00:00:36.933 [wlr] [render/swapchain.c:101] Allocating new swapchain buffer
00:00:36.933 [wlr] [render/allocator/gbm.c:146] Allocated 48x48 GBM buffer with format AR24 (0x34325241), modifier GFX11,64KB_R_X,PIPE_XOR_BITS=2,PACKERS=2,DCC,DCC_MAX_COMPRESSED_BLOCK=128B,DCC_INDEPENDENT_128B,DCC_PIPE_ALIGN (0x020000001046BB04)
00:00:36.933 [wlr] [render/gles2/renderer.c:157] Created GL FBO for buffer 48x48
00:00:36.934 [sway/ipc-server.c:570] IPC Client 40 disconnected
00:00:36.942 [sway/desktop/transaction.c:907] Transaction 0x1185ac80 is ready
00:00:36.942 [sway/desktop/transaction.c:717] Applying transaction 0x1185ac80
00:00:38.155 [wlr] [render/swapchain.c:101] Allocating new swapchain buffer
00:00:38.155 [wlr] [render/allocator/gbm.c:146] Allocated 48x48 GBM buffer with format AR24 (0x34325241), modifier GFX11,64KB_R_X,PIPE_XOR_BITS=2,PACKERS=2,DCC,DCC_MAX_COMPRESSED_BLOCK=128B,DCC_INDEPENDENT_128B,DCC_PIPE_ALIGN (0x020000001046BB04)
00:00:38.156 [wlr] [render/gles2/renderer.c:157] Created GL FBO for buffer 48x48
00:01:06.361 [sway/ipc-server.c:196] New client: fd 43
00:01:06.361 [sway/commands.c:261] Handling command 'output X11-1 scale 1.8'
00:01:06.361 [sway/commands.c:436] Subcommand: scale 1.8
00:01:06.361 [sway/config/output.c:272] Config stored for output X11-1 (enabled: -1) (-1x-1@-1.000000Hz position 2147483647,2147483647 scale 1.800000 subpixel unknown transform -1) (bg (null) (null)) (power -1) (max render time: -1) (allow tearing: -1)
00:01:06.361 [sway/config/output.c:880] Committing 1 outputs
00:01:06.361 [sway/config/output.c:643] Output state for X11-1
00:01:06.361 [sway/config/output.c:645]     enabled:       yes
00:01:06.361 [sway/config/output.c:649]     render_format: XR24
00:01:06.361 [sway/config/output.c:667]     scale:         1.800000
00:01:06.361 [sway/config/output.c:670]     subpixel:      unknown
00:01:06.361 [wlr] [types/wlr_output_swapchain_manager.c:160] Preparing test commit for 1 outputs with explicit modifiers
00:01:06.361 [wlr] [types/wlr_output_swapchain_manager.c:181] Test commit for 1 outputs succeeded

[ii8]

I think you've gotten a few things confused here, I'm not sure why most of the maths you are doing is really necessary.

Here is roughly what I think would be needed for fractional scaling: 4654464
The buffers are only as big as they need to be and the font is only loaded once.

I haven't tested this so it might not work properly but it should show what I mean.

[nglibc]

I have tested your solution on sway and dwl.

In sway it is producing nicely rendered text (see top window in screenshot).

In dwl the approach you have taken is caused text to be blurred - but correctly sized (see second window in screenshot). The third window in the screenshot is dwl + my havoc-fs, which does produce clear text rendering. (I did all this on wayland with an X-windows backend, so that I can do a screenshot of all 3 side by side. But when I render wayland + your havoc directly via drm backend - ie: eDP-1 output, I get the same blurred text).

[ii8]

dwl sends the preferred scale event before the surface is configured. fix: 4211859

[nglibc]

I tested on labwc and dwl, and fonts are being rendered well.
Your approach of setting the destination viewport size only, and not changing the buffer scale is good and cleanly bypasses the need to set an integer buffer scale (which is in line with supporting only fractional scaling, and not integer output scaling).

[ii8]

The main issue that remains then is when the margin is disabled the buffer might not cleanly divide by the scale factor to produce a logical surface size for the viewport destination. Unfortunately there is no way to fix this properly because it's an inherent problem with the wayland protocol. One fix could be to add transparent pixels to increase the size of the buffer, but this would then offset decorations and look wonky there instead.

The difference in sharpness is quite subtle:

If I didn't know about the problem perhaps I would not even notice it. It also only happens at specific scale and buffer size combinations, and it never happens when margin is enabled because then we use the surface size advertised by the compositor. So maybe we can just leave it as is, but I'm not too happy about having half a solution like this.

The other remaining thing is scaling the cursor surface.

[nglibc]

The main issue that remains then is when the margin is disabled the buffer might not cleanly divide by the scale factor to produce a logical surface size for the viewport destination. Unfortunately there is no way to fix this properly because it's an inherent problem with the wayland protocol. One fix could be to add transparent pixels to increase the size of the buffer, but this would then offset decorations and look wonky there instead.

The difference in sharpness is quite subtle: If I didn't know about the problem perhaps I would not even notice it. It also only happens at specific scale and buffer size combinations, and it never happens when margin is enabled because then we use the surface size advertised by the compositor. So maybe we can just leave it as is, but I'm not too happy about having half a solution like this.

The other remaining thing is scaling the cursor surface.

What combinations of buffer size and scale did you get that problem with?

[ii8]

I can't remember exactly, the buffer size depends on your font anyway so it's hard to replicate, but the larger the scale factor the more it happens. You can set scale to 2x, set margin to no, resize the window around and you'll see it.

[ii8]

#64

[nglibc]

I can't remember exactly, the buffer size depends on your font anyway so it's hard to replicate, but the larger the scale factor the more it happens. You can set scale to 2x, set margin to no, resize the window around and you'll see it.

It get good font rendering no matter what dimensions I resize the havoc window to on sway, dwl and on labwc (amdgpu graphics driver in all 3 cases). Do you have any guess on what is causing it in your case?