WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
44403
[Qt] -webkit-text-stroke is broken due to Qt::TextBypassShaping
https://bugs.webkit.org/show_bug.cgi?id=44403
Summary
[Qt] -webkit-text-stroke is broken due to Qt::TextBypassShaping
Andreas Kling
Reported
Monday, August 23, 2010 5:13:36 AM UTC
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
Monday, August 23, 2010 5:16:54 AM UTC
Created
attachment 65072
[details]
Proposed patch
Ariya Hidayat
Comment 2
Monday, August 23, 2010 5:50:25 AM UTC
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
Monday, August 23, 2010 6:05:14 AM UTC
(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
Monday, August 23, 2010 6:05:53 AM UTC
Comment on
attachment 65072
[details]
Proposed patch Self r-, need to fix this for CRC2D strokeText()/fillText(), too.
Andreas Kling
Comment 5
Monday, August 23, 2010 10:48:13 AM UTC
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
Monday, August 23, 2010 12:18:12 PM UTC
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
Monday, August 23, 2010 12:30:39 PM UTC
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
Monday, August 23, 2010 12:32:08 PM UTC
Comment on
attachment 65101
[details]
Proposed patch v3 A lot cleaner indeed!
Kenneth Rohde Christiansen
Comment 9
Monday, August 23, 2010 12:33:17 PM UTC
(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
Monday, August 23, 2010 12:34:37 PM UTC
(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
Monday, August 23, 2010 12:44:59 PM UTC
Committed
r65801
: <
http://trac.webkit.org/changeset/65801
>
Simon Hausmann
Comment 12
Tuesday, August 24, 2010 11:09:33 AM UTC
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.
Top of Page
Format For Printing
XML
Clone This Bug