Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

win32: add option to change backdrop style #12452

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

DeadSix27
Copy link
Contributor

@DeadSix27 DeadSix27 commented Sep 21, 2023

Adds the ability to change the style of the backdrop to mica (alt), none or acrylic.

This is a Windows 11 only feature, I can't test this on 10 and it's most likely not gonna work on 7 at all, but since we only support 10+ it doesn't matter.
Feel free to test on fully updated 10 and report back.

The backdrop style mainly affects the Top bar/border because we have no other GUI elements where the backdrop is visible, e.g no button popup windows or anything.

As far as I can tell, Mica is transparent to the desktop background only, same as mica-alt, while acrylic is transparent to the windows/objects directly behind it.

While we're at it, should we set acrylic to be the default? or just let windows handle it, idk.. RFC on that.

Another question, but to my code, is the enum mapping fine? I can't think of a prettier solution other than having lots of conditionals in the update function itself.. I can't have the enum values directly in OPT_CHOICE since it's not supported afaik.

EDIT: had to force-push leftover debug out.

Here's a comparison of the available styles:
image

@DeadSix27
Copy link
Contributor Author

DeadSix27 commented Sep 21, 2023

The code works fine on my end and compiles fine, is the mingw install of the runner too old? I build/test with Clang17/GCC14 and latest mingw commit

grep -Rnw . -e '.*DWM_SYSTEMBACKDROP_TYPE.*'                                                            
./mingw-w64-headers/include/dwmapi.h:69:  enum DWM_SYSTEMBACKDROP_TYPE {

@Dudemanguy
Copy link
Member

It seems to be missing DWMSBT_NONE and other related things. You may need to guard it.

@DeadSix27
Copy link
Contributor Author

It seems to be missing DWMSBT_NONE and other related things. You may need to guard it.

Against? Old mingw versions?

@Dudemanguy
Copy link
Member

Beats me, I don't know windows. Whatever provides that enum I guess.

@DeadSix27
Copy link
Contributor Author

DeadSix27 commented Sep 21, 2023

Beats me, I don't know windows. Whatever provides that enum I guess.

No clue how to guard it either, the enums have been added months ago: mingw-w64/mingw-w64@bbae5ec
and mingw has no defines for versions or similar

EDIT: yep, ubuntu has outdated mingw-headers, thats why. Idk how to make the github workflow use more recent mingw-headers, their version is from the stoneage

@DeadSix27
Copy link
Contributor Author

DeadSix27 commented Sep 21, 2023

@Dudemanguy I guarded the changes now, lets hope it builds fine, the stuff I need from the mingwheader is not in ubuntu's version of mingw yet.. annoyingly, but this hopefully compiles fine at least.

EDIT: nvm merge bullshit, git really doesn't like me or I do not get the hang of it.

EDIT2: ok now, there were so many commits and merge conflicts due to the recent commits, it took me some figuring out.

options/options.c Outdated Show resolved Hide resolved
options/options.c Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Sep 21, 2023

Download the artifacts for this pull request:

Windows

@DeadSix27
Copy link
Contributor Author

DeadSix27 commented Sep 21, 2023

@Dudemanguy updated it and moved the other options together, can you check if i made a mistake anywhere?

EDIT: I could probably get rid of one guard , the most ugly one around else if (changed_option == &vo_opts->backdrop_type) by just guarding inside the function and just making it return void, but honestly the whole guarding is gonna be hopefully just very temporary until github-runner updates their mingw or we build the runner's mingw from master or something..

video/out/w32_common.c Outdated Show resolved Hide resolved
video/out/w32_common.c Outdated Show resolved Hide resolved
video/out/w32_common.c Outdated Show resolved Hide resolved
video/out/w32_common.c Outdated Show resolved Hide resolved
DOCS/man/options.rst Outdated Show resolved Hide resolved
DOCS/man/options.rst Outdated Show resolved Hide resolved
video/out/w32_common.c Outdated Show resolved Hide resolved
video/out/w32_common.c Outdated Show resolved Hide resolved
@sfan5
Copy link
Member

sfan5 commented Sep 22, 2023

EDIT: yep, ubuntu has outdated mingw-headers, thats why. Idk how to make the github workflow use more recent mingw-headers, their version is from the stoneage

While Ubuntu LTS indeed has quite old software you have to consider that the change you linked has not been in a mingw-w64 release yet. Even rolling-release distributions don't ship git versions of MinGW, only stable releases.

@DeadSix27
Copy link
Contributor Author

DeadSix27 commented Sep 22, 2023

EDIT: yep, ubuntu has outdated mingw-headers, thats why. Idk how to make the github workflow use more recent mingw-headers, their version is from the stoneage

While Ubuntu LTS indeed has quite old software you have to consider that the change you linked has not been in a mingw-w64 release yet. Even rolling-release distributions don't ship git versions of MinGW, only stable releases.

Agreed, however afaik even going by their "releases" ubuntu is ages behind them, I think 11.0.1 is the last release, the code needed will be in 12, but ubuntu wont have that either.

About the other issues, I'll come back to them after travels, to test them.

I pushed the changes but I didn't run the test yet.

EDIT: Tested, works fine on my end, any other code changes you'd like?

Copy link
Member

@Dudemanguy Dudemanguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK to me. I don't know what happens with the DwmSetWindowAttribute on older versions. Just nothing I guess?

@DeadSix27
Copy link
Contributor Author

DeadSix27 commented Sep 23, 2023 via email

@Dudemanguy
Copy link
Member

Oh indeed. Should be fine then.

@Dudemanguy Dudemanguy merged commit f19ada7 into mpv-player:master Sep 27, 2023
14 checks passed
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.

4 participants