Skip to content
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

Update bindings 3 (no merging needed) #129

Merged
merged 12 commits into from
Sep 15, 2024
Merged

Update bindings 3 (no merging needed) #129

merged 12 commits into from
Sep 15, 2024

Conversation

hwsmm
Copy link
Collaborator

@hwsmm hwsmm commented Aug 11, 2024

To be honest, I expected these PRs to last a few months before merging, but I ended up making the third updating PR in a relatively short time frame.

I don't mean it's a bad thing, but it's quite unexpected...

Same as before, feel free to take over maintaining if I look inactive here!

Relevant osu!framework branch


Notable changes:

  • Due to SDL_pen API rework, SDL no longer produces mouse events for pen inputs. I tried emulating mouse in the above o!f branch, but I'm not sure if it's in the right direction. We cannot adjust output area in this API, so making another handler for this felt too much to me. Please give it a test/review!

    You need to be on macOS/Linux(X11/Wayland) to test this since they don't have backends for other platforms yet, and also disable in-game OTD before testing this.

  • SDL_bool is now C bool, and every function that returns error with SDL_SetError now returns SDL_bool instead of int. It probably needs review.

  • GPU, Process bindings are added.

  • Removed GDK bindings for now as we don't provide binaries for Xbox. Do we really need it?

  • WinRT support is removed.

Comment on lines 8 to 12
[Typedef]
public enum SDL_bool
public enum SDL_bool : byte
{
SDL_FALSE = SDL3.SDL_FALSE,
SDL_TRUE = SDL3.SDL_TRUE
Copy link
Member

Choose a reason for hiding this comment

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

Now that SDL_bool can be C++ bool I'm not sure that it still holds that SDL_FALSE is 0x00 and SDL_TRUE is 0x01. We need to check this for the platforms that we support.

Copy link
Member

Choose a reason for hiding this comment

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

Upstream assures that SDL_FALSE is 0 and SDL_TRUE is 1. The current C# definition is fine then. I'll definitely write some tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are we able to replace SDL_bool with bool? Their definition there is binary compatible with C# afaik,

Copy link
Member

Choose a reason for hiding this comment

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

Using a bool param/return value in a callback function (such as SDL_SetEventFilter) doesn't work. Fails at runtime with:

Unhandled exception. System.InvalidProgramException: Non-blittable parameter types are invalid for UnmanagedCallersOnly methods.

Adding [return: MarshalAs(UnmanagedType.I1)] doesn't help; I also wasn't able to add the attribute to the function pointer definition.

Closest thing we can do is make SDL_bool a struct and add implicit bool conversions.

smoogipoo
smoogipoo previously approved these changes Sep 14, 2024
Importantly, this brings in a fix for SDL's gendynapi.py throwing an error.
For now, SDL_GDKSuspendGPU and SDL_GDKResumeGPU from SDL_gpu.h are missing
from auto-generated bindings. We can add them manually if needed.

These missing funtions cause warnings to be logged when running `generate_bindings.py`.
Susko3
Susko3 previously approved these changes Sep 15, 2024
@smoogipoo smoogipoo requested a review from Susko3 September 15, 2024 02:12
@Susko3 Susko3 merged commit 833814c into ppy:master Sep 15, 2024
1 check passed
This was referenced Sep 15, 2024
@hwsmm hwsmm deleted the update branch October 14, 2024 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants