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

Add Harfbuzz support #2859

Merged
merged 13 commits into from
Mar 1, 2023
Merged

Add Harfbuzz support #2859

merged 13 commits into from
Mar 1, 2023

Conversation

Ghabry
Copy link
Member

@Ghabry Ghabry commented Nov 14, 2022

WIP PR to test harfbuzz shaping support :)

First I refactored the Font and Text Api: Font is now only responsible for rendering single glyphs (char32_t). For any high level usage the Text Api must be used.

The exception to "single glyph" is the shaping Api which takes a u32string because this conditionally converts codepoints depending on the shaping rules. When Harfbuzz is enabled the Text Api will automatically handle this.

A special case is Window_Message. It renders glyph-by-glyph instead of all-in-one-go. This requires usage of the low level Font Api directly because the shaping information must be known.

@Ghabry Ghabry added the Fonts label Nov 14, 2022
@Ghabry Ghabry added this to the 0.7.1 milestone Nov 14, 2022
@Ghabry
Copy link
Member Author

Ghabry commented Nov 14, 2022

The Yume Nikki Online project is asking about Arabic text.

What impresses me here is that Harfbuzz can render right-to-left scripts correctly without changing a single line of code 🤔

a

@Ghabry
Copy link
Member Author

Ghabry commented Dec 18, 2022

Somebody asked about Times New Bastard. This is a font that makes every 7th character Sans Serif (Ligature magic).

When you count the glyphs you can see that it works. Though the font looks terrible. No good low res glyphs provided by the font:

screenshot_2

@Ghabry
Copy link
Member Author

Ghabry commented Jan 25, 2023

Linking libretro on macOS is broken since a while. Fits with Harfbuzz as its the issue:

diff --git a/builds/cmake/Modules/FindHarfbuzz.cmake b/builds/cmake/Modules/FindHarfbuzz.cmake
index 1f5a1fd0..654600e0 100644
--- a/builds/cmake/Modules/FindHarfbuzz.cmake
+++ b/builds/cmake/Modules/FindHarfbuzz.cmake
@@ -64,6 +64,30 @@ if(HARFBUZZ_FOUND)
			INTERFACE_INCLUDE_DIRECTORIES "${HARFBUZZ_INCLUDE_DIRS}"
			INTERFACE_LINK_LIBRARIES Freetype::Freetype
			IMPORTED_LOCATION "${HARFBUZZ_LIBRARY}")
+
+		if(APPLE)
+			# Framework list taken from Harfbuzz CMakeLists
+			if(IOS)
+				find_library(COREFOUNDATION CoreFoundation REQUIRED)
+				find_library(CORETEXT CoreText REQUIRED)
+				find_library(COREGRAPHICS CoreGraphics REQUIRED)
+
+				set_property(TARGET Harfbuzz::Harfbuzz APPEND PROPERTY
+					INTERFACE_LINK_LIBRARIES ${COREFOUNDATION} ${CORETEXT}
+						${COREGRAPHICS})
+
+				mark_as_advanced(COREFOUNDATION)
+				mark_as_advanced(CORETEXT)
+				mark_as_advanced(COREGRAPHICS)
+			else()
+				find_library(APPLICATION_SERVICES ApplicationServices REQUIRED)
+
+				set_property(TARGET Harfbuzz::Harfbuzz APPEND PROPERTY
+					INTERFACE_LINK_LIBRARIES ${APPLICATION_SERVICES})
+
+				mark_as_advanced(APPLICATION_SERVICES)
+			endif()
+		endif()
	endif()
 endif()
 

@Ghabry Ghabry marked this pull request as ready for review January 28, 2023 10:32
@Ghabry Ghabry changed the title WIP: Add Harfbuzz support Add Harfbuzz support Jan 28, 2023
@Ghabry
Copy link
Member Author

Ghabry commented Jan 28, 2023

Reran all the message tests of our TestGame for both with and without Freetype/Harfbuzz. Found one bug concerning the speed which is resolved now.

Ready to review. I consider this "kinda low risk" because except for "GetSize" the new code is only executed when a font is specified. The built-in font will not hit the new code.

src/text.h Outdated
* For continuous rendering use the "width" property.
*
* @param font the font used to render.
* @param ch the character to mesaure.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/ch/glyph/

@fdelapena
Copy link
Contributor

The Yume Nikki Online project is asking about Arabic text.

What impresses me here is that Harfbuzz can render right-to-left scripts correctly without changing a single line of code thinking

a

Works great, adding a note regarding a comment in chat regarding limited height for drawing glyphs: the shadow in the Arabic script above shows it's cropped (16 px height limit), and this could limit Thai scripting and "Zalgo" text, but I think it's good enough as is until some user complains ^^.

@fdelapena fdelapena requested a review from carstene1ns February 2, 2023 02:38
@fdelapena fdelapena added the Awaiting Rebase Pull requests with conflicting files due to former merge label Feb 24, 2023
@Ghabry Ghabry removed the Awaiting Rebase Pull requests with conflicting files due to former merge label Mar 1, 2023
Ghabry added 12 commits March 1, 2023 16:43
First make all Font handling go through Text namespace.
Text contains the high level API for interacting with the Font class.

This makes it possible to simplify the Font class by only handling char32_t.
Most string tests removed because this API is gone.
Remove some FIXMEs
… the font is rendered

The returned value is a ScopeGuard to ensure it is always reverted because fonts are shared.

Mostly useful for FreeType to allow reusing the same font for different sizes.
Fixes macOS libretro linker errors
@fdelapena fdelapena merged commit 0371008 into EasyRPG:master Mar 1, 2023
@Ghabry Ghabry deleted the harfbuzz branch June 12, 2023 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants