Bug 690604 - rounding errors due to inconstant precision
Summary: rounding errors due to inconstant precision
Status: RESOLVED WONTFIX
Alias: None
Product: MuPDF
Classification: Unclassified
Component: fitz (show other bugs)
Version: unspecified
Hardware: All All
: P4 normal
Assignee: Tor Andersson
URL: http://code.google.com/p/sumatrapdf/i...
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-06 10:49 UTC by zeniko
Modified: 2010-07-13 21:05 UTC (History)
1 user (show)

See Also:
Customer:
Word Size: ---


Attachments
PDF from the URL (1.21 KB, application/pdf)
2009-07-06 10:50 UTC, zeniko
Details

Note You need to log in before you can comment on or make changes to this bug.
Description zeniko 2009-07-06 10:49:49 UTC
Consider:

float x = 842.0 * 1.7142857;
assert(fz_floor(842.0 * 1.7142857 - x) == 0);

This asserts, because the (first) product used for calculating the difference has 
higher precision than the precalculated product x. This issue exists throughout 
base_matrix.c and likely other files and can lead to graphical artifacts as seen 
e.g. in the PDF linked from the URL (white line at the top at several of the lower 
zoom levels).
Comment 1 zeniko 2009-07-06 10:50:25 UTC
Created attachment 5186 [details]
PDF from the URL
Comment 2 Tor Andersson 2010-01-17 06:12:38 UTC
I will change all instances of float to double after I have merged the no-tree branch.
Comment 3 vincent.torri 2010-06-19 07:17:57 UTC
The original poster is wronf on 2 points:

1) using == with floating point numbers (float or double) is bad. You should better compare them with FLT_MIN (for example).

2) Also, 842.0 is, by default, a double. If you want a float, use 842.0f. Try that:


float x = 842.0f * 1.7142857f;
assert(fz_floor(842.0f * 1.7142857f - x) == 0);

No problem, here.
Comment 4 vincent.torri 2010-06-19 07:41:59 UTC
also, in base_matrix (and other files maybe), if you use cosinus for trigonometric computations, use cosf() instead of cos(), the latter returning a double
Comment 5 Tor Andersson 2010-06-19 09:39:06 UTC
I'd love to use sinf and cosf, but they're extensions -- not standard C. Perhaps we can get around it with preprocessor macros.
Comment 6 vincent.torri 2010-06-19 10:04:36 UTC
it's C99 conformant. Do you intend to have a C89 conformant code ?
Comment 7 Tor Andersson 2010-06-19 15:06:53 UTC
Unfortunately MSVC is still not C99 conformant, and it doesn't look like it ever will. MSVC does support sinf and friends though, so I'm going ahead with this suggestion.
Comment 8 zeniko 2010-06-29 10:57:58 UTC
Just using floats everywhere doesn't solve this issue, as it occurs due to intermediate calculations happening at double precision.

E.g. for the linked document, the following change to base_matrix'c's fz_transformpoint fixes the rendering issue:

-	t.x = p.x * m.a + p.y * m.c + m.e;
-	t.y = p.x * m.b + p.y * m.d + m.f;
+	t.x = p.x * m.a; t.x += p.y * m.c; t.x += m.e;
+	t.y = p.x * m.b; t.y += p.y * m.d; t.y += m.f;

(note that all involved variables already are of type "float")

Even MuPDF's own viewer still shows this issue with attachment 5186 [details], at least when compiled with MSVC.
Comment 9 vincent.torri 2010-06-29 11:03:41 UTC
which version of vc++ ?
Comment 10 zeniko 2010-06-29 11:26:49 UTC
(In reply to comment #9)
MSVC 2008 (the last one able to target Windows 2000 without hack-arounds).
Comment 11 Tor Andersson 2010-06-29 11:48:33 UTC
Let me ask again what the exact nature of the problem is. MSVC (and perhaps other compilers) on intel stores intermediate values in full 80-bit precision in registers. If I understand things correctly this shouldn't be an issue -- we use floating point everywhere and don't rely on the results being accurate.

The document attached has a media box [0 0 595 842]. This gets multiplied by the zoom, then rounded up to an integer when we need to create a pixmap for rendering (so we don't clip graphics). Any page which paints a black background rectangle constrained to the [0 0 595 842] coordinates will get white borders. There's nothing you can do about that other than pick a zoom that doesn't result in fractional sizes.

To further investigate the issue with the attachment, this is the content stream:

0 0 595.4 842 re W* n
0 0 0 rg
0 0.1 595.3 841.9 re f*

So, it sets a clip rectangle slightly wider and the exact height of the page.
Then it draws a rectangle slightly wider and a little bit shorter than the page.
Even if we were to use perfect page sizes and zooms we'd see a white line!

There are two ways around this:

1) Draw the black background rectangle so it's bigger than the page -- this will make sure we fill the border pixels fully.

2) Start with a transparent background, fill it with a page-sized white rectangle before rendering the page. Due to anti-aliasing, we'll still get some minor artefacts though.

I don't see how changing the precision of calculations will help this issue in general, and certainly not the test file in specific. Maybe I have misunderstood something, if so please enlighten me before I close this as "won't fix" :)
Comment 12 zeniko 2010-06-29 12:47:07 UTC
The issue isn't that the rectangle doesn't fill the whole page - it's that the page boundaries are wrongly calculated:

Looking at the issue in a different angle, you could say that this is a rounding issue. The pixel-sized box to draw into is calculated as

bbox = fz_roundrect(fz_transformrect(ctm, app->page->mediabox));

Due to the mentioned imprecisions, bbox.y0 can result to be -1 instead of 0 (rounding down from a value very slightly below 0). So instead of the change from comment #8, you could also fix fz_roundrect to either mathematical rounding or at least rounding down/up with a certain tolerance.
Comment 13 Tor Andersson 2010-07-13 21:05:49 UTC
Any time the zoom is not an even multiple of 72 dpi the bounding boxes will be "wrong". Adding a fudge factor as in http://code.google.com/p/sumatrapdf/source/detail?r=1998 does not solve the problem -- you will still see white lines around the page at various zoom levels.

I traced the page bounding boxes for various zoom levels to illustrate what I mean:

60 dpi: [0 0 495.833 701.667]
70 dpi: [0 0 578.472 818.611]
72 dpi: [0 0 595 842]
80 dpi: [0 0 661.111 935.556]
144 dpi: [0 0 1190 1684]