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

Check and report errors for Win32 functions that may fail #1255

Closed
wants to merge 1 commit into from

Conversation

AnyOldName3
Copy link
Contributor

Skips a couple in the Win32_Window destructor. One will throw because of #1254, so this should be temporary. The other's because there's a relatively benign failure as the API won't let you unregister a window class while there are still windows using it, which is what we want.

Skips a couple in the Win32_Window destructor. One will throw because of a bug that's on the tracker, so this should be temporary. The other's because there's a relatively benign failure as the API won't let you unregister a window class while there are still windows using it, which is what we want.
@robertosfield
Copy link
Collaborator

Thanks for the changes. I feel there is some more evolution of the code that is needed before merging with VSG master so I've merged as a branch: https://github.com/vsg-dev/VulkanSceneGraph/tree/AnyOldName3-check-win32-errors

One of the changes I'm thinking about is changing the Win32_Window::resize() method so it only updates the _extend2D and calls buildSwapchain() when GetClientRect(..) return true.

Another is moving the GetClientRect(..) usage in to within the Win32_Window::handleWin32Messages(..) into the blocks that actually need it rather than for all event messages.

Your comment about vsg::Exception not supporting wide strings. One thing I've been wondering about is looking at changing vsg::Exception so it subclasses from one of the std exception classes and just adding to them. I don't know if that would aid with the whole widestring situation under Windows as I haven't looked into this topic in detail.

@AnyOldName3
Copy link
Contributor Author

Deriving from std::exception would generally be a good idea (lots of applications have a global std::exception catch block that displays a fatal error message etc. before exiting), but it wouldn't help the Windows Unicode situation as std::exception::what returns a C string. There are still LTS versions of Windows under support until 2029 that don't support UTF-8 as the narrow string encoding, so the put the application into UTF-8 mode if you care about Unicode and can't be bothered with wide strings solution isn't viable for enterprise-friendly software yet. There is the option to internally use UTF-8 everywhere and convert to UTF-16 every time a function talks to the Win32 API, but in modern C++, that requires some dodgy-looking reinterpret_casts as char and char8_t are distinct types and there are a few places where STL functions assume char uses the OS' eight-bit encoding and can't be convinced otherwise.

It's particularly annoying as you can't even blame Microsoft for the mess. They followed the Unicode Consortium's advice before UTF-8 had been invented and before they'd noticed there were more than 65536 unique letters in all human languages, so were all-in on 16-bit wide strings before Unix systems supported Unicode at all.

@robertosfield
Copy link
Collaborator

The vsg::Path related functionality has functions for converting wstring to UTF8, could they be used in this context?

@AnyOldName3
Copy link
Contributor Author

Temporarily ignoring the fact that that conversion is broken (see #1133), the conversion itself isn't the tricky part, it's ensuring it happens in the right places and gets communicated to the people who need to know. It does look like lots of other bits of the VSG are working under the assumption that char-based strings are UTF-8, in which case it would make sense to always do UTF-16 conversions and always call the W-suffixed versions of Win32 functions instead of calling the A-suffixed ones or letting the UNICODE define pick. Otherwise, the Win32 API will most likely expect and return strings in a fixed-width eight-bit encoding like CP1252, which is only compatible with a small subset of UTF-8.

@robertosfield
Copy link
Collaborator

I'm closing this PR as I believe all the key functionality is merged in the https://github.com/vsg-dev/VulkanSceneGraph/tree/Win32_Window_Refinements branch.

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.

2 participants