Skip to content

wayland: directly use surface local coords for viewport when possible #17416

Open
Dudemanguy wants to merge 2 commits intompv-player:masterfrom
Dudemanguy:fullscreen-output
Open

wayland: directly use surface local coords for viewport when possible #17416
Dudemanguy wants to merge 2 commits intompv-player:masterfrom
Dudemanguy:fullscreen-output

Conversation

@Dudemanguy
Copy link
Member

@Dudemanguy Dudemanguy commented Feb 16, 2026

Unfortunately due to the way fractional scaling on wayland works, it can lead to rounding errors when calculating the physical size of the buffer. It's convoluted but essentially the compositor will communicate the logical size back to us. Depending on the fractional scale factor, the communicated size can be truncated in a significant enough way that causes final calculated size to be off by a maximum of two (truncation at both steps).

In general, there's nothing we can really do about this but for specifically fullscreen there's an easy enough workaround. Just set the size to be the same as the output. Since we might not have rendered the surface yet (and thus not have an easily accessible wl->current_output), it's easier to just loop through the output list until we have a match.

@na-na-hi
Copy link
Contributor

I think this has to be fixed in Wayland spec. Any workaround will have lots of issues.

xdg_toplevel::state.fullscreen spec says:

The surface is fullscreen. The window geometry specified in the configure event is a maximum; the client cannot resize beyond it. For a surface to cover the whole fullscreened area, the geometry dimensions must be obeyed by the client. For more details, see xdg_toplevel.set_fullscreen.

There are several issues here:

  • The maximum is not guaranteed to be identical to output geometry. For example, a compositor can have an "immersive" fullscreen mode with blurred border surrounding the fullscreened content being displayed. The content will be smaller than output geometry.
  • Even if they are identical, when output geometry is smaller than window geometry, this violates the "the client cannot resize beyond it" condition.
  • This also can make wp_viewport::set_destination not using the correct size and results in surface scaling by compositor, which we do not want.
  • The destination size can also result in a scaled size that is smaller than output size which would result in video not completely filling the screen.

@Dudemanguy
Copy link
Member Author

I'm not really concerned about a theoretical compositor that enforces a 2 pixel wide border on fullscreen for whatever reason.

This also can make wp_viewport::set_destination not using the correct size and results in surface scaling by compositor, which we do not want.

Inherently unavoidable. There will potentially be a 1 or 2 pixel compositor-side scale. Nothing we can do about that. Note the status quo is wrong anyway.

The destination size can also result in a scaled size that is smaller than output size which would result in video not completely filling the screen.

I would be concerned about this one however. Can you provide an example of this happening?

@na-na-hi
Copy link
Contributor

Inherently unavoidable. There will potentially be a 1 or 2 pixel compositor-side scale. Nothing we can do about that. Note the status quo is wrong anyway.

It should be avoidable if the viewporter source size is representable with an integer times the scale rounded halfway away from zero, as specificed by the fractional scale spec. And I thought the rounding error was fixed by #14489.

I would be concerned about this one however. Can you provide an example of this happening?

Suppose the output has a pixel size of 1000 with a fractional scale of 3. If the compositor sends a toplevel size of 334, then you are aware of the 2 pixel excess and can render an image confined within the 1000 pixel region. If you instead use the pixel size of 1000 to calculate the destination size, it will be 333, which only scales to 999 pixels, leaving 1 pixel gap.

And it can go the other way too. Suppose the output has a pixel size of 1001 with a fractional scale of 3. If the compositor sends a toplevel size of 333, then it intends to maintain a 2 pixel gap. If you instead use the pixel size of 1001 to calculate the destination size, it will be 334, which violates the spec ("the client cannot resize beyond it").

@kasper93
Copy link
Member

kasper93 commented Feb 17, 2026

I'm not really concerned about a theoretical compositor that enforces a 2 pixel wide border on fullscreen for whatever reason.

This also can make wp_viewport::set_destination not using the correct size and results in surface scaling by compositor, which we do not want.

