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

Fixed CJK characters not rendering in new Label Engine [sc-24364] #14235

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

uberbrady
Copy link
Collaborator

This is a surprisingly hard problem to solve for some reason I don't fully understand. But because the new Label rendering engine actually generates PDF's based on fonts that are installed as part of the TCPDF library, not just rendering web pages.

So this solution - while terrible - is an approach that will, in the end, solve the problem. It presents two additional drop-down selects in the Label engine settings, defaulting them to what the current font choices are. There's one for your choice of fixed-width font, and one for your variable-width font. In there, if you pick 'cid0cs' as your font, they will render correctly, for Simplified Chinese. Similarly you can use 'cid0ct' for Traditional Chinese, 'cid0jp' for Japanese, and 'cid0kr' for Korean. (Lovely font names by the way; very intuitive).

There are a ton of other fonts available as well, so for now I've included them all. If we decide to go another way on this - possibly auto-detecting characters from different languages, or something else - we can change it - possibly just showing 'useful' subsets, or maybe doing something else entirely.

But this already is useful because you can quickly test out different font choices and see how the characters render when you do. Additionally, if you 'import' a new font, it should show up on the list (as it's dynamically generated). So if we find new fonts to play with, we can at least try them out.

I have not added the additional translation strings yet because we aren't even convinced we're going to go in this direction. If we do decide to go with this approach, I'll need to get all of those translation strings set up. (Interestingly, we can recommend different font choices for different languages, maybe? Or maybe that's the trick - the fonts you get are based on the language you're configured for? Not sure.)

Copy link

This pull request has been linked to Shortcut Story #24364: Asian (possibly more) characters don't display on labels.

Copy link

what-the-diff bot commented Feb 8, 2024

PR Summary

  • Enhanced Settings
    The addition of use TCPDF_FONTS and $fontlist in the getLabels method offers a wider range of font options in the Settings Controller.
  • Improved Label Functionality
    The integration of label2_mono_font and label2_variable_font in the postLabels method have expanded the font capabilities for labels.
  • Updated Label Abstract Class
    The inclusion of $variable_font and $mono_font properties improves the versatility of the Label class.
  • Adapted Write Method in DefaultLabel Class and Other Sheet Classes
    By adding $this->variable_font and $this->mono_font to the write method of the DefaultLabel class and the multiple sheet classes (L7162_A.php, L7162_B.php, L7163_A.php, _5267_A.php, _5520_A.php, TZe_12mm_A.php, TZe_24mm_A.php, LabelWriter_30252.php), variable and mono fonts can now be used more effectively.
  • Migration File Addition
    The new migration file, 2024_02_08_165209_add_fonts_to_settings.php, updates the database schema to include this expanded font selection.
  • Enhancement of Font Options Interface
    The changes in resources/views/settings/labels.blade.php deliver an improved user interface that includes font options for variable-width and fixed-width fonts, adding to the flexibility and ease of customization for end users.

app/View/Label.php Outdated Show resolved Hide resolved
@snipe
Copy link
Owner

snipe commented Feb 15, 2024

I hate this solution, but I hate literally everything else we've talked through more :(

@uberbrady uberbrady marked this pull request as ready for review February 19, 2024 14:01
@snipe
Copy link
Owner

snipe commented May 6, 2024

@uberbrady this has conflicts now - do we maybe want to re-target this to v7?

@uberbrady
Copy link
Collaborator Author

Although it's designed to be a non-breaking change - yes, I think retargeting it is probably a smart move. "But it's a non-breaking change!" is probably how most laments begin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants