Restore ANSI win32 API functions (PR #25965)
Maarten
wxWidgets is always build in UNICODE using the WIDE win32 API functions. But applications using it might be built without UNICODE, using the ANSI win32 functions.
Right now wxWidgets #undefs some of the win32 API functions and defines them as inline functions using the WIDE win32 API. Making these unavailable for applications that want to use the ANSI versions later.
Fix this by inlining the functions as WIDE only when UNICODE was already defined, not when wxWidgets defines it themselves in platform.h.
See #25803
cc @jamarr81 would this work?
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/25965
Commit Summary
- 9df7257 Remove unneeded winundef.h includes
- 3cbe762 Partially revert 53b3b979fe (Remove tests for _UNICODE too)
- 3378f9f Restore ANSI win32 functions if UNICODE was not set
- bafbaa6 White-space and style cleanup in winundef.h
File Changes
(10 files)
- M include/wx/image.h (4)
- M include/wx/msw/ole/automtn.h (4)
- M include/wx/msw/winundef.h (312)
- M include/wx/platform.h (1)
- M src/common/fmapbase.cpp (4)
- M src/common/fontcmn.cpp (5)
- M src/common/fontmap.cpp (5)
- M src/generic/dirctrlg.cpp (3)
- M src/generic/helpext.cpp (5)
- M src/generic/scrlwing.cpp (5)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/25965.patch
- https://github.com/wxWidgets/wxWidgets/pull/25965.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- ce274d3 fix
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
Thanks, I think this should work and I can't think of anything else that this would break, but I'd prefer to condense all the wrapper definitions using a helper macro before merging this.
> + #if wxUSE_UNICODE_WINDOWS_H
return CreateDialogW(hInstance, pTemplate, hwndParent, pDlgProc);
+ #else
+ return CreateDialogA(hInstance, pTemplate, hwndParent, pDlgProc);
+ #endif
Could we just define our own wxFUNC_AW() macro once, depending on wxUSE_UNICODE_WINDOWS_H value, and then replace all these fragments with just
- #if wxUSE_UNICODE_WINDOWS_H - return CreateDialogW(hInstance, pTemplate, hwndParent, pDlgProc); - #else - return CreateDialogA(hInstance, pTemplate, hwndParent, pDlgProc); - #endif + return wxFUNC_AW(CreateDialog)(hInstance, pTemplate, hwndParent, pDlgProc);
?
> + #define wxLPCTSTR LPCWSTR + #define wxLPSTR LPWSTR + #define wxLPWNDCLASS LPWNDCLASSW + #define wxDOCINFO DOCINFOW + #define wxLPFINDREPLACE LPFINDREPLACEW
Minor, but these could be typedefs (or even using) if we try to minimize the use of preprocessor (possibly not really helpful in this header...).
> // // This looks quite ugly here but allows us to write clear (and correct!) code // elsewhere because the functions, unlike the macros, respect the scope
Maarten
@MaartenBent pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent commented on this pull request.
> + #define wxLPCTSTR LPCWSTR + #define wxLPSTR LPWSTR + #define wxLPWNDCLASS LPWNDCLASSW + #define wxDOCINFO DOCINFOW + #define wxLPFINDREPLACE LPFINDREPLACEW
typedef/using doesn't work when these types are not defined. Apparent in the non-PCH builds.
This can happen for example when winundef.h is included but windows.h is not, like in wrapcdlg.h or ioswrap.h or iosfwrap.h.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 1 commit.
- 92900ac fix
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#25965)Not sure how to fix these last errors in CI. In this case UNICODE is not defined by the build system / user. So wxWidgets defines is. Then windows.h is included (with UNICODE defined), resulting in TEXT and other defines/macros becoming the WIDE variant.
But later in the application code, after winundef.h, the ANSI variants will be used. But TEXT is still the WIDE variant.
I think I'll just let it default to the WIDE variant, unless the application defines wxWIN32_UNDEF_AS_ANSI before including wx headers:
#if defined(WXBUILDING) || !defined(wxWIN32_UNDEF_AS_ANSI) // wide #else // ansi #endif
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25965)We could define something (_UNICODE?) in the sample to work around this problem there but it seems indeed safer and better to require predefining some symbol if you really need A versions of the API. But arguably, wxNO_WIN32_WIDE_FUNCTIONS (or just wxNO_WIN32_W) would be more in line with our preprocessor symbol naming conventions.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 4 commits.
- 8593de0 Remove unneeded winundef.h includes
- 5228892 White-space and style cleanup in winundef.h
- 2b2160c Restore ANSI win32 functions if UNICODE is not set
- dff7453 Use WIDE variant of win32 API functions
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
jamarr
jamarr81 left a comment (wxWidgets/wxWidgets#25965)@MaartenBent @vadz tysm for following up on this! I haven't had a chance to revisit it; however, these changes look good to me on paper.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz approved this pull request.
Looks good, thanks!
It would be great if this could be tested in some application which needed this, but if nobody does it soon, I'll just merge this and hope that it works for them.
> @@ -847,13 +847,13 @@ class ClassRegistrar
bool IsRegistered() const { return m_registered == 1; }
// try to register the class if not done yet, return true on success
- bool Register(const WNDCLASS& wc)
+ bool Register(const WNDCLASSW& wc)
It would be good to add a test for compiling this file with wxNO_WIN32_W to at least one MSW CI job to catch any errors that could be introduced into it later by forgetting to explicitly use W symbol versions.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.