Inherently unavoidable. There will potentially be a 1 or 2 pixel compositor-side scale. Nothing we can do about that. Note the status quo is wrong anyway.

This is actually critical issue to ensure that it works correctly or at least try. I was thinking about this yesterday, but na-na-hi was faster to bring this up.

Scaling by 1 or 2 pixels will introduce resampling blur to mpv window. All the "high quality" processing in mpv would be thrown away.

We have to make sure that wp_viewport::set_source and wp_viewport::set_destination have the same size. Also I'm not sure what happens, if we set smaller size than "preferred" by window, but as I understand it would add border(?) and render surface at this size. Reading documentation, we probably should not set destination at all, unless we explicitly request scaling (dmabuf needs that) and instead create properly sized buffers.

Firefox recently had the same issue, with blur on Wayland. This initial issue [1] was that it's blurry on Wayland and solution was to use "stable rounding" algo suggested in one of the WP PR [2]. While this fixed some targets, it regressed on KWin [3]... and the solution was to not use "stable rounding" for KDE.

So... it looks messy, I would rather think of better solution that matching rounding algorithm with compositor implementation. Either way, we have to be careful here to make it display mpv content without unwanted scaling.
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=2000769
[2] https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/309
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=2004350

@Dudemanguy
Copy link
Member Author

Dudemanguy commented Feb 17, 2026

We're talking about the specific case that this PR address which is mismatched fullscreen sizes. Being off by one pixel is already bad. You trade one bad thing for another bad thing in this scenario.

Suppose the output has a pixel size of 1000 with a fractional scale of 3. If the compositor sends a toplevel size of 334, then you are aware of the 2 pixel excess and can render an image confined within the 1000 pixel region. If you instead use the pixel size of 1000 to calculate the destination size, it will be 333, which only scales to 999 pixels, leaving 1 pixel gap.

The current status quo on this example already is off by 1 pixel.

Reading documentation, we probably should not set destination at all

If we're not using fractional scaling, we could get away with this but that greatly complicates the code and was moved away from. In the non-fractional case, we don't have rounding problems so they shouldn't be an issue.

@mahkoh
Copy link
Contributor

mahkoh commented Feb 17, 2026

There are several issues here:

Firefox recently had the same issue, with blur on Wayland. This initial issue [1] was that it's blurry on Wayland and solution was to use "stable rounding" algo suggested in one of the WP PR [2]. While this fixed some targets, it regressed on KWin [3]... and the solution was to not use "stable rounding" for KDE.

This is about subsurfaces and therefore only affects vo-dmabuf-wayland. Otherwise mpv does not use subsurfaces, AIUI. In vo-dmabuf-wayland, mpv currently uses the following parent -> child relationships:

  • background-surface -> video-surface -> OSD-surface

This will lead to blurriness of the video-surface since the rounding for subsurface is not defined. This is the problem firefox is having.

However, for mpv the solution is easy. mpv can simply use this relationship:

  • video-surface -> background-surface -> OSD-surface

making the video-surface the root surface with fully defined rounding. The only other change required is a call to wl_subsurface.place_below to place the background surface below the video surface.

Therefore, vo-dmabuf-wayland can be made to behave exactly like every other wayland VO and we can ignore its use of subsurfaces for the purpose of this MR.


Even if they are identical, when output geometry is smaller than window geometry, this violates the "the client cannot resize beyond it" condition.

That is so. However, the protocol does not define a protocol error for this and therefore it is possible that no compositor actually sends an error in this case.


The destination size can also result in a scaled size that is smaller than output size which would result in video not completely filling the screen.

AIUI, mpv uses this code to set the destination:

mpv/video/out/wayland_common.c

Lines 4220 to 4224 in aa82213

void vo_wayland_handle_scale(struct vo_wayland_state *wl)
{
wp_viewport_set_destination(wl->viewport, lround(mp_rect_w(wl->geometry) / wl->scaling_factor),
lround(mp_rect_h(wl->geometry) / wl->scaling_factor));
}

