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 a manifest on Windows for UTF8 handling #1752

Closed
wants to merge 2 commits into from

Conversation

vrabaud
Copy link
Collaborator

@vrabaud vrabaud commented Nov 13, 2023

This first reverts #1693 (except for test_cmd.sh) and applies a manifest on Windows platforms.

CMake handles manifests as source files on Windows but most likely only for MSVC (cf https://cmake.org/cmake/help/v3.4/release/3.4.html#other).

@vrabaud vrabaud force-pushed the utf8_manifest branch 3 times, most recently from 7afca00 to e9a5c7d Compare November 14, 2023 09:57
add_custom_command(
TARGET ${PROG}
POST_BUILD
COMMAND "mt.exe" -manifest \"${CMAKE_SOURCE_DIR}\\src\\utf8.manifest\"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this can work on MinGW, there is no mt.exe...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can MINGW access the Windows SDK and therefore "mt.exe"?
It seems adding the manifest as source (or at least the .rc) works, cf https://doc.magnum.graphics/magnum/platforms-windows.html . Can you confirm this is the way to go? I have no experience with manifest files, @wantehchang just found this cleaner solution for handling UTF8.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think one can assume MinGW users/devs will have access to the Windows SDK.

I might be completely wrong here, but perhaps a conversation is to be had w/ the MinGW (or is it Cygwin?) project to ship a pre-built UTF8 object manifest that can be statically linked in then, like the existing ("regular"?) one?

Copy link
Contributor

@kmilos kmilos Nov 15, 2023

Choose a reason for hiding this comment

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

Furthermore, the https://cygwin.com/git/?p=cygwin-apps/windows-default-manifest.git;a=tree also seems to wrap that default manifest into a .rc file (that's then compiled to an object), and resource files can be dealt with w/o a problem in MinGW toolchains (windres is shipped, so just add them as another source file). Maybe that could indeed be a temporary workaround?

With a gotcha from your magnum link:

A downside is that MinGW is not able to merge information from multiple manifests like the MSVC toolchain can.

So a complete UTF8 .rc needs to be constructed it seems?

Copy link
Contributor

@kmilos kmilos Nov 15, 2023

Choose a reason for hiding this comment

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

Maybe take a lead from RStudio? Seems like wrapping the manifest in the .rc file is the way to go indeed...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Miloš: Thank you very much for the research.

The UTF-8 support for libavif is simpler than the UTF-8 support for RStudio, because the libavif library itself has only one function, avifIOCreateFileReader(), that takes a filename input parameter, and that function faithfully passes filename to fopen() with no processing. So we can document that the libavif library honors the code pages selected by the application, and we just need to solve the UTF-8 support problem for the three command-line programs, avifdec, avifenc, and are_images_equal. And it is fine to solve the UTF-8 support problem on Windows 10 and later only so that we can use the simple solution of the manifest file.

target_link_libraries(${PROG} avif_apps)
if(WIN32)
# Add a manifest file to deal with UTF8.
if(CMAKE_C_COMPILER_ID MATCHES "MSVC")
Copy link
Contributor

Choose a reason for hiding this comment

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

if(MSVC) simply

@vrabaud
Copy link
Collaborator Author

vrabaud commented Jan 8, 2024

Go with the cleaner #1900

@vrabaud vrabaud closed this Jan 8, 2024
@vrabaud vrabaud deleted the utf8_manifest branch January 8, 2024 10:52
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.

None yet

3 participants