star-micronics / star-cloudprnt-for-woocommerce

Print to Star CloudPRNT compatible printers from WooCommerce
MIT License
9 stars 8 forks source link

About using star_cloudprnt_get_column_separated_data in a multibyte language #8

Open ko31 opened 3 years ago

ko31 commented 3 years ago

The print layout seems to go wrong when I use star_cloudprnt_get_column_separated_data in Japanese.

As you know, I guess that the cause is in the following code. https://github.com/star-micronics/star-cloudprnt-for-woocommerce/blob/master/order-handler.inc.php#L53-L54

The following are results I tried in my development environment. In the current code, it will wrap as shown on the left. On the other hand, if line 53 is enabled, it will not wrap as shown on the right.

sample

If possible, how about adding a selection to the settings screen so that users can choose?

Lozrus commented 3 years ago

@ko31 yes, this has been bothering me too for a while, it's very cool that PHP has an mb_strwidth() function that is aware of both full/half width characters, but unfortunately it seems to give consistently wrong results, at least for our printers. Really, I'd like to find the underlying cause and address that (e.g., maybe one common character, like the space, is being treated as full width by php and half-width by our printer. This also affects the word-wrapping support which is current based on the PHP wordwrap function, that is totally not mb aware.

I'm happy to include an option, if I can't find a better solution, but I'd like to try to to make it just work - I worry about options that aren't easy for the end user to understand.

Also, you may be interested adding your own rendering filters for the problem parts, which is possible from version 2.1, without you needing to modify our plugin directly.

I will remove the star_cloudprnt_get_column_separated_data() function in 2.1 anyway, this facility is being moved into the document building interface for version 2.1 (to make it more easily accessible to rendering filters), so it is a good time to see if it can be improved.

ko31 commented 3 years ago

@Lozrus Thank you for sharing that information. I think your idea is great. I'm going to try version 2.1 first anyway.

Lozrus commented 3 years ago

@ko31 Great, I'm hoping that 2.1 will allow us to move away from customers needing to modify the plugin code directly, and push that out to an external plugin, so doesn't get broken every time we make a minor release. So I'd be very interested in your feedback. I've enabled discussions for this repository which might be a good place for that.

Lozrus commented 3 years ago

@ko31 I I did some testing and figured out the cause. It was unfair of me to blame the PHP mb_strwidth() function, because that is actually returning pretty good results - it will not be perfect, not all character agree on which cars are full or half width, but it should be good enough 99.9% of the time.

The actual cause, is that our printers assign slightly different default spacing to half width, and full width characters, which means that a full width character is not exactly double a half width character, which is what out plugin code is assuming.

If you add something like:

$printer->add_hex("1B730000");

Before printing any two column sections, then it will eliminate the extra character spacing for full width characters, and your columns should align correctly.

My only concern is then for the readability of any Japanese text, for Hiragana/Katakana I think it looks ok, but I do not know enough Kanji to be able to judge that correctly.

Anyway, for version 2.1 I will need to make sure that the character spacing is correctly accounted for when doing column layout.

ko31 commented 3 years ago

@Lozrus I tried adding the following code In version 2.1.

$printer->add_hex("1B730000");

As a result, every two-column sections were printed cleanly.

IMG_5530

I've only tried it once, but just to let you know.

Lozrus commented 3 years ago

Thanks for confirming, that looks much better 😄 I committed the same fix to the 2.1 branch yesterday. I think it's not the final solution, but it's a good first step. How do you feel about the readability of the Japanese text after this change? I worry that eliminating the character spacing makes it look a little too squashed?

ko31 commented 3 years ago

@Lozrus Thank you for the update. I think this change is hardly noticeable and is no problem 👍