The logic should always be

  1. Compositor sends logical size L.
  2. mpv calculates buffer size B from L.
  3. mpv sets viewport destination to L.
  4. mpv attaches buffer with size B.

However, mpv seems to be doing the following:

  1. Compositor sends logical size L.
  2. mpv calculates buffer size B from L.
  3. mpv calculates logical size L' from B.
  4. mpv sets viewport destination to L'
  5. mpv attaches buffer with size B.

This is incorrect because L' != L and B' calculated from L' is not necessarily the same as B. If so, this will lead to blurriness.

However, this also means that, if mpv changes B, as this PR does, then L' will automatically be updated as well. Therefore this PR does not make it worse than it already is.

If mpv wants the video surface to be blited 1-to-1 to the screen, it must do the following:

  1. Define some logical size L, whether that is the size send by the compositor in xdg_toplevel.configure or some other size.
  2. Calculate buffer size B from L.
  3. Set viewport destination to L.
  4. Attach buffer with size B.

Never calculate L from B.


My own opinion is that using anything other than the size sent in xdg_toplevel.configure for fullscreen surface is incorrect. If there was a better logical size L than the one sent by the compositor, the compositor would already send it.

@na-na-hi
Copy link
Contributor

na-na-hi commented Feb 17, 2026

The current status quo on this example already is off by 1 pixel.

If you are referring to the example in the issue, it's off by 1 pixel too large, not too small. It trades one bad thing for another, but in this case is the issue mentioned guaranteed fixed when Wayland cannot represent in the logical coordinate system a pixel size of 1000 when scale is 3? It is possible that it is "fixed" in KWin for the 1 pixel too large case because of luck and compositor implementation details, while it will cause issues in other compositors. It is not fixing the root cause.

This is incorrect because L' != L

The "mpv calculates buffer size B from L" introduces at most a 0.5 pixel difference (delta_B) for B from the "ideal" B: B = L * scale_factor + delta_B where abs(delta_B) <= 0.5

When converting B to L', L' L' = round((L * scale_factor + delta_B) / scale_factor) = round(L + delta_B / scale_factor). Since L is an integer and abs(delta_B / scale_factor) is less then 0.5 as long as scale factor is larger than 1, L' = L.

However, this also means that, if mpv changes B, as this PR does, then L' will automatically be updated as well.

And this will actually make the L' incorrect.

@mahkoh
Copy link
Contributor

mahkoh commented Feb 17, 2026

Since L is an integer and abs(delta_B / scale_factor) is less then 0.5 as long as scale factor is larger than 1, L' = L.

Ah, you are correct. I had thought about this in the past and determined that you cannot calculate L from B. But this only applies to the case where the scale is less than 1.

Nevertheless, is there any reason to not handle that case correctly?

And this will actually make the L' incorrect.

In the sense that L' != L. Not in the sense that it will be blurry. As long as L' expands to B, the surface will be blitted 1-to-1.

@kasper93
Copy link
Member

This is about subsurfaces and therefore only affects vo-dmabuf-wayland.

In dmabuf-wayland mpv, AIUI, actually depends on the scaling done by the compositor.

However there are few special cases when it should be blitted 1-to-1. One of which is when targeting fullscreen and video resolution matches the resolution exactly.

Another, more general, case is when someone prescale video to window resolution with a filter, where I would expect it be blitted 1-to-1.

If we introduce 1-2 pixel error, it may be hard for user to control the scaling and implicit resampling may be introduced.

I don't know the extent of the issue currently, but reading this thread doesn't make me confident in dmabuf-wayland.

@mahkoh
Copy link
Contributor

mahkoh commented Feb 17, 2026

However there are few special cases when it should be blitted 1-to-1. One of which is when targeting fullscreen and video resolution matches the resolution exactly.

There are two steps:

  1. mpv copies the video to its buffer without scaling
  2. the compositor copies the buffer to the output without scaling

