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

Fix build without shadows enabled #118

Closed
wants to merge 2 commits into from

Conversation

polter-rnd
Copy link
Contributor

@polter-rnd polter-rnd commented Nov 1, 2022

Build without DECORATION_SHADOWS_SUPPORT was broken (at least with Qt 5.15 from Fedora). It also required some changes to get it work correctly.

Also added "Use more updated window state values" patch that existed in Fedora repos until 0.9 version -- I think it could be still useful when building without shadows (espetially it could be used for PR #116 to gracefully disable shadows if not needed)

@polter-rnd
Copy link
Contributor Author

polter-rnd commented Nov 1, 2022

I see there's a build failure with Qt5 -- seems that CI uses vanilla 5.15.2 while I was trying of 5.15.7 from Fedora Rawhide which has backported QWaylandAbstractDecoration::margins(MarginsType marginsType = Full) from Qt6.

@grulja
Copy link
Collaborator

grulja commented Nov 2, 2022

The support for shadows work either with patched Qt5 (Fedora → I did it there myself) and with Qt6. It is intentional so I'm not going to merge this change as it would break it for everyone using non-patched Qt5.

@grulja grulja closed this Nov 2, 2022
@grulja
Copy link
Collaborator

grulja commented Nov 2, 2022

Also this https://github.com/qt/qtwayland/blob/dev/src/client/qwaylandwindow_p.h#L136 is missing in Qt 5.15. Looks we had that in Fedora backported before and I probably dropped it accidentally. I'll check. Either way, this is not something we are able to do here as the rest of the world doesn't have all the backports we have in Fedora.

@grulja
Copy link
Collaborator

grulja commented Nov 2, 2022

Btw. I brought back the patch with updated states back to Fedora and pushed it here #119.

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.

2 participants