Bug 55734 - REGRESSION (r72141): Acid3 rendering is not pixel perfect.
Summary: REGRESSION (r72141): Acid3 rendering is not pixel perfect.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL: http://acid3.acidtests.org/
Keywords: HasReduction, InRadar, Regression
Depends on:
Blocks:
 
Reported: 2011-03-03 17:38 PST by Andy Estes
Modified: 2011-04-14 13:34 PDT (History)
3 users (show)

See Also:

Attachments
WebKit rendering of Acid3 (deleted)
2011-03-03 17:38 PST, Andy Estes 		no flags Details
Reference rendering (deleted)
2011-03-03 17:38 PST, Andy Estes 		no flags Details
WebKit rendering of Acid3 (43.29 KB, image/png)
2011-03-03 18:06 PST, Andy Estes 		no flags Details
Reference rendering (43.15 KB, image/png)
2011-03-03 18:07 PST, Andy Estes 		no flags Details
Reduction (650 bytes, text/html)
2011-04-11 14:15 PDT, Andy Estes 		no flags Details
Patch (20.53 KB, patch)
2011-04-14 03:33 PDT, Andy Estes 		mitz: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2011-03-03 17:38:21 PST 
Created attachment 84665 [details]
WebKit rendering of Acid3

Our Acid3 rendering no longer matches the reference image pixel-for-pixel after <http://trac.webkit.org/changeset/72141>. It appears that the "100/100" text is a pixel higher in our rendering than it is in the reference image. See the attached screenshots.
Comment 1 Andy Estes 2011-03-03 17:38:39 PST 
Created attachment 84666 [details]
Reference rendering
Comment 2 Andy Estes 2011-03-03 17:40:02 PST 
<rdar://problem/9084992>
Comment 3 William Siegrist 2011-03-03 18:02:03 PST 
The content of attachment 84665 [details] has been deleted by
    William Siegrist <wsiegrist@apple.com>
who provided the following reason:

Deleted per request from Andy Estes

The token used to delete this attachment was generated at 2011-03-03 18:01:27 PST.
Comment 4 William Siegrist 2011-03-03 18:02:16 PST 
The content of attachment 84666 [details] has been deleted by
    William Siegrist <wsiegrist@apple.com>
who provided the following reason:

Deleted per request from Andy Estes

The token used to delete this attachment was generated at 2011-03-03 18:02:12 PST.
Comment 5 Andy Estes 2011-03-03 18:06:48 PST 
Created attachment 84670 [details]
WebKit rendering of Acid3
Comment 6 Andy Estes 2011-03-03 18:07:32 PST 
Created attachment 84671 [details]
Reference rendering
Comment 7 Andy Estes 2011-04-11 14:15:12 PDT 
Created attachment 89085 [details]
Reduction
Comment 8 Andy Estes 2011-04-11 20:54:15 PDT 
I took a look at it for an hour or so. It seems like this is specific to the combination of having 0 font-size and 0 line-spacing, and due to this I think we compute a bogus descent when looking at the root linebox (1px ascent, -1px descent). It doesn't seem right that a 0px font with 0 leading should have an ascent.
Comment 9 Andy Estes 2011-04-14 03:33:41 PDT 
Created attachment 89552 [details]
Patch
Comment 10 mitz 2011-04-14 11:03:06 PDT 
Comment on attachment 89552 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=89552&action=review

> Source/WebCore/css/CSSStyleSelector.cpp:6808
> +    if (fabsf(specifiedSize - 0.0f) < std::numeric_limits<float>::epsilon())

Looks good, but why is it necessary to subtract 0?
Comment 11 Andy Estes 2011-04-14 12:00:30 PDT 
(In reply to comment #10)
> (From update of attachment 89552 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=89552&action=review
> 
> > Source/WebCore/css/CSSStyleSelector.cpp:6808
> > +    if (fabsf(specifiedSize - 0.0f) < std::numeric_limits<float>::epsilon())
> 
> Looks good, but why is it necessary to subtract 0?

Ah, it isn't! Good call. I can take that out before landing.
Comment 12 Andy Estes 2011-04-14 13:34:39 PDT 
Committed r83889: <http://trac.webkit.org/changeset/83889>