Currently: The first step is not fulfilled if the mode size of the output is not divisible by the scale. In this case, B will not be the mode size but something smaller. The second step is fulfilled.

With this patch: The first step is fulfilled since mpv always sets B equal to the mode size. The second step is not fulfilled because L calculated by mpv will in general not expand to B. Therefore the compositor will scale the buffer.

@kasper93
Copy link
Member

kasper93 commented Feb 17, 2026

B will not be the mode size but something smaller.

I think it's actually can also be bigger. As demonstrated by the original issue reported.

Mode width: 3072
Scale: 222 / 120 = 1.85

round(3072 / 1.85)=1661
round(1661 * 1.85)=3073

It's rounded up twice, by compositor and by mpv. (this is not related to "stable rounding")

@mahkoh
Copy link
Contributor

mahkoh commented Feb 17, 2026

It's rounded up twice

It is possible. But I always round it down so that everything is visible.

@Dudemanguy
Copy link
Member Author

The reason we calculate L is because we don't always want to use L from the compositor (custom geometry settings and other things). But for the case of fullscreen, we of course can.

@mahkoh
Copy link
Contributor

mahkoh commented Feb 17, 2026

That is fine, but then you either have to calculate B from L or you have to ensure that the calculated L maps to B.

As @llyyr showed above, if there is some L' such that L' expands to B, then the L you calculate is correct. However, there are values for B such that there is no such L' and then the L you calculate is wrong. This is because going from L to L+1 sometimes skips a B value and therefore this B has no preimage. I.e. it is impossible for a buffer of size B to ever be displayed 1-to-1.

@Dudemanguy
Copy link
Member Author

Added a couple of commits to address the discussion.

@na-na-hi
Copy link
Contributor

In the sense that L' != L. Not in the sense that it will be blurry. As long as L' expands to B, the surface will be blitted 1-to-1.

L' will never expand to B if scale factor is > 1, because B and B' differ by more than 1.

@mahkoh
Copy link
Contributor

mahkoh commented Feb 18, 2026

Here I meant the buffer size B that mpv is actually attaching.

@mahkoh
Copy link
Contributor

mahkoh commented Feb 18, 2026

I've created a 1920x1080 test image and tested how mpv displays it at scale 1.625:

Original image:

test-image

mpv current release:

20260218_03h31m58s_grim

mpv with this patch:

20260218_03h32m04s_grim

@mahkoh
Copy link
Contributor

mahkoh commented Feb 18, 2026

The difference at scale 1.4 is more striking, but not all compositors can handle scales that are not representable as floats.

Release:

20260218_03h51m09s_grim

This patch:

20260218_03h51m14s_grim

@Dudemanguy
Copy link
Member Author

Forgot about this. I'll try to test on multiple monitors later to make sure nothing weird happens.

@mahkoh
Copy link
Contributor

mahkoh commented Feb 24, 2026

You should limit this hack to situations where the video size is the mode of the output. Otherwise the video will be scaled twice: once by mpv and once by the compositor.

@Dudemanguy
Copy link
Member Author

Dudemanguy commented Feb 24, 2026

Then we're back to off by 1 pixel errors.

@mahkoh
Copy link
Contributor

mahkoh commented Feb 24, 2026

Yes, it is a tradeoff between picture quality and being off by 1 px.

@mahkoh
Copy link
Contributor

mahkoh commented Feb 24, 2026

You could of course not scale the picture client side at all. That is, a 720p video will be presented via a 720p buffer and the scaling will be left entirely to the compositor via viewporter. Then you would have to put the OSD on a subsurface to ensure that it is sharp.

I think you have to put the OSD on a subsurface either way if you go through with this patch since otherwise the compositor will stretch and therefore blur the OSD.

@Dudemanguy
Copy link
Member Author

Dudemanguy commented Feb 24, 2026

The whole premise here is that not being off by 1 px is better. I mean yes the extra compositor scaling is not really great, but that's not nearly as bad as 1 pixel missing. At least imo. And this is only for fullscreen of course.

