Another day, another wayland refactor. Way back when, dcc3c2e added
support for the hidpi-window-scale option (something you probably should
never set to no but whatever) to wayland. Well technically, it never had
any runtime toggling support (don't remember if detecting when vo_opts
changed was possible or not then; maybe not). Anyways in the process of
fixing that, I went ahead and refactored how this is all handled. The
key difference is that when hidpi-window-scale is disabled, wl->scaling
is directly set to 1 instead of forcibly setting
wl->current_output->scale to 1. Note that scaling operations don't
always require a geometry reset/resize so set_surface_scaling needs to
be separate from set_geometry. The logic here is kind of complicated but
it (should) be correct.
The wl_surface lives for the entire lifetime of the vo. It's only
neccesary to set the scale initially and when the output scaling changes
(the surface moves to a different output with a different scale or the
output itself changes it scale). All of the calls that were being made
in the egl/vulkan resize functions are not needed. vo_wlshm wasn't
correctly rescaling itself before this commit since it had no logic to
handle scale changes. This should all be shared, common code in the
surface/output listeners.
A subtle regression from c26d833. On sway if mpv was set to be a
floating window in the config, set_buffer_scale would actually get
applied twice according to the wayland log. That meant a 1920x1080
window would appear as a 960x540 window if the scale of the wl_output
was set to 2. This only affected egl on sway (didn't occur on weston and
was too lazy to try anything else; probably they were fine). Since
wl->render is initially false, that meant that the very first run
through the render loop returns false. This probably caused something
weird to happen with the set_buffer_scale calls (the egl window gets
created and everything but mpv doesn't write to it just yet) which makes
the set_buffer_scale call happen an extra time. Since it was always
intended for mpv to initally render, this is worth fixing. Just chnage
wl->render to wl->hidden (again) and flip the bools around. That way,
the initial false value results in render == true and mpv tries to draw
on the first pass. This fixes the weird scaling behavior because
reasons.
Not sure what I was on when I wrote this. wayland-app-id is supposed to
default to "mpv". Just set that in the vo_sub_opts and don't do this
weird m_config_cache_write_opt thing. Also make the doc entry nicer.
Mostly a cosmetic change that (hopefully) makes things look better. Some
functions and structs that were previously being exported in the wayland
header were made static to the wayland_common.c file (these shouldn't be
accessed by anyone else).
If a plane is not connected to any displays, we won't set an entry in
the mapping of planes to displays. So ensure these unset entries are
null and skip them.
Fixes#8913
This is the Vulkan equivalent of the drm context for OpenGL, with
the big difference that it's implemented purely in terms of Vulkan
calls and doesn't actually require drm or kms.
The basic idea is to identify a display, mode, and plane on a device,
and then create a display backed surface for the swapchain. In theory,
past that point, everything is the same, and this is in fact the case
on Intel hardware. I can get a video playing on a vt.
On nvidia, naturally, things don't work that way. Instead, nvidia only
implemented the extension for scenarios where a VR application is
stealing a display from a running window system, and not for
standalone scenarios. With additional code, I've got this scenario to
work but that's a separate incremental change.
Other people have tested on AMD, and report roughly the same behaviour
as on Intel.
Note, that in this change, the VT will not be correctly restored after
qutting. The only way to restore the VT is to introduce some drm
specific code which I will illustrate in a separate change.
The VkDisplayKHR context type requires making calls against the
physical device before the libplacebo context is initialised.
That means we can't simply use the physical device object that
libplacebo would create - instead we have to create a separate one,
but make sure it's referring to the same physical device.
To that end, we need the device name that the user may have requested
so we can pass it on.
This was originally just a bugfix for a race condition, but the scope
expanded a bit. Currently, the wayland code does a prepare_read ->
dispatch_pending -> display_flush -> read_events -> dispatch_pending
routine that's basically straight from the wayland client API
documentation. This essentially just queues up all the wayland events
mpv has and dispatches them to the compositor. We do this for blocking
purposes on every frame we render. A very similar thing is done for
wait_events from the VO.
This code can pretty easily be unified and split off into a separate
function, vo_wayland_dispatch_events. vo_wayland_wait_frame will call
this function in a while loop (which will break either on timeout or if
we receive frame callback from the compositor). wait_events needs to
just check this in case we get some state change on the wakeup_pipe
(i.e. waking up from the paused state).
As for the actual bugfix part of this, it's a slight regression from
c26d833. The toplevel config event always forced a redraw when a surface
became activated again. This is for something like displaying cover art
on a music file. If the window was originally out of view and then later
brought back into focus, no picture would be rendered (i.e. the window
is just black). That's because something like cover art is just 1 frame
and the VO stops doing any other additional rendering. If you miss that
1 frame, nothing would show up ever again. The fix in this case is to
always just force a redraw when the mpv window comes back into view.
Well with the aforementioned commit, we stopped doing
wl_display_roundtrip calls on every frame. That means we no longer do
roundtrip blocking calls. We just be sure to queue up all of the events
we have and then dispatch them. Because wayland is fundamentally an
asynchronous protocol, there's no guarantee what order these events
would be processed in. This meant that on occasion, a
vo_wayland_wait_frame call (this could occur multiple times depending on
the exact situation) would occur before the compositor would send back
frame callback. That would result in the aforementioned bug of having
just a black window. The fix, in this case, is to just do a
vo_wayland_wait_frame call directly before we force the VO to do a
redraw. Note that merely dispatching events isn't enough because we
specifically need to wait for the compositor to give us frame callback
before doing a new render.
P.S. fix a typo too.
And also change the existing WM_KILLFOCUS handler to return 0 instead
of 'break' (which later calls DefWindowProcW), as MSDN says we should
do for WM_{KILL,SET}FOCUS.
It seems that the 'focused' property is now supported by all main VOs:
x11, macOS, wayland, Windows.
TCT/sixel/caca probably don't support it, and unknown with SDL.
Fixes#8868
Not only does this have semantics that make far more sense, it also has
a default that makes far more sense. (Equivalent to the old
`icc-contrast=inf`)
This removes the weird 1000:1 contrast default assumption which
especially broke perceptual profiles and also screws things up for
OLED/CRT/etc.
Should probably close some issues but I honestly can't be bothered to
figure out which of the thousands colorimetry-related issues are
affected.
Take two. f4e89dd went wrong by moving vo_wayland_wait_frame before
start_frame was called. Whether or not this matters depends on the
compositor, but some weird things can happen. Basically, it's a
scheduling issue. vo_wayland_wait_frame queues all events and sends them
to the server to process (with no blocking if presentation time is
available). If mpv changes state while rendering (and this function is
called before every frame is drawn), then that event also gets
dispatched and sent to the compositor. This, in some cases, can cause
some funny behavior because the next frame gets attached to the surface
while the old buffer is getting released. It's safer to call this
function after the swap already happens and well before mpv calls its
next draw. There's no weird scheduling of events, and the compositor log
is more normal.
The second part of this is to fix some stuttering issues. This is mostly
just conjecture, but probably what was happening was this thing called
"composition". The easiest way to see this is to play a video on the
default audio sync mode (probably easiest to see on a typical 23.976
video). Have that in a window and float it over firefox (floating
windows are bloat on a tiling wm anyway). Then in firefox, do some short
bursts of smooth scrolling (likely uses egl). Some stutter in video
rendering could be observed, particularly in panning shots.
Compositors are supposed to prevent tearing so what likely was happening
was that the compositor was simply holding the buffer a wee bit longer
to make sure it happened in sync with the smooth scrolling. Because the
mpv code waits precisely on presentation time, the loop would timeout on
occasion instead of receiving the frame callback. This would then lead
to a skipped frame when rendering and thus causing stuttering.
The fix is simple: just only count consecutive timeouts as not receiving
frame callback. If a compositor holds the mpv buffer slightly longer to
avoid tearing, then we will definitely receive frame callback on the
next round of the render loop. This logic also appears to be sound for
plasma (funfact: Plasma always returns frame callback even when the
window is hidden. Not sure what's up with that, but luckily it doesn't
matter to us.), so get rid of the goofy 1/vblank_time thing and just
keep it a simple > 1 check.
Checking against 1.0 is wrong for this code, because it's in an absolute
luminance scale relative to reference white. We should be normalizing
this by `dst.sig_peak`.
This is actually a very nice simplification that should have been
thought of years ago (sue me). In a nutshell, the story with the
wayland code is that the frame callback and swap buffer behavior doesn't
fit very well with mpv's rendering loop. It's been refactored/changed
quite a few times over the years and works well enough but things could
be better. The current iteration works with an external swapchain to
check if we have frame callback before deciding whether or not to
render. This logic was implemented in both egl and vulkan.
This does have its warts however. There's some hidden state detection
logic which works but is kind of ugly. Since wayland doesn't allow
clients to know if they are actually visible (questionable but
whatever), you can just reasonably assume that if a bunch of callbacks
are missed in a row, you're probably not visible. That's fine, but it is
indeed less than ideal since the threshold is basically entirely
arbitrary and mpv does do a few wasteful renders before it decides that
the window is actually hidden.
The biggest urk in the vo_wayland_wait_frame is the use of
wl_display_roundtrip. Wayland developers would probably be offended by
the way mpv abuses that function, but essentially it was a way to have
semi-blocking behavior needed for display-resample to work. Since the
swap interval must be 0 on wayland (otherwise it will block the entire
player's rendering loop), we need some other way to wait on vsync. The
idea here was to dispatch and poll a bunch of wayland events, wait (with
a timeout) until we get frame callback, and then wait for the compositor
to process it. That pretty much perfectly waits on vsync and lets us
keep all the good timings and all that jazz that we want for mpv. The
problem is that wl_display_roundtrip is conceptually a bad function. It
can internally call wl_display_dispatch which in certain instances,
empty event queue, will block forever. Now strictly speaking, this
probably will never, ever happen (once I was able to to trigger it by
hardcoding an error into a compositor), but ideally
vo_wayland_wait_frame should never infinitely block and stall the
player. Unfortunately, removing that function always lead to problems
with timings and unsteady vsync intervals so it survived many refactors.
Until now, of course. In wayland, the ideal is to never do wasteful
rendering (i.e. don't render if the window isn't visible). Instead of
wrestling around with hidden states and possible missed vblanks, let's
rearrange the wayland rendering logic so we only ever draw a frame when
the frame callback is returned to use (within a reasonable timeout to
avoid blocking forever).
This slight rearrangement of the wait allows for several simplifications
to be made. Namely, wl_display_roundtrip stops being needed. Instead, we
can rely entirely on totally nonblocking calls (dispatch_pending, flush,
and so on). We still need to poll the fd here to actually get the frame
callback event from the compositor, but there's no longer any reason to
do extra waiting. As soon as we get the callback, we immediately draw.
This works quite well and has stable vsync (display-resample and audio).
Additionally, all of the logic about hidden states is no longer needed.
If vo_wayland_wait_frame times out, it's okay to assume immediately that
the window is not visible and skip rendering.
Unfortunately, there's one limitation on this new approach. It will only
work correctly if the compositor implements presentation time. That
means a reduced version of the old way still has to be carried around in
vo_wayland_wait_frame. So if the compositor has no presentation time,
then we are forced to use wl_display_roundtrip and juggle some funny
assumptions about whether or not the window is hidden or not. Plasma is
the only real notable compositor without presentation time at this stage
so perhaps this "legacy" mechanism could be removed in the future.
This reverts commit b8156a9a86.
Apparently there are two problems here. One on Linux (fixed by the
original change and this revert) and one on alternative-medicine-Job's
OS. Since the latter has deprecated OpenGL and OpenGL is a second-class
citizen, I think it's better to prefer the fix for a platform that is
still alive.
When inserting vf_sub, the VO OSD is directed not to draw itself. But
this flag was never unset, even when removing vf_sub.
The fix is pretty shit, but appears to work.
For some reason, this never existed before. Add VOCTRL_GET_DISPLAY_RES
and use it to obtain the current display's resolution from each
vo/windowing backend if applicable. Users can then access the current
display resolution as display-width and display-height as per the client
api. Note that macOS/cocoa was not attempted in this commit since the
author has no clue how to write swift.
3909e4cdfc seems to have replaced this 0.5 constant by 0.75, using its
presence in glumpy as justification.
0.75 is clearly a bug in glumpy, as its own source code doesn't even
match the comment immediately above it. Every other implementation of
this window I could find uses 0.5, including e.g. ImageMagick.
It turns out that if a user specifies fullscreen=yes and a width/height
in an autoprofile, the compositor can execute its toplevel listener
event. This can happen before we have any mpv window rendered at all so
we end up performing a bunch of geometry operations and state checks
when we shouldn't. It subtly messes up a lot of things like state
detection. Just return from this function if the geometry has no width
or height yet.
this is a regression of af26402, where we changed the geometry
calculation to use the visible frame and consider its origin. though the
origin is not screen relative but relative to the main screen. the rest
of the code expects a screen relative rectangle. because of that windows
would be placed out of the screen bounds when not on the main screen.
recalculate to origin to be screen relative and use those values for the
rest of the calculations. to make the code a bit more comprehensible
be a bit more explicit about what is done where with temporary variables
and comments. also move the mp_rect calculation that moves the origin
and flips the y position to a separate function, so we can reuse it
later.
The fit_on_screen logic was a bit twisted. Simplify it a bit
and update few comments to explain better what it's used for.
Note that the new logic is not identical to before, but its intent
should now be clearer. This means there might be regressions or
improvements at edge cases. If followup fixes are needed, then we
should keep the intent clear. Most likely though that it's fine.
fit_on_screen is called only from reinit_window_state.
Move the yes/no logic unmodified from fit_on_screen to
reinit_window_state, and remove the w32->parent condition because it's
already checked earlier at reinit_window_state.
Previously, because the video (client area) was centered but the top
and bottom borders are uneven (title is taller), then if the window
is shrunk vertically to just-fit the desktop - the top edge of the
title bar ended above the top edge of the display.
This is a state which Windows prevents during manual move, but
apparently it's not rejected at the Windows API.
Now we ensure it doesn't happen, and nudge the window down to align
the top edges if necessary.
This is a commulative regression of commits 981048e0 and 364af7c6.
To clarify functionality, this includes a no-op change: fit_rect was
renamed to fit_rect_size and it now takes explicit width and height,
because it only used the width/height of rc2 anyway.
Fixes#6695
The accurate description of this option was:
- fit-border is enabled by default. When disabled, it adds a bug where
if the window has borders and mpv shrinks it to fit the desktop, then
the calculation ignores the borders and adds incorrect video crop.
The option was added at commits 70f64f3c and 949247d6, in order to
solve an issue (#2935) where if mpv wanted to display a video with
size WxH, then w32_common.c incorrectly set the window to WxH, while
down-scaling the video slightly to fit (even with small sizes).
It was addressed with a new option which is enabled by default, but
does the right thing (sets the client area to WxH) only when disabled,
so that everyone who prefers their video slightly downscaled could
keep their default behavior.
(#2935 also addressed an off-by-one issue, fixed before fit-border)
While disabling the option did avoid unnecessary downscaling, it also
added a bug when disabled: the borders are no longer taken into
account when the size is too big for the desktop. Most users don't
notice and are unaffected as it's enabled by default.
Shortly later (981048e0) the core issue is fixed, and now the client
area is correctly set to WxH instead of the window (and together with
the three following commits which center the video, adds a new bug
where the window title can be outside the display - addressed next).
However, fit-border remained, now without any effect, except that it
still has the same bug when disabled and the window is too big.
Later code changes and refactoring preserved this issue with great
attention to details, and it remained in identical form until now.
Simply rip out fit-border.
See #8137 for justification.
This is not ideal, because a large cache results in a lot of `strcmp`
invocations for every single shader invocation. But it's way better than
resulting in a lot of shader recompilations for every single shader
invocation.
The only reason I don't want to uncap it altogether is because there are
conceivable edge cases in which users load dynamically generated shaders
with updated parameters (indeed, I've seen IRC discussions to this
effect), and in this case, we don't want to grow the cache infinitely as
a result of something like a floating point parameter being continuously
updated. (Never mind that this *would* actually trigger worst case
behavior for the `strcmp`, since the updated float constant is likely to
be near the bottom of the shader body)
Whatever. vo_placebo will liberate us all in the end.
The wayland code uses a heuristic to determine whether or not the mpv
window is hidden since the xdg-shell protocol does not provide a way for
a client to directly know this. We don't render with the frame callback
function for various, complicated reasons but the tl;dr is that it
doesn't work well with mpv's core (maybe an essay should be written on
this one day).
Currently, the aforementioned heuristic considers a window hidden if we
miss more frames in a row than the display's current refresh rate
(completely arbitrary number). However, the wayland protocol does allow
for the display's refresh rate to be 0 in certain cases (like a virtual
output). This completely wrecks the heuristic and basically causes only
every other frame to be rendered (real world example: nested sway
sessions).
Instead let's slightly redesign this mechanism to be a little smarter.
For coming up with the vblank time (to predict when to timeout on the
wait function), instead use the vsync interval calculated using
presentation time. That is the most accurate measure available. If that
number is not available/invalid, then we try to use the vsync interval
predicted by the presentation event.
If we still don't have that (i.e. no presentation time supported by the
compositor), we can instead use the old way of using the expected vsync
interval from the display's reported refresh rate. If somehow we still
do not have a usable number, then just give up and makeup shit. Note
that at this point we could technically ask the vo for the estimated
vsync jitter, but that would involve locking/unlocking vo which sounds
horrifying. Ideally, you never reach here.
See https://github.com/swaywm/wlroots/issues/2566 for the actual target
of this fix. wlroots uses presentation time so in practice we are mostly
just using that calculated vsync interval number.
The wayland output listener can update whenever something about the
output changes (resolution, scale). Currently, the mpv VO updates
correctly when the refresh rate changes, but changes of both scale and
resolution were not considered. This causes a bug in certain cases like
the mouse surface not being shown at certain scale factors due to the
cursor scale not being updated correctly. Also autofit type options
would not update if the resolution changes.
To fix this, we must always reset the window geometry and wl scaling
whenever the output event occurs on wl->current_output. There is no way
to know precisely what changed from the previous state, so all of the
parameters must be reset and then resized.
As an aside, this apparently doesn't fix all cursor problem as there's
apparently a bug in libwayland-cursor(*). It's still something we should
be doing regardless.
*: https://gitlab.freedesktop.org/wayland/wayland/-/issues/194
So apparently this property had existed since 2019. Internally, it's
used as a part of the console.lua script for scaling. Yours truly
somehow didn't bat an eye at the fact that the text in the console was
super small (made worse by the fact that xwayland does scale) and just
ignored it for all this time. Oh well.
To report dpi changes to mpv's core, we need to use VO_EVENT_DPI in a
couple of places. One place is, of course, the surface listener if the
scale value reported by the wayland server changes. The other place is
in the very first reconfig since mpv's core will not find the correct
scale value until we actually get a wl_output from the wayland server.
When mpv attempts to play a video that is, on average, 60 FPS on a
display that is not exactly 60.00 Hz, two options try to fight each
other: `video-sync-max-video-change` and `interpolation-threshold`.
Normally, container FPS in something such as an .mp4 or a .mkv is
precise enough such that the video can be retimed exactly to the display
Hz and interpolation is not activated.
In the case of something like certain live streaming videos or other scenario
where container FPS is not known, the default option of 0.0001 for
`interpolation-threshold` is extremely low, and while
`video-sync-max-video-change` retimes the video to what it approximately
knows as the "real" FPS, this may or may not be outside of
`interpolation-threshold`'s logic at any given time, which causes
interpolation to be frequently flipped on and off giving an appearance
of stuttering or repeated frames that is oftern quite jarring and makes
a video unwatchable.
This commit changes the default of `interpolation-threshold` to 0.01,
which is the same value as `video-sync-max-video-change`, and guarantees
that if the user accepts a video being retimed to match the display,
they do not additionally have to worry about a much more
precise interpolation threshold randomly flipping on or off. No internal
logic is changed so setting `interpolation-threshold` to -1 will still
disable this logic entirely and always enable interpolation.
The documentation has been updated to reflect this change and give
context to the user for which scenarios they might want to disable
`interpolation-threshold` logic or change it to a smaller value.
Today, validation is only possible for string type options. But there's
no particular reason why it needs to be restricted in this way, and
there are potential uses, to allow other options to be validated
without forcing the option to have to reimplement parsing from
scratch.
The first part, simply making the validation function an explicit
field instead of overloading priv is simple enough. But if we only do
that, then the validation function still needs to deal with the raw
pre-parsed string. Instead, we want to allow the value to be parsed
before it is validated. That in turn leads to us having validator
functions that should be type aware. Unfortunately, that means we need
to keep the explicit macro like OPT_STRING_VALIDATE() as a way to
enforce the correct typing of the function. Otherwise, we'd have to
have the validator take a void * and hope the implementation can cast
it correctly.
For help, we don't have this problem, as help doesn't look at the
value.
Then, we turn validators that are really help generators into explicit
help functions and where a validator is help + validation, we split
them into two parts.
I have, however, left functions that need to query information for both
help and validation as single functions to avoid code duplication.
In this change, I have not added an other OPT_FOO_VALIDATE() macros as
they are not needed, but I will add some in a separate change to
illustrate the pattern.
This fixes an issue where dithering was effectively broken on libplacebo
versions >= 103 because the dither texture was being sampled with
edge-clamped rather than repeating semantics.
The wayland code takes mouse dragging into account in order to trigger a
client-side request for a window move or window resize. According to the
xdg-shell spec*, "[t]he server may ignore move[/resize] requests
depending on the state of the surface (e.g. fullscreen or maximized)".
Since it is not actually a hard requirement, that means the compositor
could actually respond to a clientside move/resize request even if the
mpv window was fullscreen. For example, it was pointed out that in sway,
if mpv is a floating window, you could drag it around off screen even
though the window is fullscreen.
This kind of behavior does not really have any practical use. A user can
should pan a video if he/she wishes to move its orientation while
fullscreen (or maximized for that manner). Naturally, a maximized or
fullscreened window should never be manually resized (every compositor
likely ignores this anyway). The fix is to simply just not trigger the
smecial mouse dragging case if the wayland surface is fullscreened or
maximized.
*:https://gitlab.freedesktop.org/wayland/wayland-protocols/-/blob/master/stable/xdg-shell/xdg-shell.xml
the fullscreen style mask is not supported on macOS 11 anymore outside
of the native fullscreen animation. this can lead to a none working fs
or in the worst case a crash.
to fix this we will simulate a fullscreen window with a borderless
window with the size of the whole screen, though only on macOS 11.
Fixes#8490
It's about a year old, and packaged pretty much everywhere that bothers
to package libplacebo at all. Older versions are only a thing on LTS,
which will probably also use older mpv so it works out.
Starting with v2.72.0, libplacebo validates all of its parameters
internally and turns them into function failures. Doing it twice is
awfully redundant, so we can drop the parameter validation.
Also allows us to drop some preprocessor macros.
initialising UnsafeMutableRawPointer the way we did won't free those
pointers and we get dangling pointers. explicitly define a scope those
pointers are alive and auto freed.
