-
-
Notifications
You must be signed in to change notification settings - Fork 21.1k
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
wayland: Update to 1.22.0 #87977
wayland: Update to 1.22.0 #87977
Conversation
Thanks for looking into this. Some API changes require downstream changes too: https://github.com/godotengine/godot/pull/87977/files#diff-c86c10b4f67151b187e2f364912792198ba5a3ef23346cfe43cb054b466b4c59 |
Yup, we need to implement even just stub event handlers. Note that this isn't terribly important yet as we don't use all the new features, which is on my TODO list. Edit: to be clear, for now the new handlers must be stubs, as we're purposely asking for older versions. It's not needed to implement the various logic yet. If that's really needed though, please let me know. |
At first I added short implementations that looked like this: void WaylandThread::_wl_surface_on_preferred_buffer_scale(void *data, struct wl_surface *wl_surface, int32_t factor) {
WindowState *ws = (WindowState *)data;
ERR_FAIL_NULL(ws);
ws->preferred_fractional_scale = factor;
DEBUG_LOG_WAYLAND_THREAD(vformat("Window preferred buffer scale %d.\n", factor));
} But given that this is way out of the scope I was prepared to tackle in this PR, I'm leaving them empty for the time being. |
Does this: void WaylandThread::_wl_surface_on_preferred_buffer_scale(void *data, struct wl_surface *wl_surface, int32_t factor) {
} fall under the definition of "stub" @Riteo? If so, I can rebase the PR and it'll be ready for review :) |
Yeah, it's best to keep them empty, as this code won't be run and thus can't even be tested yet. Also, we can't just randomly bump the version as there's a lot more event handlers or changes between versions that we have to keep in mind. This is all planned but for now I'm concentrating on other important foundational stuff now that most of the work is merged.
Yup! FTR, there's also another approach, which is to assign a simple void "noop" method, but I think that C++ might annoy us (it's way less lenient with pointer assignment) and we'd still have to implement those anyways later so IMO it's more worth it to make empty stubs as you've done, which is what I'd recommend :D |
902157b
to
292df42
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine, thanks!
Thanks! |
https://gitlab.freedesktop.org/wayland/wayland/-/releases/1.22.0