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

WindowIdentifier::from_native() may only be called once for Wayland GTK windows #20

Closed
Cogitri opened this issue Aug 24, 2021 · 7 comments · Fixed by #46
Closed

WindowIdentifier::from_native() may only be called once for Wayland GTK windows #20

Cogitri opened this issue Aug 24, 2021 · 7 comments · Fixed by #46

Comments

@Cogitri
Copy link
Contributor

Cogitri commented Aug 24, 2021

Hello,

it seems like WindowIndentifier::from_native() can only be called once, otherwise the following GCritical is emitted:

(dev.Cogitri.Health:21): Gdk-CRITICAL **: 12:22:08.835: gdk_wayland_toplevel_export_handle: assertion '!impl->display_server.xdg_exported' failed

This GCritical origins from gdk_wayland_toplevel_export_handle. The documentation for that function says:

* gdk_wayland_toplevel_export_handle:
[...]
* It is an error to call this function on a surface that is already
* exported.

Not sure what would be the best way to handle this, should we just put a notice in the docs?

@bilelmoussaoui
Copy link
Owner

Yes that's expected because the handle is unexported when the WindowIdentifier is dropped.

I wonder what would be the best way forward here, but addikg a comment at least seems fine to me

@Cogitri
Copy link
Contributor Author

Cogitri commented Aug 24, 2021

Alright, I’ll submit a PR later today.

Another question: To work around this I currently just hold onto the WindowIndentifier in a OnceCell that’s initialised when the window is created. Is that fine or should I always re-request the WindowIdentifier?

@bilelmoussaoui
Copy link
Owner

I would say you can keep only once instance around by wrapping it in a OnceCell. I don't know if we can do something about it on the crate side?

@Cogitri
Copy link
Contributor Author

Cogitri commented Aug 24, 2021

Hm, I don’t really think we can other than hacky ways like having a global HashMap<WeakRef, WindowIdentifier> to remember the WindowIdentifier

@bilelmoussaoui
Copy link
Owner

Ah yeah indeed. That would be very painful to do. I guess going with a comment would be good enough for now.

Feel free to open another issue regarding the usage of the Default implementation of WindowIdentifier in some of the portals

@A6GibKm
Copy link
Collaborator

A6GibKm commented Oct 5, 2021

Cant we do a static Once that holds the identifier?

@bilelmoussaoui
Copy link
Owner

You can, from the application side

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 a pull request may close this issue.

3 participants