@llyyr
Copy link
Contributor

llyyr commented Feb 24, 2026

instead of it being 1 pixel missing, we could just prefer to waste 1 pixel instead in that scenario no?

@Dudemanguy
Copy link
Member Author

I'm not sure what you mean.

@kasper93
Copy link
Member

kasper93 commented Feb 24, 2026

The whole premise here is that not being off by 1 px is better. I mean yes the extra compositor scaling is not really great, but that's not nearly as bad as 1 pixel missing. At least imo. And this is only for fullscreen of course.

In the standard mpv way, we can have an option to control that.

I think it should be fixed on the protocol side, because both of those ways are really suboptimal to say the least.

If we could fix this on the protocol side, we could just add big fat warning that fractional scaling v1 may result in unexpected image quality degradation.

EDIT: Maybe we should add such warning either way? Users could opt to set 1.0 scale, if they are affected.

@guidocella
Copy link
Contributor

The warning can recommend changing the scale. I already reported on IRC 5 months ago that all widescreen videos with a 1.5 scale on a 1440 output are 1 pixel smaller and found that the solution is to use a 1.507812 scale.

@mahkoh
Copy link
Contributor

mahkoh commented Feb 25, 2026

I'm not sure what you mean.

I believe @llyyr is saying that instead of rendering at 1919 pixels you should render at 1921 pixels and accept that one pixel won't be visible.

@mahkoh
Copy link
Contributor

mahkoh commented Feb 25, 2026

The warning can recommend changing the scale.

I believe this is the way to go. The issue the user in #17415 is experiencing is that 185% does not cleanly divide his 3072 x 1920 screen. However, the user can instead use 223 / 120 = 185.83333333% which is extremely close to his desired scaling and does cleanly divide his screen size (in the sense of the protocol, e.g. (1653 * 223 + 60) / 120 = 3072), leading to no off-by-one errors.

I am aware that KDE currently does not allow users to set scalings that are not multiples of 3 * 5 and 223 is a prime number. This is a limitation in KDE. mpv should not hack around this limitation.

@llyyr
Copy link
Contributor

llyyr commented Feb 25, 2026

The main goal here should be to avoid being scaled by the compositor no matter what. Compositor scaling will result in either a blurry mess or a blocky mess. Most wayland compositors either use bilinear or nearest for scaling, which is fine for regular desktop applications but not for videos or pictures.

I'm not sure what you mean.

I meant we should err on the side of rendering at 1919 instead of 1921, but it seems the compositor will scale mpv window in this case so this might be pointless.

I'm also +1 for the warning, if the dimensions after fractional calculations isn't exactly a mode of the output then we should simply warn users about it and do nothing else

@mahkoh
Copy link
Contributor

mahkoh commented Feb 25, 2026

I meant we should err on the side of rendering at 1919 instead of 1921

Ah, that makes more sense. I thought mpv was already rendering at 1919 and I was wondering why @Dudemanguy was so concerned about one pixel at the edge of the screen being black when every monitor I've ever owned displays a small border area around the usable area anyway.

I meant we should err on the side of rendering at 1919 instead of 1921, but it seems the compositor will scale mpv window in this case so this might be pointless.

Can't you just decrement the logical size until size * scale <= mode?

Previously, we calculated out the value for simplicity but there can be
a mismatch due to round where what we calculate as the viewport
width/height is not what surface local coordinates the compositor gave
us. Note that we don't always want to use these values directly (e.g.
whenever we set our own geometry), so some notion of overriding has to
be added.
For rounding reasons, we should have the surface with the video on the
top. Rounding subsurfaces is tricky business that we don't need to get
into.
@Dudemanguy Dudemanguy changed the title wayland: set geometry directly to output resolution for fullscreen wayland: directly use surface local coords for viewport when possible Feb 25, 2026
@Dudemanguy
Copy link
Member Author

Since nobody wants it, I've dropped the first commit.

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

Successfully merging this pull request may close these issues.

6 participants