Bug 44403

Summary:

[Qt] -webkit-text-stroke is broken due to Qt::TextBypassShaping

Product:

WebKit

Reporter:

Andreas Kling <kling>

Component:

Layout and Rendering

Assignee:

QtWebKit Unassigned <webkit-qt-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ariya.hidayat, hausmann, jturcotte, kenneth

Priority:

Keywords:

Qt, QtTriaged, Regression

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Screenshot showing this bug	none
Proposed patch	kling: review-
Proposed patch v2	none
Proposed patch v3	kenneth: review+

Andreas Kling

Reported 2010-08-22 21:13:36 PDT

Created attachment 65071 [details] Screenshot showing this bug Since there is no way to pass the Qt::TextBypassShaping flag to QPainterPath::addText(), what we stroke may differ from what we fill (and what we paint as a shadow.) See the attached image for an example of this bug.

Attachments
Screenshot showing this bug (23.87 KB, image/png) 2010-08-22 21:13 PDT, Andreas Kling	no flags	Details
Proposed patch (1.41 KB, patch) 2010-08-22 21:16 PDT, Andreas Kling	kling: review-	Details Formatted Diff Diff
Proposed patch v2 (5.76 KB, patch) 2010-08-23 02:48 PDT, Andreas Kling	no flags	Details Formatted Diff Diff
Proposed patch v3 (2.81 KB, patch) 2010-08-23 04:30 PDT, Andreas Kling	kenneth: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Andreas Kling

Comment 1 2010-08-22 21:16:54 PDT

Created attachment 65072 [details] Proposed patch

Ariya Hidayat

Comment 2 2010-08-22 21:50:25 PDT

Even with the patch, does that mean if I fill the text first and then stroke it, they will not be the same? Hopefully drawTextCommon is never called twice with such order.

Andreas Kling

Comment 3 2010-08-22 22:05:14 PDT

(In reply to comment #2) > Even with the patch, does that mean if I fill the text first and then stroke it, they will not be the same? > Hopefully drawTextCommon is never called twice with such order. Fudge! You're right. The bug can still be triggered by separate calls to strokeText() and fillText() on a CanvasRenderingContext2D.

Andreas Kling

Comment 4 2010-08-22 22:05:53 PDT

Comment on attachment 65072 [details] Proposed patch Self r-, need to fix this for CRC2D strokeText()/fillText(), too.

Andreas Kling

Comment 5 2010-08-23 02:48:13 PDT

Created attachment 65091 [details] Proposed patch v2 To avoid the problem when using the Canvas API, I've added a "shouldForceComplexShaping" property to Font (Qt-only.)

Jocelyn Turcotte

Comment 6 2010-08-23 04:18:12 PDT

I think that using forceComplexShaping would be shorter and have the same meaning as shouldForceComplexShaping, though it doesn't matter much. Still looks good to me.

Andreas Kling

Comment 7 2010-08-23 04:30:39 PDT

Created attachment 65101 [details] Proposed patch v3 Talked to Simon (IRL) about this and he suggested that I use Font::setCodePath() instead, which is much cleaner than adding a platform-specific member to Font.

Kenneth Rohde Christiansen

Comment 8 2010-08-23 04:32:08 PDT

Comment on attachment 65101 [details] Proposed patch v3 A lot cleaner indeed!

Kenneth Rohde Christiansen

Comment 9 2010-08-23 04:33:17 PDT

(In reply to comment #8) > (From update of attachment 65101 [details]) > A lot cleaner indeed! Still maybe some comments in the ifdef's would be nice!

Andreas Kling

Comment 10 2010-08-23 04:34:37 PDT

(In reply to comment #9) > Still maybe some comments in the ifdef's would be nice! Will land with comments. Thanks for the review <3

Andreas Kling

Comment 11 2010-08-23 04:44:59 PDT

Committed r65801: <http://trac.webkit.org/changeset/65801>

Simon Hausmann

Comment 12 2010-08-24 03:09:33 PDT

Revision r65801 cherry-picked into qtwebkit-2.1 with commit 1a184f3f6e088a4d5e4afd5ed71d3cb8d07a5a53

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