Skip to content

Implement Debug for Context, Surface and Buffer #268

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

madsmtm
Copy link
Member

@madsmtm madsmtm commented Jun 5, 2025

Builds upon #267.

@madsmtm madsmtm added the enhancement New feature or request label Jun 5, 2025
@madsmtm madsmtm force-pushed the madsmtm/context-easier branch from e2d94d0 to b97e10e Compare June 5, 2025 13:58
@madsmtm madsmtm force-pushed the madsmtm/context-easier branch from b97e10e to 831973c Compare August 12, 2025 22:38
Base automatically changed from madsmtm/context-easier to master August 12, 2025 22:45
@madsmtm madsmtm marked this pull request as ready for review August 12, 2025 22:45
@@ -163,6 +166,7 @@ enum Buffer {
Wire(Vec<u32>),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this print the entire pixel buffer?

Copy link
Member Author

@madsmtm madsmtm Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I guess so. Might make sense to make a small wrapper struct BufferMinimalDebug(Vec<u32>) to avoid that.

Comment on lines +117 to 120
#[derive(Debug)]
pub struct BufferImpl<'a, D: ?Sized, W> {
native_window_buffer: NativeWindowBufferLockGuard<'a>,
buffer: Vec<u32>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we want to spam the user with all pixel values 😅

Copy link
Member

@notgull notgull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would prefer if it just printed some platform-generic info like width, height, current state, etc instead of platform-specific info. Would probably be more parsable and helpful for users.

@MarijnS95
Copy link
Member

I do think having that, in addition to perhaps having some platform-specific handle (maybe even just Wayland vs X11) and format should be useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

3 participants