utf8 / utf16 conversion modernization #3307
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Our Strutil utilities for utf8 <-> utf16 conversion were based on Windows specific calls that we had added even before C++11 was our minimum. Replace the Windows-specific code with std calls, and expose them in strutil.h for all platforms (why not?).
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Jan 30, 2022
Our Strutil utilities for utf8 <-> utf16 conversion were based on Windows specific calls that we had added even before C++11 was our minimum. Replace the Windows-specific code with std calls, and expose them in strutil.h for all platforms (why not?).
lgritz
added a commit
to lgritz/OpenImageIO
that referenced
this pull request
Feb 2, 2022
Our Strutil utilities for utf8 <-> utf16 conversion were based on Windows specific calls that we had added even before C++11 was our minimum. Replace the Windows-specific code with std calls, and expose them in strutil.h for all platforms (why not?).
aras-p
added a commit
to aras-p/OpenImageIO
that referenced
this pull request
Dec 1, 2024
OIIO 2.3.13 with PR AcademySoftwareFoundation#3307 changed MultiByteToWideChar/WideCharToMultiByte usage to C++11 <codecvt> functionality, but that has two issues: 1) it is *way* slower, primarily due to locale object access (on Visual C++ STL implementation in VS2022 at least). Since primary use case of these conversions is on Windows, maybe it is better to use a fast code path. 2) whole of <codecvt> machinery is deprecated with C++17 accross the board, and will be removed in C++26. I've kept the existing functions in there since otherwise it would have been an API break, but really maybe with OIIO they should have been un-exposed. Too late now though :( Performance numbers: doing ImageInput::create() on 1138 files where they are not images at all (so OIIO in turns tries all the input plugins on them). Ryzen 5950X, VS2022, Windows: - utf8_to_utf16 3851ms -> 21ms - utf16_to_utf8 1055ms -> 4ms Signed-off-by: Aras Pranckevicius <[email protected]>
5 tasks
lgritz
pushed a commit
that referenced
this pull request
Dec 1, 2024
OIIO 2.3.13 with PR #3307 changed MultiByteToWideChar/WideCharToMultiByte usage to C++11 <codecvt> functionality, but that has two issues: 1) it is *way* slower, primarily due to locale object access (on Visual C++ STL implementation in VS2022 at least). Since primary use case of these conversions is on Windows, maybe it is better to use a fast code path. 2) whole of <codecvt> machinery is deprecated with C++17 accross the board, and will be removed in C++26. I've kept the existing functions in there since otherwise it would have been an API break, but really maybe with OIIO 3.0 they should have been un-exposed. Too late now though :( ## Tests Performance numbers: doing ImageInput::create() on 1138 files where they are not images at all (so OIIO in turns tries all the input plugins on them). Ryzen 5950X, VS2022, Windows: - utf8_to_utf16 3851ms -> 21ms - utf16_to_utf8 1055ms -> 4ms Signed-off-by: Aras Pranckevicius <[email protected]>
lgritz
pushed a commit
to lgritz/OpenImageIO
that referenced
this pull request
Dec 1, 2024
…ation#4549) OIIO 2.3.13 with PR AcademySoftwareFoundation#3307 changed MultiByteToWideChar/WideCharToMultiByte usage to C++11 <codecvt> functionality, but that has two issues: 1) it is *way* slower, primarily due to locale object access (on Visual C++ STL implementation in VS2022 at least). Since primary use case of these conversions is on Windows, maybe it is better to use a fast code path. 2) whole of <codecvt> machinery is deprecated with C++17 accross the board, and will be removed in C++26. I've kept the existing functions in there since otherwise it would have been an API break, but really maybe with OIIO 3.0 they should have been un-exposed. Too late now though :( ## Tests Performance numbers: doing ImageInput::create() on 1138 files where they are not images at all (so OIIO in turns tries all the input plugins on them). Ryzen 5950X, VS2022, Windows: - utf8_to_utf16 3851ms -> 21ms - utf16_to_utf8 1055ms -> 4ms Signed-off-by: Aras Pranckevicius <[email protected]>
lgritz
pushed a commit
to lgritz/OpenImageIO
that referenced
this pull request
Dec 9, 2024
…ation#4549) OIIO 2.3.13 with PR AcademySoftwareFoundation#3307 changed MultiByteToWideChar/WideCharToMultiByte usage to C++11 <codecvt> functionality, but that has two issues: 1) it is *way* slower, primarily due to locale object access (on Visual C++ STL implementation in VS2022 at least). Since primary use case of these conversions is on Windows, maybe it is better to use a fast code path. 2) whole of <codecvt> machinery is deprecated with C++17 accross the board, and will be removed in C++26. I've kept the existing functions in there since otherwise it would have been an API break, but really maybe with OIIO 3.0 they should have been un-exposed. Too late now though :( ## Tests Performance numbers: doing ImageInput::create() on 1138 files where they are not images at all (so OIIO in turns tries all the input plugins on them). Ryzen 5950X, VS2022, Windows: - utf8_to_utf16 3851ms -> 21ms - utf16_to_utf8 1055ms -> 4ms Signed-off-by: Aras Pranckevicius <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Our Strutil utilities for utf8 <-> utf16 conversion were based on
Windows specific calls that we had added even before C++11 was our
minimum. Replace the Windows-specific code with std calls, and
expose them in strutil.h for all platforms (why not?).