Bug 109535 - [Cairo] Anti-aliasing should not be always disabled for lines
Summary: [Cairo] Anti-aliasing should not be always disabled for lines
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Viatcheslav Ostapenko
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-11 18:47 PST by Laszlo Gombos
Modified: 2020-09-05 09:40 PDT (History)
9 users (show)

See Also:


Attachments
Unscaled page sample with underlines. (359.85 KB, image/png)
2013-03-22 08:00 PDT, Viatcheslav Ostapenko
no flags Details
Scaled page sample. (196.95 KB, image/png)
2013-03-22 08:11 PDT, Viatcheslav Ostapenko
no flags Details
Patch (73.54 KB, patch)
2013-03-22 23:49 PDT, Viatcheslav Ostapenko
no flags Details | Formatted Diff | Diff
Patch for test results (17.99 KB, patch)
2013-05-03 11:36 PDT, Viatcheslav Ostapenko
no flags Details | Formatted Diff | Diff
Patch for test results (73.13 KB, patch)
2013-05-03 11:43 PDT, Viatcheslav Ostapenko
no flags Details | Formatted Diff | Diff
Failing test to get pixel test results from EWS. Not for review!!! (73.18 KB, patch)
2013-05-03 11:59 PDT, Viatcheslav Ostapenko
no flags Details | Formatted Diff | Diff
Patch for landing (43.60 KB, patch)
2013-05-03 14:43 PDT, Viatcheslav Ostapenko
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.