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

Support unicode in window title under DesktopGL #6335

Merged
merged 1 commit into from
Jun 18, 2018

Conversation

Jjagg
Copy link
Contributor

@Jjagg Jjagg commented Jun 10, 2018

This issue was reported on the forums: http://community.monogame.net/t/encoding-problem/10700

Encodes window title with UTF-8 so SDL handles it right.
https://wiki.libsdl.org/SDL_SetWindowTitle

Before:
image

After:
image

@Jjagg Jjagg requested a review from harry-cpp June 10, 2018 17:08
@@ -431,7 +431,7 @@ public static void SetFullscreen(IntPtr window, int flags)
public static d_sdl_setwindowsize SetSize = FuncLoader.LoadFunction<d_sdl_setwindowsize>(NativeLibrary, "SDL_SetWindowSize");

[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
public delegate void d_sdl_setwindowtitle(IntPtr window, string title);
public delegate void d_sdl_setwindowtitle(IntPtr window, ref byte value);
Copy link
Member

Choose a reason for hiding this comment

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

You should create a helper function which accepts string in SDL2.cs and not modify any code in SDLGameWindow.

Also is in allowed instead of ref?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated! We can't use in because it's a C# 7.2 feature.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, ok :(

Copy link
Contributor

Choose a reason for hiding this comment

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

@Jjagg @cra0zy Not sure if you're aware, but there's also directional attributes which can be used to optimise marshaling. So even though you can't use in, you can still use InAttribute. I know it works with extern, but I'm not sure when using delegates as I haven't tried that before. But it should work the same way I'd assume. e.g.

[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
private delegate void d_sdl_setwindowtitle(IntPtr window, [In] byte[] value);
SDL_SetWindowTitle(handle, Encoding.UTF8.GetBytes(title));

The code is fine though. This is not a nit-pick, just more of an FYI. Hope it's useful!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we should probably do a run through for all delegates and add In or Out where applicable (it does work the same). I didn't add it because we don't use them anywhere.

Copy link
Member

@harry-cpp harry-cpp Jun 12, 2018

Choose a reason for hiding this comment

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

Directional attributes are optional. You apply them to method parameters when you want to
alter the default behavior of the marshaler. If you omit directional attributes from a method
parameter, the marshaler determines the directional flow based on the type of the
parameter (value or reference) and its modifiers, if any.

https://msdn.microsoft.com/en-us/library/77e6taeh(v=vs.85).aspx

@tomspilman tomspilman changed the title [DGL] Support unicode in window title Support unicode in window title under DesktopGL Jun 18, 2018
@tomspilman tomspilman added this to the 3.7 Release milestone Jun 18, 2018
@tomspilman
Copy link
Member

Merging!

@tomspilman tomspilman merged commit 61e5807 into MonoGame:develop Jun 18, 2018
@Jjagg Jjagg deleted the sdl_unicode branch June 18, 2018 11:45
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.

4 participants