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

Non-locale aware of scale and translate #100

Merged
merged 2 commits into from Mar 18, 2024
Merged

Conversation

JercSi
Copy link
Contributor

@JercSi JercSi commented Feb 19, 2022

When using a Slovenian locale, output of scale and translate is wrong.

Expected result: scale(3.123)
Actual result: scale(3,123) which means wrong scale
Issue: Slovenian decimal separator is ',' and not a '.' (https://www.localeplanet.com/icu/sl-SI/index.html)

Solution: when using a sprintf use a non-local aware float formatting.

Test code

$precision = 3;
$size = 3.1234;

$xml = new \XMLWriter();
$xml->openMemory();
$xml->startDocument('1.0', 'UTF-8');
$xml->startElement('svg');
$xml->writeAttribute('xmlns', 'http://www.w3.org/2000/svg');
$xml->writeAttribute('version', '1.1');
$xml->writeAttribute('width', (string) $size);
$xml->writeAttribute('height', (string) $size);
$xml->writeAttribute('viewBox', '0 0 '. $size . ' ' . $size);

// ========= Current code
// English: 3.1234
// Result is scale for x and y the same = 3.1234
setlocale(LC_ALL, array('English', 'en', 'en_GB.UTF-8'));
$xml->startElement('g');
$xml->writeAttribute(
    'transform',
    sprintf('scale(%s)', round($size, $precision))
);
$xml->text("English");
$xml->endElement();

// Slovenian: 3,1234
// Result is scale in x=3 and y=1234
setlocale(LC_ALL,array('sl_SI.UTF-8', 'Slovenian', 'sl_SI'));
$xml->startElement('g');
$xml->writeAttribute(
    'transform',
    sprintf('scale(%s)', round($size, $precision))
);
$xml->text("Slovenian with a bug");
$xml->endElement();

// ========= With fixed code
// Using a 'F': The argument is treated as a float and presented as a floating-point number (non-locale aware)
setlocale(LC_ALL, array('English', 'en', 'en_GB.UTF-8'));
$xml->startElement('g');
$xml->writeAttribute(
    'transform',
    sprintf('scale(%.' . $precision . 'F)', round($size, $precision))
);
$xml->text("English");
$xml->endElement();
setlocale(LC_ALL,array('sl_SI.UTF-8', 'Slovenian', 'sl_SI'));
$xml->startElement('g');
$xml->writeAttribute(
    'transform',
    sprintf('scale(%.' . $precision . 'F)', round($size, $precision))
);
$xml->text("Slovenian fixed");
$xml->endElement();

$xml->endDocument();
echo $xml->outputMemory();

Test code result

<?xml version="1.0" encoding="UTF-8"?>
<svg xmlns="http://www.w3.org/2000/svg" version="1.1" width="3.1234" height="3.1234" viewBox="0 0 3.1234 3.1234">
    <g transform="scale(3.123)">English</g>
    <g transform="scale(3,123)">Slovenian with a bug</g>
    <g transform="scale(3.123)">English</g>
    <g transform="scale(3.123)">Slovenian fixed</g>
</svg>

@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2022

Codecov Report

Merging #100 (84cc0d5) into master (0069435) will decrease coverage by 0.32%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #100      +/-   ##
============================================
- Coverage     63.43%   63.11%   -0.33%     
+ Complexity      934      928       -6     
============================================
  Files            47       47              
  Lines          3036     2879     -157     
============================================
- Hits           1926     1817     -109     
+ Misses         1110     1062      -48     
Impacted Files Coverage Δ
src/Renderer/Image/SvgImageBackEnd.php 0.00% <0.00%> (ø)
src/Renderer/Color/Gray.php 41.66% <0.00%> (-4.49%) ⬇️
src/Renderer/Image/ImagickImageBackEnd.php 62.34% <0.00%> (-2.19%) ⬇️
src/Renderer/Color/Rgb.php 43.33% <0.00%> (-1.83%) ⬇️
src/Writer.php 90.00% <0.00%> (-1.67%) ⬇️
src/Common/BitMatrix.php 35.77% <0.00%> (-1.62%) ⬇️
src/Common/ErrorCorrectionLevel.php 78.57% <0.00%> (-1.43%) ⬇️
src/Common/CharacterSetEci.php 51.42% <0.00%> (-1.35%) ⬇️
src/Renderer/RendererStyle/RendererStyle.php 66.66% <0.00%> (-1.34%) ⬇️
src/Encoder/QrCode.php 63.33% <0.00%> (-1.19%) ⬇️
... and 38 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0069435...84cc0d5. Read the comment docs.

@DASPRiD DASPRiD merged commit 13ea674 into Bacon:master Mar 18, 2024
DASPRiD pushed a commit that referenced this pull request Mar 19, 2024
* Non-locale aware of scale and translate
* Fix lint warning - too long line
bardiir pushed a commit to bardiir/BaconQrCode that referenced this pull request Mar 23, 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.

None yet

3 participants