Skip to content

Commit

Permalink
Account for rounding errors when comparing float lengths
Browse files Browse the repository at this point in the history
Adapted from [1] and [2]. More details on the underlying issue can also
be found there.

We neither need to deal with very large, nor with very small numbers in
layout. So using an arbitrary epsilon should be perfectly fine
(`PHP_FLOAT_EPSILON` is only available starting from PHP 7.2); it is
selected so that it results in a precision of at least:

* 7 decimal digits at around 1
* 4 decimal digits at around 1000 (around the size of common paper formats)
* 2 decimal digits at around 100,000 (100,000pt ~ 35.28m)

This should be more than enough for any reasonable number used. If
anything, the epsilon value can easily be tuned later; just note that
the tests would need change, as many of them demonstrate the effect of
the (approximate) epsilon.

I started using the new comparison functions in critical places I
found, i.e. where a failure to detect equality could result in a
meaningful difference in rendering, particularly the checks for fitting
text and inline-block elements onto the current line, as the available
width there may be the result of a shrink-to-fit width calculation.

Open question: Maybe it would be more appropriate to treat all negative
values as unequal to zero, regardless of the difference, so that we
don't accidentally end up with (very small) negative width or height
values in some places.

[1] https://floating-point-gui.de/errors/comparison/
[2] https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/

Fixes #2560
  • Loading branch information
Mellthas committed Jan 24, 2022
1 parent 63d0586 commit 271c651
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 8 deletions.
5 changes: 3 additions & 2 deletions src/FrameDecorator/Page.php
Expand Up @@ -252,7 +252,8 @@ protected function hasGap(float $childPos, Frame $frame): bool
$style->padding_top
], $cbw);

return $childPos > $contentEdge && $contentEdge <= $this->bottom_page_edge;
return Helpers::lengthGreater($childPos, $contentEdge)
&& Helpers::lengthLessOrEqual($contentEdge, $this->bottom_page_edge);
}

/**
Expand Down Expand Up @@ -574,7 +575,7 @@ function check_page_break(Frame $frame)
}

// Check if $frame flows off the page
if ($max_y <= $this->bottom_page_edge) {
if (Helpers::lengthLessOrEqual($max_y, $this->bottom_page_edge)) {
// no: do nothing
return false;
}
Expand Down
7 changes: 4 additions & 3 deletions src/FrameReflower/Text.php
Expand Up @@ -158,7 +158,7 @@ protected function line_break(string $text, BlockFrameDecorator $block)
], $line_width);
$frame_width = $text_width + $mbp_width;

if ($frame_width <= $available_width) {
if (Helpers::lengthLessOrEqual($frame_width, $available_width)) {
return false;
}

Expand All @@ -181,8 +181,9 @@ protected function line_break(string $text, BlockFrameDecorator $block)
$sep = $words[$i + 1] ?? "";
$word = $sep === " " ? $words[$i] : $words[$i] . $sep;
$word_width = $fontMetrics->getTextWidth($word, $font, $size, $word_spacing, $char_spacing);
$used_width = $width + $word_width + $mbp_width;

if ($width + $word_width + $mbp_width > $available_width) {
if (Helpers::lengthGreater($used_width, $available_width)) {
// If the previous split happened by soft hyphen, we have to
// append its width again because the last hyphen of a line
// won't be removed
Expand Down Expand Up @@ -225,7 +226,7 @@ protected function line_break(string $text, BlockFrameDecorator $block)
$c = mb_substr($word, $j, 1);
$w = $fontMetrics->getTextWidth($s . $c, $font, $size, $word_spacing, $char_spacing);

if ($w > $available_width) {
if (Helpers::lengthGreater($w, $available_width)) {
break;
}

Expand Down
63 changes: 63 additions & 0 deletions src/Helpers.php
Expand Up @@ -962,4 +962,67 @@ public static function mb_ucwords(string $str): string

return $str;
}

/**
* Check whether two lengths should be considered equal, accounting for
* inaccuracies in float computation.
*
* The implementation relies on the fact that we are neither dealing with
* very large, nor with very small numbers in layout. Adapted from
* https://floating-point-gui.de/errors/comparison/.
*
* @param float $a
* @param float $b
*
* @return bool
*/
public static function lengthEqual(float $a, float $b): bool
{
// The epsilon results in a precision of at least:
// * 7 decimal digits at around 1
// * 4 decimal digits at around 1000 (around the size of common paper formats)
// * 2 decimal digits at around 100,000 (100,000pt ~ 35.28m)
static $epsilon = 1e-8;
static $almostZero = 1e-12;

$diff = abs($a - $b);

if ($a === $b || $diff < $almostZero) {
return true;
}

return $diff < $epsilon * max(abs($a), abs($b));
}

