-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Make "fill" module used by examples more useful + add an input tester example #4219
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
base: master
Are you sure you want to change the base?
Conversation
|
Huh, cargo-deny is failing on |
|
|
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'm very much in favour of getting this in! Having examples that actually do something interesting would help a lot with debugging rendering issues, as well as figuring out API warts in general.
(And uh, I also didn't see this in my notifications, so similarly, feel free to ping me on Matrix if there's stuff I miss).
| #[cfg(web_platform)] | ||
| use wasm_bindgen::prelude::wasm_bindgen; | ||
| #[cfg(web_platform)] | ||
| #[wasm_bindgen(start)] |
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.
Nit: Is this still required? I thought that wasm-bindgen supported fn main nowadays?
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 had trouble getting it to work properly with a normal fn main, don't know why.
| tracing = { version = "0.1.40", default-features = false } | ||
|
|
||
| [dev-dependencies] | ||
| font8x8 = "0.3.1" |
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.
Nit: Seems like this hasn't been updated in 4 years. Maybe there's something a bit more maintained we can use?
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 considered it, but because it's "just" a font, I figured it probably wasn't that important that it's unmaintained 🤔 Maybe the decision between different debugging/embedded bitmap font crates could better be made based off of license or looks?
Actually, I think I found a potential snafu: the font data font8x8 is based on says it's public domain, but the maintainer/author seems to located in Germany, and there isn't a workaround license like CC0 to make up for the fact that "speech act" public domain dedications don't work under German law? This is almost certainly not a problem, but it COULD be one, hypothetically. Maybe I'll open an issue on it. https://github.com/dhepper/font8x8
| [target.'cfg(not(target_family = "wasm"))'.dev-dependencies] | ||
| web-time = "1" |
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.
Nit: Slight preference for doing:
#[cfg(target_family = "wasm")]
use web_time::Instant;
#[cfg(not(target_family = "wasm"))]
use std::time::Instant;In example code too.
| </head> | ||
| <body> | ||
| </body> | ||
| </html> No newline at end of 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.
Nit: Missing trailing newline
| @@ -0,0 +1,309 @@ | |||
| //! Basic winit interactivity example. | |||
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.
Nit: Maybe call the example something like interactivity.rs or drawing_app.rs?
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, you can group stuff in a folder, like examples/drawing_app/main.rs + examples/drawing_app/index.html
| // Don't run hundreds of thousands of times per second even if the platform asks. | ||
| // (Can't sleep on web, so gated off there. Browsers won't ask for too much.) | ||
| #[cfg(not(web_platform))] | ||
| { | ||
| std::thread::sleep(std::time::Duration::from_millis(1)); | ||
| } |
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 fact that you currently need this kind of stuff is exactly why this example is so useful!
| for _y in -(thickness as i32) / 2..(thickness as i32 + 1) / 2 { | ||
| for _x in -(thickness as i32) / 2..(thickness as i32 + 1) / 2 { |
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.
Nit: I prefer shadowing in these situations (use y instead of _y).
| height: r[3].try_into().unwrap(), | ||
| }) | ||
| .collect::<Vec<_>>(); | ||
| buffer.present_with_damage(&rects).expect("Failed to present the softbuffer buffer"); |
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.
It seems like you're reusing the buffer in your impl? That doesn't work unless buffer.age() == 1 (which it e.g. isn't on macOS).
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.
Good catch!
changelogmodule if knowledge of this change could be valuable to usersDev-only change.
Adds a
fill_window_with_fnfunction to thefillmodule (only used by examples, not winit's outward-facing API), which allows examples to graphically display real information instead of just a blank screen. However,softbuffer, used internally by thefillmodule, is very slow on certain platforms (web, mac os), so the usage of this new function should be limited to basic test-that-this-thing-works examples, and not used for "game" or "application" style examples. Also adds a dev dependency onfont8x8so that examples can easily display text.Also adds an input tester example that uses the new function. The input tester is a low-complexity scribbling/drawing application. The tester is force-aware for Touch events when Touch force is supported.
The html file for the web version of the example has a fixed canvas size because of what seems to be a bug inside of
softbufferwhere it crashes on web if the width of the canvas is a weird value.I tested the input tester tested on windows and web.
tester_2025-05-08_17-08-37.mp4
This was inspired by a tablet tester I made for locally testing whether my tablet support fork worked (not part of this PR)
tablet_2025-05-06_15-35-34.mp4