Skip to content

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
@vlakoff
Copy link
Contributor

vlakoff commented Nov 26, 2025

As I mentioned in this comment, the changes introduced by this PR are no longer necessary since PHP 8.0. They should be reverted to avoid the redundant trailing zeroes.

edit: submitted #218 to address this.

vlakoff added a commit to vlakoff/BaconQrCode that referenced this pull request Nov 27, 2025
This reverts Bacon#100 which replaced %s with %.3F.

PHP ≥ 8.0 formatting is locale-independent, so %.3F is no longer necessary
and introduced trailing zeroes in scale/translate attributes.
vlakoff added a commit to vlakoff/BaconQrCode that referenced this pull request Nov 27, 2025
This reverts Bacon#100 which replaced %s with %.3F.

PHP ≥ 8.0 formatting is locale-independent, so %.3F is no longer necessary
and introduced trailing zeroes in scale/translate attributes.
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