Bug 55734

Summary: REGRESSION (r72141): Acid3 rendering is not pixel perfect.
Product: WebKit Reporter: Andy Estes <aestes>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, hyatt, mitz
Priority: P2 Keywords: HasReduction, InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
URL: http://acid3.acidtests.org/
Attachments:
Description Flags
WebKit rendering of Acid3
none
Reference rendering
none
WebKit rendering of Acid3
none
Reference rendering
none
Reduction
none
Patch mitz: review+

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>