Bug 690631 - fz_mul255 is off-by-one for negative input
Summary: fz_mul255 is off-by-one for negative input
Status: RESOLVED FIXED
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:
: 690751 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-07-14 09:21 UTC by zeniko
Modified: 2009-12-05 03:08 UTC (History)
1 user (show)

See Also:
Customer:
Word Size: ---


Attachments
demonstration of the problem (2.25 KB, application/pdf)
2009-07-16 14:51 UTC, Lev Bishop
Details

Note You need to log in before you can comment on or make changes to this bug.
Description zeniko 2009-07-14 09:21:58 UTC
E.g. fz_mul255(100, 100) yields 39 while fz_mul255(-100, 100) yields -40 instead 
of the expected -39. This causes visible artifacts such as mentioned at the URL.
Comment 1 Lev Bishop 2009-07-16 14:51:01 UTC
Created attachment 5215 [details]
demonstration of the problem

This file demonstrates the problem -- there are 256 black angled lines so the
background gets decremented by 1 for each line and there ends up being an
unwanted gradient.
Comment 2 Tor Andersson 2009-07-18 05:43:49 UTC
We shouldn't ever be calling fz_mul255 with negative numbers, if we do it's an oversight in our 
subtraction where those numbers should be "overflown" to 0..255 range.
Comment 3 Lev Bishop 2009-07-19 11:27:43 UTC
See {path,text}_w4i1o4() for places where fz_mul255() is called with negative
numbers. For example when painting a black line over a white page (causing the
artifacts in the test file).

Note that overflowing the negative numbers to 0..255 range is completely the
wrong thing to do here, and in fact those callsites do 
fz_mul255((short)r - dst[1], ca)
casting the unsigned chars to signed just to avoid this problem...

See my second suggested patch at the sumatrapdf bug in the URL field for one
possible solution: making fz_mul255 return a properly-rounded positive answer,
and modifying the callsites to only use fz_mul255 with positive values. This
seems solves the immediate problem (although I havent checked the performance
impact).

If you want something more performant, perhaps look at the freedesktop.org
pixman library, where they do something similar, but with optimizations, such as
packing all 4 colour components into an int, thus allowing the equivalent of
fz_mul255 on a full RGBA value in one go (pixman-combine.h). They also have MMX,
SSE, etc accelerated implementations of all the pdf blend modes, in case that is
needed.
Comment 4 Lev Bishop 2009-09-05 21:06:58 UTC
*** Bug 690751 has been marked as a duplicate of this bug. ***
Comment 5 zeniko 2009-12-04 10:05:06 UTC
This issue has not been fixed yet (in the public repository). Attachment 5215 [details] 
still misrenders while it renders correctly with the proposed fix from 
http://code.google.com/p/sumatrapdf/issues/detail?id=568#c1 .
Comment 6 zeniko 2009-12-05 03:08:13 UTC
My bad, looks like I somehow failed to recompile from scratch, even though I 
explicitly made sure I did when testing this fix.