-
Notifications
You must be signed in to change notification settings - Fork 22
Conversation
Can you elaborate? If you have a |
When you mentioned |
src/extensions/idle.rs
Outdated
} | ||
|
||
/// Restart the timers for the seat | ||
pub fn notify_activity(&mut self, seat: *mut wlr_seat) { |
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.
This is not safe, you should be passing a &Seat
here and internally using as_ptr
to convert it.
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.
Fixed
src/extensions/idle.rs
Outdated
|
||
/// Restart the timers for the seat | ||
pub fn notify_activity(&mut self, seat: *mut wlr_seat) { | ||
unsafe { wlr_idle_notify_activity(self.manager, seat as *mut wlr_seat) } |
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.
Unnecessary cast
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.
Fixed
src/extensions/idle.rs
Outdated
} | ||
|
||
/// If we are passed a null pointer, | ||
/// update timers for all seats. |
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.
Put this on one doc line.
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.
Fixed
src/extensions/idle.rs
Outdated
/// If we are passed a null pointer, | ||
/// update timers for all seats. | ||
pub fn set_enabled(&mut self, seat: *mut wlr_seat, enabled: bool) { | ||
wlr_log!(WLR_INFO, "Idle timer status: {:?}", enabled); |
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.
Remove this log
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.
Fixed
src/extensions/idle.rs
Outdated
|
||
/// If we are passed a null pointer, | ||
/// update timers for all seats. | ||
pub fn set_enabled(&mut self, seat: *mut wlr_seat, enabled: bool) { |
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.
This should also be a &Seat
.
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.
Fixed
src/extensions/idle.rs
Outdated
/// update timers for all seats. | ||
pub fn set_enabled(&mut self, seat: *mut wlr_seat, enabled: bool) { | ||
wlr_log!(WLR_INFO, "Idle timer status: {:?}", enabled); | ||
unsafe { wlr_idle_set_enabled(self.manager, seat as *mut wlr_seat, enabled) } |
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.
Also an unnecessary cast
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.
Fixed
@@ -35,6 +35,26 @@ mod generated { | |||
include!(concat!(env!("OUT_DIR"), "/server_decoration_server_api.rs")); | |||
} | |||
} | |||
pub mod idle { |
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.
I mainly copied-pasted this part and subbed wl_surface with wl_seat. What's the point of this section?
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.
The code generated in the build script is placed into files, specifically <interface_name>_api.rs
and <interface_name>_interfaces.rs
. We need these definitions because they implement glue code for taking our Rust types and marshaling them into a form to be sent over the Wayland socket. Since we marshal to this type, this means the response can be consumed by any language (because they can convert from the Wayland types to their lang types).
The use statements are here because we need literally copy and paste the code from the files (kind of like how #include
works in C actually) using the include!
macro.
It needs to be in a module because this generates a lot of code we don't care about. The only code we about is in the generated server module, which we re-export at the end of the file.
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.
So considering that I only did it for idle, should I do a PR for the rest of the extensions that I've done so far? Since from what I understand, the other extensions aren't still linked up properly then..
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.
Hm, actually looking at this again I don't think this is necessary. I haven't looked at the extensions in a while, so it might have changed.
This should not be here actually, since wlroots already links to it. I believe this to be the case, so this generation code can actually be removed since it's dead code.
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.
Yeah this code is dead. The only one that is needed is the server decoration, and that's for an enum defined in the protocol. So sorry about that, your other protocols are fine. So is this one, it's just that this code is unnecessary. I'll make an issue for that now: #266
I dug a little more and found out my main mistake of using the unsafe |
Good! In order to make sure the knowledge is correct, let me explain further. At first glance the two seem isomorphic, since you can get the pointer from the A function is safe if there's no way to produce memory safety in any of its inputs. For a |
Also also the Though that's getting into the weeds of the rest of the library, which has a very complicated memory model and not very relevant to this immediate work. |
Thanks! |
Closes #100
WIP as I can't get it to see the seat pointer. I was trying to do what server decoration does but 🤷♂️
Everything else is supposedly working according to cargo... 🤞
IF working, we can support swayidle now.