Move SDL objects to TLS, allowing multiple windows to co-exist.

[hansl]

Fixes #47.

Thank you for helping out with embedded-graphics-simulator development! Please:

• Check that you've added passing tests and documentation
• Add a CHANGELOG.md entry in the Unreleased section under the appropriate heading (Added, Fixed, etc) if your changes affect the public API
• Run rustfmt on the project
• Run just build (Linux/macOS only) and make sure it passes. If you use Windows, check that CI passes once you've opened the PR.

PR description

Move SDL objects to TLS, allowing multiple windows to co-exist. This is an updated version of #37.

[hansl]

I have no idea why the half dependency would fail for this PR. There might be a moving transitive dependency that update that crate and now it refuses to build in Rust 1.61.

[hansl]

@jamwaffles Do you think this is something we could get merged in?

[pcvonz]

Would be awesome to get this merged in! For what it's worth, I pulled and re-based this locally and it works great. I have a device which uses both an led matrix and a display and it has been very helpful for testing. @jamwaffles @hansl

[rfuest]

I can merge the PR, but I'm not sure if I have all necessary permissions to create a new release on crates.io.

[rfuest]

Wouldn't polling the events for one window also discard the events for all other windows?

[hansl]

Wouldn't polling the events for one window also discard the events for all other windows?

I actually saw that too when building the example. In my own code that's not an issue so I missed this. There's now a global event vector for missed events. It's limited and unless someone never polls a window's events it should never be full. I tested it locally at least.

[almindor]

LGTM

[rfuest]

Adding a example was a good idea.

I've found one bug: It is no longer possible to close the simulator by clicking the close window icon in the titlebar of the window. It still works when one window is displayed, but as soon as at least two windows are open the simulator can only be quit via the ESC key.

[hansl]

@rfuest PTAL, thanks for the review.

[rfuest]

The simulator was always intended to be something to quickly implement a demo and I'm starting to think that adding this feature in this way might no match this original idea.

Here is an experiment I did, which instead let's the user handle the SDL2 initialization and event handling: rfuest@e2d1314. In my opinion this keeps the code inside the simulator simpler and is also more flexible for the user, who can handle SDL events any way they like. Please take a look at that code and let me know if this would work for you.

[hansl]

@rfuest I don't mind either way. I fixed the problem in my code by basically reimplementing a subset of this library. I just thought more people would like this functionality, as it is fully backward compatible.

[rfuest]

@rfuest I don't mind either way. I fixed the problem in my code by basically reimplementing a subset of this library. I just thought more people would like this functionality, as it is fully backward compatible.

With the addition of the new SimulatorEvent::WindowEvent it did technically become a breaking change, although this wouldn't be a problem in most cases, because I don't expect a lot of users will use a match expression without a wildcard pattern for the event.

I think that I would prefer my solution, because it also provides advantages for users who don't want to use multiple windows. Not exiting on ESC, which you mentioned in a commend, would be one use case when only one window is used.

Unfortunately I'm not able to open a PR for the code I suggested, because I wouldn't be able to merge it. The permissions are setup in a way where PRs need an approving review from a code owner and I can't review my own code. I'm effectively blocked from writing any code for e-g and the related projects at the moment.

Out of curiosity: Does your application need multiple windows or would it also work by displaying multiple displays in a single window? This had been the way I had intended to implement multi display support in the past. Only having one window that contains multiple displays has the advantage that you can specify the relative positioning in the app, without having to worry about how the window manager places the separate SDL windows. I imagine that creating a simulation for a Stream Deck clone with one display per key would get confusing if you use one window per display. But that's a separate feature and multi windows support is none the less.

[jamwaffles]

The permissions are setup in a way where PRs need an approving review from a code owner and I can't review my own code.

I changed your role from "Member" to "Owner" in the e-g core team group, so you might be able to do more now - if nothing else, to change the setting in repos of your choice to allow merges without reviews. If that doesn't work, I don't know what other levers I have available to pull.

[rfuest]

@jamwaffles Thanks for the quick response, but I don't think that did a lot. I can override the branch protection rules for this PR, so maybe I could merge a PR I create in this repo. But I tried to do the same with embedded-graphics/embedded-graphics#738, where I don't have the option to override the branch protection rules. In neither repo I have the permissions to disable the protection rules in the settings.

[jamwaffles]

I just made e-g-core-team admin for most repos in the org (was previously "Maintain") which may now allow you to merge e-g/738.

In neither repo I have the permissions to disable the protection rules in the settings.

The description under "Admin" level implies this is possible, but maybe there's yet another knob I need to twizzle to make it work...

[rfuest]

@jamwaffles That has fixed it, thanks!

[jamwaffles]

Ah, finally! I've been a terrible steward of this project recently so my apologies for being slow and uncooperative with all this, but it sounds like I don't hold any keys any more which is great :)

[rfuest]

Ah, finally! I've been a terrible steward of this project recently so my apologies for being slow and uncooperative with all this, but it sounds like I don't hold any keys any more which is great :)

Who knows perhaps you will come back in the future to take a more active role in the project again, but until then I'll try to keep the project alive and not mess anything up too much ;).

Would you mind to also make me a co-owner of the GH org? Having at least two owners is recommended by GH: https://docs.github.com/en/organizations/managing-peoples-access-to-your-organization-with-roles/maintaining-ownership-continuity-for-your-organization#about-maintaining-ownership-continuity-for-your-organization

[jamwaffles]

Who knows perhaps you will come back in the future to take a more active role in the project again

I'd still like to get integer-only AA working at some point, so maybe!

Would you mind to also make me a co-owner of the GH org?

You're an Owner now :)

[rfuest]

I'd still like to get integer-only AA working at some point, so maybe!

That would be a nice to have feature. Maybe we'll even have some support for alpha blending by then, which I might work on after the mess that is PR embedded-graphics/embedded-graphics#711 is merged in smaller chunks.

You're an Owner now :)

Thanks again!

[hansl]

Out of curiosity: Does your application need multiple windows or would it also work by displaying multiple displays in a single window?

Multi window is just easier as I have multiple draw buffers (which ends up in different code path on the embedded device). I guess I could technically make it a single display, but for the developer it's easier to see where one display end and the other begins. Just simpler code.

Note that I also duplicated the code because I support sdl3 in my base.

I'll close this PR and if you want to merge this branch into another PR go for it.