Bug 109535

Summary: [Cairo] Anti-aliasing should not be always disabled for lines
Product: WebKit Reporter: Laszlo Gombos <laszlo.gombos>
Component: WebCore Misc.Assignee: Viatcheslav Ostapenko <ostap73>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, d-r, gyuyoung.kim, igor.oliveira, laszlo.gombos, mrobinson, rakuco, treatassignmenthelp, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Unscaled page sample with underlines.
none
Scaled page sample.
none
Patch
none
Patch for test results
none
Patch for test results
none
Failing test to get pixel test results from EWS. Not for review!!!
none
Patch for landing none

Description Laszlo Gombos 2013-02-11 18:47:35 PST
GraphicsContextCairo.cpp::drawLineOnCairoContext() sets anti-aliasing to CAIRO_ANTIALIAS_NONE with no condition. This is not desired and no other ports are handling anti-aliasing the same way. Antialiasing is desired e.g. when the page is scaled.
Comment 1 Viatcheslav Ostapenko 2013-03-22 08:00:29 PDT
Created attachment 194547 [details]
Unscaled page sample with underlines.
Comment 2 Viatcheslav Ostapenko 2013-03-22 08:11:44 PDT
Created attachment 194550 [details]
Scaled page sample.

Notice, that "Global Attributes in HTML" underline is too bold and "Event Attribute in HTML" underline disappeared.
Comment 3 Viatcheslav Ostapenko 2013-03-22 23:49:08 PDT
Created attachment 194693 [details]
Patch
Comment 4 Martin Robinson 2013-03-23 01:22:50 PDT
Comment on attachment 194693 [details]
Patch

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

> LayoutTests/ChangeLog:10
> +        * fast/css3-text/css3-text-decoration/text-decoration-line-scaled-expected.txt: Added.

What platform is this result for? I imagine that few platforms will be able to share this result.
Comment 5 Viatcheslav Ostapenko 2013-03-23 07:15:29 PDT
(In reply to comment #4)
> (From update of attachment 194693 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=194693&action=review
> 
> > LayoutTests/ChangeLog:10
> > +        * fast/css3-text/css3-text-decoration/text-decoration-line-scaled-expected.txt: Added.
> 
> What platform is this result for? I imagine that few platforms will be able to share this result.

IMHO, GTK should be able to share. And other cairo platforms. Chromium has somewhat strange code there and still has a problem similar to what EFL has without patch (like on scaled page sample).
Comment 6 Martin Robinson 2013-03-23 10:18:54 PDT
(In reply to comment #5)
 
> > What platform is this result for? I imagine that few platforms will be able to share this result.
> 
> IMHO, GTK should be able to share. And other cairo platforms. Chromium has somewhat strange code there and still has a problem similar to what EFL has 
without patch (like on scaled page sample).

Sorry my comment was pretty unclear. The result in your patch is placed in the platform-independent directory, but it's almost certain that Mac, Qt, Chromium and all other ports will required separate baselines.
Comment 7 Viatcheslav Ostapenko 2013-03-23 10:55:36 PDT
(In reply to comment #6)
> (In reply to comment #5)
> 
> > > What platform is this result for? I imagine that few platforms will be able to share this result.
> > 
> > IMHO, GTK should be able to share. And other cairo platforms. Chromium has somewhat strange code there and still has a problem similar to what EFL has 
> without patch (like on scaled page sample).
> 
> Sorry my comment was pretty unclear. The result in your patch is placed in the platform-independent directory, but it's almost certain that Mac, Qt, Chromium and all other ports will required separate baselines.

Test results that are in common directory are generated for chromium build. Should I move it to chromium platform results?
Comment 8 Martin Robinson 2013-03-31 19:49:45 PDT
(In reply to comment #7)

> Test results that are in common directory are generated for chromium build. Should I move it to chromium platform results?

Sorry for the late reply. I think these need to be moved to Chromium specific directories. I'm not sure if they need to be specific to Chromium linux or all Chromium ports.
Comment 9 Viatcheslav Ostapenko 2013-05-03 11:36:12 PDT
Created attachment 200444 [details]
Patch for test results
Comment 10 Viatcheslav Ostapenko 2013-05-03 11:43:22 PDT
Created attachment 200447 [details]
Patch for test results
Comment 11 Viatcheslav Ostapenko 2013-05-03 11:59:43 PDT
Created attachment 200451 [details]
Failing test to get pixel test results from EWS. Not for review!!!
Comment 12 Viatcheslav Ostapenko 2013-05-03 14:43:16 PDT
Created attachment 200469 [details]
Patch for landing
Comment 13 WebKit Commit Bot 2013-05-03 15:12:04 PDT
Comment on attachment 200469 [details]
Patch for landing

Clearing flags on attachment: 200469

Committed r149540: <http://trac.webkit.org/changeset/149540>
Comment 14 WebKit Commit Bot 2013-05-03 15:12:08 PDT
All reviewed patches have been landed.  Closing bug.