Bug 114999

Summary: Disable Subpixel layout on mac
Product: WebKit Reporter: Roger Fong <roger_fong>
Component: WebKit Misc.Assignee: Roger Fong <roger_fong>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, kbalazs, simon.fraser, thorton, zalan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
patch
none
test rebaselines
none
patch
none
Patch simon.fraser: review+

Description Roger Fong 2013-04-22 16:30:06 PDT
<rdar://problem/13667607>
Comment 1 Roger Fong 2013-04-23 17:05:03 PDT
Created attachment 199335 [details]
patch
Comment 2 zalan 2013-04-24 03:53:24 PDT
+@@ -256,7 +256,7 @@
+ #endif
+ 
+ #if !defined(ENABLE_SUBPIXEL_LAYOUT)
+-#define ENABLE_SUBPIXEL_LAYOUT 1
++#define ENABLE_SUBPIXEL_LAYOUT 0
+ #endif
+ 
+ #endif /* PLATFORM(EFL) */

This is EFL. I don't think it is applicable.
Comment 3 Roger Fong 2013-04-24 11:36:29 PDT
Oops, yeah, wrong place. Meant to do the one near the end of the file.
Comment 4 zalan 2013-04-24 11:39:50 PDT
(In reply to comment #3)
> Oops, yeah, wrong place. Meant to do the one near the end of the file.
I dont think that needs to be changed either.
Comment 5 Roger Fong 2013-04-25 19:02:01 PDT
Created attachment 199771 [details]
patch

Got rid of the unnecessary define.
This is just to disable sub pixel layout. 
The test rebaselines will come in a separate patch.
Comment 6 Roger Fong 2013-04-25 19:03:24 PDT
Created attachment 199772 [details]
test rebaselines

test reaselines.
Note there’s there’s still a couple dozen failures that I plan on skipping and dealing with separately after this lands.
Comment 7 zalan 2013-04-25 22:45:44 PDT
Comment on attachment 199771 [details]
patch

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

> WebCore/platform/graphics/LayoutRect.h:224
>  #endif

Any particular reason why you switched from 'explicit IntRect(const LayoutRect&)' to 'IntRect(int x, int y, int width, int height)'?
Comment 8 Allan Sandfeld Jensen 2013-04-26 09:14:04 PDT
Since Qt and GTK enabled sub-pixel layout last week, it would be interesting if you could share some of the reasoning for Mac disabling it again.
Comment 9 zalan 2013-04-26 09:51:45 PDT
(In reply to comment #8)
> Since Qt and GTK enabled sub-pixel layout last week, it would be interesting if you could share some of the reasoning for Mac disabling it again.

Various off-by-one pixel issues as we just discussed it at #qtwebkit.
Comment 10 Roger Fong 2013-04-26 11:06:10 PDT
(In reply to comment #7)
> (From update of attachment 199771 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=199771&action=review
> 
> > WebCore/platform/graphics/LayoutRect.h:224
> >  #endif
> 
> Any particular reason why you switched from 'explicit IntRect(const LayoutRect&)' to 'IntRect(int x, int y, int width, int height)'?

Build fix. Not quite sure why the old one stopped working. I'll look into it.
Comment 11 Roger Fong 2013-04-26 13:23:46 PDT
Created attachment 199855 [details]
patch

Change build fix to add an export
Comment 12 Roger Fong 2013-04-26 14:00:00 PDT
Created attachment 199857 [details]
Patch
Comment 13 Simon Fraser (smfr) 2013-04-26 14:11:50 PDT
Comment on attachment 199857 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Reviewed by NOBODY (OOPS!).
> +
> +        * Configurations/FeatureDefines.xcconfig:
> +        * WebCore.exp.in:

This should say why you changed WebCore.exp.in
Comment 14 Roger Fong 2013-04-26 15:20:25 PDT
committed: http://trac.webkit.org/changeset/149209
Comment 15 zalan 2013-04-26 15:42:59 PDT
yay!
Comment 16 Balazs Kelemen 2013-05-09 06:02:26 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > Since Qt and GTK enabled sub-pixel layout last week, it would be interesting if you could share some of the reasoning for Mac disabling it again.
> 
> Various off-by-one pixel issues as we just discussed it at #qtwebkit.

This is a great source of platform bugs/misbehavior. I absolutely incompetent in the topic but it seems to me that diverging across ports in such a fundamental behavior is unhealthy. I think all platforms should make a consensus on this otherwise we are developing different engines in the same project which does not make sense.