Hi all, I have recently updated my allegro from git to the current master branch. After that, I noticed that two things don't work (for me) as they did before:

1) copy to clipboard from my app only shows "I" after paste into an editor.

2) the file selection dialog, when called with a file filter, shows no files.

I looked back into the history and found that the last changes to win_dialog.c (bac7bddc9d79cf6d520f27335e01f78ddbb99d34) and wclip board.c (b2e6f42d9525b16553913a041063118d803ab3f5) broke it, so I copied those files back from the version before and things works fine again (for me).

Not sure what it is. I'm using MSVC 2017, platform toolset is "Visual Studio 2017 (v141)" and WIndows SDK is "10.0.17763.0". So I'm not aware that this is so much out of date, and I see that I'm also using MBCS instead of Unicode, but I would expect that the allegro code adapts to that setting in a more graceful way. Or would I need to switch to Unicode here? Hm.

I guess that this question goes mostly to Peter Hull, he did the changes some months ago.

Interesting! Can you clarify what you are doing? Are you passing non-UTF8 strings to Allegro? I think the idea of those changes was that they were mostly internal, Allegro always expects UTF8 coming in, and internally converts it to a 16 bit encoding that Windows wants. It shouldn't matter what the host program is compiled under.

Well, it's easy enough, here's some code that runs at the beginning of main:

So what worked before is that the box showed the matching files, and the button is labelled "Select xyz file". With the changes the button is correctly labelled, but no files seem to match, so matching files are shown.

However, when I type "*.xyz" into the selection field, then the matching files appear...

Alright it is I think because the filter has embedded NULLs in it and my al_ustr->utf16 conversion didn't account for that.
Filter is assembled in create_filter_string (win_dialog.c:128) and conversion is via _twin_ustr_to_tchar which you will see assumes a C-style null termination.

Might be good for the ex_native_filechooser to make use of the filter which it doesn't currently.

Haven't looked at the clipboard thing yet.

[edit] Please can someone file a bug or bugs? Thanks.

Good call, Peter! I could reproduce tobings issue with the file dialog, and made a fix based on your observation: https://github.com/liballeg/allegro5/pull/1114.

tobing, would you mind testing it? I also looked at the clipboard stuff, but it didn't appear to have any obvious issues. Can you explain a bit more what you do there? Are you passing a unicode string to al_set_clipboard_text?

Great. I had written my own fix which was almost word-for-word identical with your PR . But, does it need fixing for the non-UNICODE build too?

[edit]
On the clipboard, we are always converting ALLEGRO_USTR to UTF-16, but we are setting it onto the clipboard using either CF_UNICODETEXT or CF_TEXT depending on the value of UNICODE. See https://github.com/liballeg/allegro5/blob/36c1549bae816c35a79d9a842beb777bde4b4a28/src/win/wclipboard.c#L91

I think regardless of the value UNICODE used when compiling Allegro, we should be using CF_UNICODETEXT as I don't think there are supported non-unicode-aware versions of Windows any more. (Windows ME?)
What do you think?
Also there is probably something equally wrong on the 'paste' side.

I thought we decided to get rid of the ANSI code path? We could still add it in the meantime, I suppose.

Interesting. That does look wrong for the non-UNICODE case, but the UNICODE path seems fine, no? The only way to compile Allegro with ANSI is to edit the CMakeLists.txt, and that seems unlikely to have happened in this case.

Tobing did say he was using MBCS, in the first post. But I agree, there should be no reason not to use Unicode these days.

Actually, I mis-remembered - I thought it was easy to build a non-Unicode version but, as you say, it does require editing the CMake files. How are you building Allegro, tobing?

Sorry, I was a bit captured by something else for some days. Thanks for looking into it.

I'm using more or less hand-made visual studio 2017 projects, and the settings are the same for allegro and all the other libraries I'm building this way. Almost all is statically linked, and I also have zip png jpg ogg lua freetype physfs alguichan in the boat. So it won't be easy to share the exact configuration, but I think I could extract the typical parameter lines for compiler and linker, if needed.

I'll try to figure out how to check out the fix you provided, so I can test it.

For the copy&paste, most of the time I'm pasting into an empty Notepad++ tab, and sometimes into outlook mail directly, without difference in behavior - either both work fine, or they don't. For the question about how I copy the text into the clipboard, I'll have to look up the code...

Edit: checked out your change, and tested. When I finally compiled allegro, aladdons and the main program with UNICODE, the fix worked. So that's nice, but to be honest, why doesn't it also work when I don't use UNICODE? Seems that the code uses _al_win_utf8_to_ansi instead of _al_win_utf8_to_utf16 which has been replaced by your change, so maybe the MBCS-part also needs some fix?

The reason is simply that SL's patch didn't make any changes to the non-Unicode code path.

I believe we are on the verge of making the Unicode version the only version - all supported Windows use Unicode internally and all Allegro's string handling works via UTF-8. However it would not be too difficult to maintain if you've got a good reason for not using Unicode?

Thanks,
Pete

I see, it's a custom built Allegro, things are starting to make sense! I'd still love to hear more about what's done to hit the clipboard bug.

We could certainly keep the ANSI versions around, in principle, thanks to Peter's hard work it's essentially free. It's easy to fix the file dialog for ANSI as well, I just didn't do it since I didn't know you were compiling Allegro in ANSI mode yourself.

EDIT: I added the implementation of the fix for ANSI as well.

The extended fix solves the issues with the file selection box, both UNICODE and MBCS.

About the question why I used MBCS, there's no reason in particular, just used to have that setting all the time and didn't need to change it. But switching to UNICODE for everything I do (with allegro) is certainly possible, and I think I'll do that for future builds. Because when I'm using strings and text (also from files), I'm thinking UTF8 actually.

The clipboard thing is a different story. Works with UNICODE, but does not work with MBCS. I'll check the details and come back with more information, especially about how I feed my string into the clipboard...

Thanks a lot for the file selection box fix!!!

Edit:

I#m using al_set_clipboard_text with a char* grom a std::string. Some debugging shows that the string is converted using _al_win_utf8_to_utf16 and a few lines later, memmove is used to copy/move the bytes to the dst buffer. There's no obvious difference between UNICODE and MBCS, except that after the memmove, in case of UNICODE, the dst string looks correct, while with MBCS the dst string only shows the first character, even though all bytes have been moved...

That brought me onto the right track, so here's the changed method that works for both UNICODE and MBCS:

Great, makes sense! But might need a bit more thought about edge-cases etc.

Also the API/docs are a bit vague, al_set_clipboard_text takes const char* which has to be UTF-8 but the docs don't say that - if you were writing ansi code with a specific code-page and using characters > 127 it would fail.

Thanks for the fixes!