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:

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

Laszlo Gombos

Reported 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.

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
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Viatcheslav Ostapenko

Comment 1 2013-03-22 08:00:29 PDT

Created attachment 194547 [details] Unscaled page sample with underlines.

Viatcheslav Ostapenko

Comment 2 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.

Viatcheslav Ostapenko

Comment 3 2013-03-22 23:49:08 PDT

Created attachment 194693 [details] Patch

Martin Robinson

Comment 4 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.

Viatcheslav Ostapenko

Comment 5 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).

Martin Robinson

Comment 6 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.

Viatcheslav Ostapenko

Comment 7 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?

Martin Robinson

Comment 8 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.

Viatcheslav Ostapenko

Comment 9 2013-05-03 11:36:12 PDT

Created attachment 200444 [details] Patch for test results

Viatcheslav Ostapenko

Comment 10 2013-05-03 11:43:22 PDT

Created attachment 200447 [details] Patch for test results

Viatcheslav Ostapenko

Comment 11 2013-05-03 11:59:43 PDT

Created attachment 200451 [details] Failing test to get pixel test results from EWS. Not for review!!!

Viatcheslav Ostapenko

Comment 12 2013-05-03 14:43:16 PDT

Created attachment 200469 [details] Patch for landing

WebKit Commit Bot

Comment 13 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>

WebKit Commit Bot

Comment 14 2013-05-03 15:12:08 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.