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

Enable full Unicode range in SDL2 port #270

Merged

Conversation

juliusikkala
Copy link
Contributor

SDL_ttf 2.0.18 introduced 32-bit versions of the character rendering functions, which makes it simple to implement characters beyond the Basic Multilingual Plane. I tested this with #268, but it doesn't exactly depend on that PR (it's just that those characters happen to be incorrectly interpreted otherwise), which is why I submitted these separately.

2.0.18 was released on 2022-01-11, is that too recent? If that's an issue, it would also be possible to gate these with some #if SDL_TTF_VERSION_ATLEAST(2,0,18) or consider another, more complicated method.

Copy link
Collaborator

@GitMensch GitMensch left a comment

Choose a reason for hiding this comment

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

Thank you for this PR! Please adjust it using a define as suggested to use the old or new variant. I assume this would work out of the box also with the Makefile builds then.

Is there a reason that the explicit build is not updated to the current (August 2022) release? Otherwise please adjust this, ideally with changing the URL (because that libsdl switched from their own server to GitHub).

Thanks again!

@juliusikkala
Copy link
Contributor Author

Alright, I added the library version number guards now. The reason for not updating further was that newer SDL_ttf versions wanted newer FreeType versions, which caused some havoc as they had changed some of their CMake option names. I figured them out now and updated both FreeType and SDL_ttf, as well as SDL2. I also changed the SDL stuff to point to Github repos instead, as you requested.

@Bill-Gray
Copy link
Owner

This does look promising. Seems as if we should be able to make this work... thanks!

It looks as if something along the lines of

#if SDL_VERSION_ATLEAST(2, 0, 18)
   #define SUPPLEMENTAL_MP_SUPPORT   1
#endif

(and then use TTF_RenderGlyph32_XXX() or TTF_RenderGlyph_Solid() accordingly) would be the way to go. We should probably have some logic to say that SMP glyphs beyond 0xffff are rendered as, say, ? if we can't draw them, e.g.,

#ifndef SUPPLEMENTAL_MP_SUPPORT
   if( ch > 0xffff)
      ch = '?';
#endif

And we may need to add logic for fullwidth characters, of the sort currently implemented in the VT and WinGUI ports. But aside from minor issues of that ilk... this looks very good.

@juliusikkala
Copy link
Contributor Author

juliusikkala commented Jan 7, 2023

I've now put the TTF_RenderGlyph* calls into a separate function so that there's less #if spam, and did the improvements suggested by Bill, other than the fullwidth stuff. I'm not really particularly familiar with any Curses stuff (just started toying around with it a week ago for fun). What's the desired behavior for fullwidth, are they supposed to advance the cursor twice? Is it font-dependent or not?

If it's just a matter of advancing the cursor twice and a font-dependent solution is OK, it should be fairly simple to just check the width of the SDL_Surface returned from those TTF_RenderGlyph functions and round that to the next multiple of pdc_fwidth.

@Bill-Gray
Copy link
Owner

Looks good to me. Do you have any further changes in mind, or should I just merge this?

Don't worry about the fullwidth support. I think that will be more on my end, and will not be all that hard to do. In PDCursesMod, most of the logic for it applies already to all platforms. Fullwidth characters are stored with an undisplayed "dummy" character next to them; we display the fullwidth character (consuming two columns), show nothing for the "dummy" character, and we're back in sync : two "characters" were shown and we advanced by two columns. PDC_wcwidth() tells us which characters have which width.

Currently, only the VT and WinGUI platforms handle fullwidth correctly. I've hopes of eventually extending support to the framebuffer/DRM and DOSVGA platforms, and maybe X11, and (now) SDL2.

(Perhaps fullwidth should be more of a priority, since emoji and the languages of about a third of the world's people use them. But I don't use emoji and the languages I know all use halfwidth characters, so I've not been personally motivated to do anything beyond the above platforms.)

@juliusikkala
Copy link
Contributor Author

Great! In that case, I don't have any further changes in mind, so go ahead with the merge 👍

@Bill-Gray Bill-Gray merged commit 602bc38 into Bill-Gray:master Jan 7, 2023
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