fix: add dispose() methods to prevent VRAM/GTT memory leaks#701
fix: add dispose() methods to prevent VRAM/GTT memory leaks#701bechampion wants to merge 1 commit intoErikReider:mainfrom
Conversation
032a278 to
1d810ea
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds dispose() methods to four widget classes to prevent GPU memory (VRAM/GTT) leaks when notifications and media players are created and destroyed repeatedly. The changes ensure that Gdk.Texture objects and related GPU resources are explicitly released rather than relying on garbage collection timing.
Changes:
- Added
dispose()methods to properly release GPU textures and other resources in notification, media player, and widget classes - Added preemptive texture clearing before loading new images in notification bodies and MPRIS album art
- Enhanced MPRIS player cleanup to cancel ongoing downloads and clear textures on destruction
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/underlay/underlay.vala | Added dispose() method to properly unparent both child and underlay_child widgets |
| src/notificationGroup/notificationGroup.vala | Added dispose() method to stop animations and clear collections; includes extensive whitespace formatting changes |
| src/notification/notification.vala | Added dispose() method to clear image resources and remove timeouts; added body_image clearing before loading new textures |
| src/controlCenter/widgets/mpris/mpris_player.vala | Added dispose() method calling before_destroy(); enhanced before_destroy() to cancel downloads and clear album art textures; added texture clearing before loading new album art |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4996fbb to
549a38d
Compare
…memory leaks Add proper resource cleanup in dispose() methods for several widget classes and scale body images to display size to prevent GPU memory (VRAM/GTT) leaks. Changes: - Notification: Add dispose() to clear img, img_app_icon, body_image and remove pending timeouts. Also clear body_image before setting new paintable. - Notification: Scale body images to display size instead of loading full resolution (e.g., 2560x1440 screenshots were using ~11MB GPU memory each, now use ~120KB when scaled to 200x100 display size). - MprisPlayer: Add dispose() and enhance before_destroy() to cancel downloads and clear album_art/background_picture textures. Clear textures before loading new album art. - Underlay: Add dispose() to properly unparent children. - NotificationGroup: Add dispose() to skip/null animations and clear collections. These fixes address GPU memory accumulation observed when: - Notifications with large images (screenshots) are shown - MPRIS players update album art frequently - Notification groups are expanded/collapsed The body image scaling fix provides ~100x reduction in GPU memory per image notification containing large images like screenshots.
ErikReider
left a comment
There was a problem hiding this comment.
Just a few nits, but looks good. You used AI right? I have nothing against its usage, but I'd appreciate it if you mentioned that in the PR description.
|
|
||
| // NOTE: We removed the unrealize() call that was here. | ||
| // It was causing GPU memory leaks on repeated open/close cycles | ||
| // because textures had to be re-uploaded each time. | ||
| // If keyboard shortcuts stop working after clearing notifications, | ||
| // a different fix may be needed (e.g., reset keyboard mode via layer shell). |
There was a problem hiding this comment.
Yeah, I agree. This was a temporary hack to fix keyboard input.
This was needed though to know which output the ControlCenter was opened on as the underlying wl_surface doesn't get destroyed / the wl_surface::leave signal isn't called when unmapped. Without a wl_surface::leave, the ::enter signal is only called once for each output, which makes tracking which monitor the ControlCenter is open on impossible.
So please remove this change as it should be in a separate PR
| private void set_style_urgency () { | ||
| // Reset state | ||
| // Reset state - clear paintable to release GPU memory | ||
| body_image.set_paintable (null); |
There was a problem hiding this comment.
Why remove the paintable here?
| private void set_inline_reply () { | ||
| // Reset state | ||
| // Reset state - clear paintable to release GPU memory | ||
| body_image.set_paintable (null); |
Summary
This PR adds proper
dispose()methods to several widget classes to prevent GPU memory (VRAM/GTT) leaks when notifications and media players are created and destroyed.Problem
Over time, swaync accumulates GPU memory (observable via
/sys/class/drm/card*/device/mem_info_vram_usedor GTT usage) due to:Gdk.Textureobjects backingimg,img_app_icon, andbody_imageare not explicitly releasedUnderlayclass (parent ofMprisPlayer) doesn't clean up its children on disposalNotificationGroupanimations can hold references that prevent garbage collectionChanges
src/notification/notification.valadispose()method that clearsimg,img_app_icon,body_image, removes pending timeouts, and nulls gesture/controller referencesbody_image.set_paintable(null)before setting new paintable inset_body()src/controlCenter/widgets/mpris/mpris_player.valadispose()method that callsbefore_destroy()before_destroy()to cancel ongoing downloads and clearalbum_art/background_picturetexturesupdate_album_art()src/underlay/underlay.valadispose()method that properly unparents both_childand_underlay_childsrc/notificationGroup/notificationGroup.valadispose()method that skips/nulls animations, disconnects handlers, and clears collectionsTesting
Tested by monitoring VRAM usage while:
GPU memory now stabilizes after dismissal instead of continuously growing.
Technical Details
In GTK4/Wayland,
Gdk.Textureobjects are backed by GPU memory. When new paintables are assigned toGtk.ImageorGtk.Picturewidgets without clearing the old ones first, or when widgets are destroyed without explicit cleanup, the GPU memory may not be immediately released due to:The
dispose()pattern ensures deterministic cleanup of GPU resources when widgets are destroyed.