/**
* Check `$a < $b`, accounting for inaccuracies in float computation.
*/
public static function lengthLess(float $a, float $b): bool
{
return $a < $b && !self::lengthEqual($a, $b);
}

/**
* Check `$a <= $b`, accounting for inaccuracies in float computation.
*/
public static function lengthLessOrEqual(float $a, float $b): bool
{
return $a <= $b || self::lengthEqual($a, $b);
}

/**
* Check `$a > $b`, accounting for inaccuracies in float computation.
*/
public static function lengthGreater(float $a, float $b): bool
{
return $a > $b && !self::lengthEqual($a, $b);
}

/**
* Check `$a >= $b`, accounting for inaccuracies in float computation.
*/
public static function lengthGreaterOrEqual(float $a, float $b): bool
{
return $a >= $b || self::lengthEqual($a, $b);
}
}
4 changes: 3 additions & 1 deletion src/Positioner/Inline.php
Expand Up @@ -11,6 +11,7 @@
use Dompdf\FrameDecorator\AbstractFrameDecorator;
use Dompdf\FrameDecorator\Inline as InlineFrameDecorator;
use Dompdf\Exception;
use Dompdf\Helpers;

/**
* Positions inline frames
Expand Down Expand Up @@ -40,8 +41,9 @@ function position(AbstractFrameDecorator $frame)
// Atomic inline boxes and replaced inline elements
// (inline-block, inline-table, img etc.)
$width = $frame->get_margin_width();
$available_width = $cb["w"] - $line->left - $line->w - $line->right;

if ($width > ($cb["w"] - $line->left - $line->w - $line->right)) {
if (Helpers::lengthGreater($width, $available_width)) {
$block->add_line();
$line = $block->get_current_line_box();
}
Expand Down
63 changes: 61 additions & 2 deletions tests/HelpersTest.php
Expand Up @@ -6,7 +6,7 @@

class HelpersTest extends TestCase
{
public function testParseDataUriBase64Image()
public function testParseDataUriBase64Image(): void
{
$imageParts = [
'mime' => 'data:image/png;base64,',
Expand Down Expand Up @@ -37,9 +37,68 @@ public function dec2RomanProvider(): array
/**
* @dataProvider dec2RomanProvider
*/
public function testDec2Roman($number, $expected)
public function testDec2Roman($number, string $expected): void
{
$roman = Helpers::dec2roman($number);
$this->assertSame($expected, $roman);
}

public function lengthEqualProvider(): array
{
// Adapted from
// https://floating-point-gui.de/errors/NearlyEqualsTest.java
return [
[0.0, 0.3 - 0.2 - 0.1, true],
[0.3, 0.1 + 0.1 + 0.1, true],

// Large numbers
[100000000.0, 100000001.0, true],
[100000.0001, 100000.0002, true],
[100000.01, 100000.02, false],
[1000.0001, 1000.0002, false],

// Numbers around 1
[1.000000001, 1.000000002, true],
[1.0000001, 1.0000002, false],

// Numbers between 1 and 0
[0.00000010000001, 0.00000010000002, true],
[0.00000000001001, 0.00000000001002, true],
[0.000000100001, 0.000000100002, false],

// Close to zero
[0.0, 0.0, true],
[0.0, -0.0, true],
[1e-38, 1e-37, true],
[1e-38, -1e-37, true],
[1e-38, 0.0, true],
[1e-13, 1e-38, true],
[1e-13, 0.0, true],
[1e-13, -1e-13, true],
[1e-12, -1e-12, false],
[1e-12, 0.0, false],

// Very large numbers
[1e38, 1e38, true],
[1e38, 1.000001e38, false],

// Infinity and NaN
[INF, INF, true],
[INF, -INF, false],
[INF, 1e38, false],
[NAN, NAN, false],
[NAN, 0.0, false],
];
}

/**
* @dataProvider lengthEqualProvider
*/
public function testLengthEqual(float $a, float $b, bool $expected): void
{
$this->assertSame($expected, Helpers::lengthEqual($a, $b));
$this->assertSame($expected, Helpers::lengthEqual($b, $a));
$this->assertSame($expected, Helpers::lengthEqual(-$a, -$b));
$this->assertSame($expected, Helpers::lengthEqual(-$b, -$a));
}
}

0 comments on commit 271c651

Please sign in to comment.