Skip to content

Re-introduce Web examples#3637

Merged
daxpedda merged 1 commit intorust-windowing:masterfrom
daxpedda:web-examples
Apr 18, 2024
Merged

Re-introduce Web examples#3637
daxpedda merged 1 commit intorust-windowing:masterfrom
daxpedda:web-examples

Conversation

@daxpedda
Copy link
Copy Markdown
Member

@daxpedda daxpedda commented Apr 18, 2024

This adds back the custom cursor stuff to the examples.
The only other example I didn't port was the scale factor one, which I didn't feel make a whole lot sense after #2859.
The web example only showed events inside the window, which we don't need anymore if we print them properly in the console, which I've done now.

As part of showing thing properly in the console, I've switched to using tracing instead of println! in the relevant examples.

I didn't find any mention of how to run examples on any target, e.g. Android or iOS.
So I feel we either have to add docs for all of these targets or none. Ergo I'm also tending towards removing our run-wasm dependency.
WDYT?

Fixes #3473.

@daxpedda daxpedda added the DS - web Affects the Web backend (WebAssembly/WASM) label Apr 18, 2024
@daxpedda daxpedda added this to the Version 0.30.0 milestone Apr 18, 2024
kchibisov
kchibisov requested changes Apr 18, 2024
View reviewed changes
examples/window.rs Outdated
custom_cursors[1].clone(),
event_loop.create_custom_cursor(CustomCursor::from_url(
format!(
"https://picsum.photos/128?random={}",
Copy link
Copy Markdown
Member

@kchibisov kchibisov Apr 18, 2024

Choose a reason for hiding this comment

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

Could we use url to github for that? Like we have cursors laying around in the repo already in png formats, so they should be fine.

Copy link
Copy Markdown
Member Author

@daxpedda daxpedda Apr 18, 2024

Choose a reason for hiding this comment

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

Well the point is to always have it be a different picture, so you can see what happens when its not loading immediately.

@daxpedda daxpedda force-pushed the web-examples branch 3 times, most recently from bded977 to f011577 Compare April 18, 2024 15:27
madsmtm
madsmtm approved these changes Apr 18, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

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

I'm also tending towards removing our run-wasm dependency.

CC @rukai

@daxpedda
Copy link
Copy Markdown
Member Author

daxpedda commented Apr 18, 2024

I'm also tending towards removing our run-wasm dependency.

I just went ahead and did that, we can consider re-introducing it if we decide we want it after all.

@daxpedda daxpedda merged commit c15fa6e into rust-windowing:master Apr 18, 2024
@rukai
Copy link
Copy Markdown
Contributor

rukai commented Apr 18, 2024 •
edited
Loading

For the web example drawing events to the dom: that was valuable for testing the examples on mobile devices which do not have a console accessible in their browsers.

For run-wasm, if there's an alternative way you want users to run wasm examples then by all means swap to that. But currently there is no documented way to run wasm examples so I'm quite confused.
This all or nothing approach to documentation is really strange, it doesn't have to be perfect but surely we should support the major platforms well with documentation?

@daxpedda
Copy link
Copy Markdown
Member Author

daxpedda commented Apr 18, 2024

For the web example drawing events to the dom: that was valuable for testing the examples on mobile devices which do not have a console accessible in their browsers.

Good point!

For run-wasm, if there's an alternative way you want users to run wasm examples then by all means swap to that. But currently there is no documented way to run wasm examples so I'm quite confused.
This all or nothing approach to documentation is really strange, it doesn't have to be perfect but surely we should support the major platforms well with documentation?

My original comment:

So I feel we either have to add docs for all of these targets or none. Ergo I'm also tending towards removing our run-wasm dependency.

Agreed, this is stupid. I think this was just my first thought: if I'm gonna start adding documentation, where is the documentation for the other targets?

run-wasm is not required to run the Wasm examples, there are plenty of Rust Wasm runners out there that can be configured globally, e.g. I'm using wasm-server-runner. So I don't need any additional work at my local Winit repo to run the examples.

I'm aware of the advantages run-wasm offers in comparison to other runners and I don't think Winit really needs them. This is why I decided to remove run-wasm, in addition to it not being documented.

Though I'm happy to re-add them upon popular request.

Apologies for not originally stating my full reasons in the OP.

@rukai
Copy link
Copy Markdown
Contributor

rukai commented Apr 18, 2024 •
edited
Loading

All good.
I agree that cargo-run-wasm requires a certain level of integration with the repo that isn't ideal. I think the solution here is to just point users towards wasm-server-runner in the readme.

Edit: or probably just a readme specific to the examples directory

@daxpedda
Copy link
Copy Markdown
Member Author

daxpedda commented Apr 18, 2024

Would stick with keeping it in the README, pending input from the other maintainers.

Happy to look at a PR!

@madsmtm
Copy link
Copy Markdown
Member

madsmtm commented Apr 18, 2024 •
edited
Loading

I think I'd prefer it in winit::platform::web, and then perhaps instead link more prominently to that from lib.rs

@daxpedda
Copy link
Copy Markdown
Member Author

daxpedda commented Apr 18, 2024

I think I'd prefer it in winit::platform::web, and then perhaps instead link more prominently to that from lib.rs

I think it definitely needs something visible from the repo, when you clone the repo you don't automatically run cargo doc, where usually "repo documentation" isn't found.

@madsmtm
Copy link
Copy Markdown
Member

madsmtm commented Apr 18, 2024

I'd also be fine with a examples/README.md with instructions specific to the examples

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@madsmtm madsmtm madsmtm approved these changes

@kchibisov kchibisov kchibisov approved these changes

Assignees

No one assigned

Labels

DS - web Affects the Web backend (WebAssembly/WASM)

Milestone

Version 0.30.0

Development

Successfully merging this pull request may close these issues.

Merge web example into window example

4 participants

@daxpedda @rukai @madsmtm @kchibisov