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

Use DefWindowProcW in win32 examples #5961

Closed
wants to merge 1 commit into from

Conversation

markreidvfx
Copy link
Contributor

The current win32 examples will truncate the window title if you try to modify it with Platform_SetWindowTitle.

Before
image
After
image

It took me a while to figure out it was because the examples are using DefWindowProc instead DefWindowProcW.
The issue is setWindowTextW sends a WM_SETTEXT message to WndProc and gets interpreted as ascii.
Maybe this fix will help others from having to debug this issue when they are getting started using the examples :)

Fixes the window title from being truncated on calls
to Platform_SetWindowTitle. Stops the WM_SETTEXT message
that happens when calling setWindowTextW from being
interpreted as ascii.
@ocornut
Copy link
Owner

ocornut commented Dec 6, 2022

Hello,

Thanks for the PR but this needs clarification.
It is custom and considered correct to call win32 functions without the A/W suffix, the selection is done based on your compile-time UNICODE settings.

Please clarify how you are compiling the examples and what to do to repro said issue. (So i can confirm it locally).

thanks!

@markreidvfx
Copy link
Contributor Author

Oh, your right, now I see the /D UNICODE in the bat files, my build script wasn't doing that. Neverminded then, sorry to waste your time, I'll drop this pull request.

@markreidvfx markreidvfx closed this Dec 6, 2022
@ocornut
Copy link
Owner

ocornut commented Dec 6, 2022

I am still interested in this.
Can you clarify what you were doing to trigger the issue, even without building with UNICODE ?

@markreidvfx
Copy link
Contributor Author

markreidvfx commented Dec 6, 2022

In the docking branch, if your remove /D UNICODE /D _UNICODE from any of the win32_directx bat files

Add to this main loop

ImGuiViewport* viewport = ImGui::GetMainViewport();
ImGui::GetPlatformIO().Platform_SetWindowTitle(viewport, "Dynamic Title");

You get the behavior I was getting.

@nicolasnoble
Copy link
Contributor

9 chances out of 10 the string was still getting converted to full unicode before passed to DefWindowProc, which would then have been tied to DefWindowProcA, and stopped at the first '\0' from the unicode conversion of Dynamic.

@markreidvfx
Copy link
Contributor Author

@nicolasnoble yes, without the complier defs, ImGui_ImplWin32_SetWindowTitle converts the string to uft16 with MultiByteToWideChar , calls SetWindowTextW and the utf16 string gets passed to DefWindowProcA

@nicolasnoble
Copy link
Contributor

Yep, that's the wrong codepath :) We shouldn't call SetWindowTextW on non-unicode builds at all. The right PR would be (1) call SetWindowText instead of SetWindowTextW, and (2) have an #if defined(UNICODE) || defined(_UNICODE) before calling SetWindowText to decide whether or not to convert the input string to unicode.

ASCII software will simply misbehave if passed a non-ASCII UTF-8 string, but there's little to do in this case...

@ocornut
Copy link
Owner

ocornut commented Jan 24, 2023

While #5975 is not as evident because it is a demo app, we should definitively make the multi-viewport backend behave with applications not compiled with /D UNICODE.

Strangely with that setup, ImGui_ImplWin32_SetWindowTitle() works on secondary viewports but not the main viewport created by the example. Investigating the difference now.

ocornut added a commit that referenced this pull request Jan 24, 2023
ocornut pushed a commit that referenced this pull request Jan 24, 2023
…also call DefWindowProcW(). (#5725, #5961, #5975)

Fixes the window title from being truncated on calls to Platform_SetWindowTitle. Stops the WM_SETTEXT message that happens when calling setWindowTextW from being interpreted as ascii.
@ocornut
Copy link
Owner

ocornut commented Jan 24, 2023

My apologies for misunderstanding this the first round. I have now merged your PR as fe0a24f
Thank you!

@ocornut ocornut closed this Jan 24, 2023
kjblanchard pushed a commit to kjblanchard/imgui that referenced this pull request May 5, 2023
…also call DefWindowProcW(). (ocornut#5725, ocornut#5961, ocornut#5975)

Fixes the window title from being truncated on calls to Platform_SetWindowTitle. Stops the WM_SETTEXT message that happens when calling setWindowTextW from being interpreted as ascii.
ocornut added a commit that referenced this pull request Jun 13, 2023
ocornut pushed a commit that referenced this pull request Dec 29, 2023
… input for text in utf-8 code page. (#7174)

Similar to #6785, #6782, #5725, #5961 for for GLFW backend.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants