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

Make EPS & SVG Rendering usable on international locales #84

Closed
wants to merge 1 commit into from

Conversation

bardiir
Copy link

@bardiir bardiir commented May 24, 2021

Funnily enough - This does not fix the Imagick rendering as even the base PHP function of ImagickDraw::scale(float, float) does fail on international locale with floats. But it does take care of localized numeric transformations giving broken SVGs when using the library, it's at least usable with the svg and eps backends without giving spontaneous errors.

I also opened an issue there: Imagick/imagick#404

The error you're seeing when using Imagick Backend on an international locale is going to be 'non-conforming drawing primitive definition `'

I'm probably still going to use an en_US locale now and do numeric transformations manually instead of via PHP instead. But it's anyway a good idea to support other locales, so this is at least an improvement over the original.

@DASPRiD
Copy link
Member

DASPRiD commented May 24, 2021

Generally looks fine, though two things about this:

  • %F is not omitting redundant trailing zeros
  • The lines seems to became pretty long - they should not exceed 120 character.
  • Bonus: Spaces around the . operator :)

As %F does not omit trailing zeros, an alternative could be to have a helper function in the respective renderers which replaces the call to round() and instead use its own implementation which outputs non-locale aware numbers as strings.

Considering the last PR, it should actually be fine to combine number_format() with round() in its own function.

@Francismori7
Copy link

Please merge this. On a French production server, scale gets rendered as 2,572 instead of 2.572 which in turn causes a very big scaling problem.

We're currently resorting to a STR_REPLACE around the generate function and is not really ideal.

Copy link
Member

@DASPRiD DASPRiD left a comment

Choose a reason for hiding this comment

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

See previous comment.

@SerkanBe
Copy link

Ran into the same issue, setting the locale settings now around the QR-generation. Not ideal, but does the job. Without any str_replace or changes to the classes.

$locale = setlocale(LC_ALL, 0);
setlocale(LC_ALL, 'C');
try {
    $renderer = new ImageRenderer(
        new RendererStyle(132, 2),
        new SvgImageBackEnd()
    );

    $writer = new Writer($renderer);
    return $writer->writeString($data);
} finally {
    setlocale(LC_ALL, $locale);
}

Would it make sense to have this done in writeString()? I might be missing something... but it seems to work for me just fine.

@DASPRiD
Copy link
Member

DASPRiD commented Mar 18, 2024

This has been addressed by #100.

@DASPRiD DASPRiD closed this Mar 18, 2024
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.

4 participants