Bug 51584 - [Qt] Glyphs in vertical text tests are rotated 90 degrees clockwise
Summary: [Qt] Glyphs in vertical text tests are rotated 90 degrees clockwise
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: PlatformOnly, Qt, QtTriaged
Depends on:
Blocks: 98587
  Show dependency treegraph
 
Reported: 2010-12-23 23:45 PST by Koan-Sin Tan
Modified: 2014-02-03 03:17 PST (History)
9 users (show)

See Also:


Attachments
initial patch to make vertical text work (9.69 KB, patch)
2010-12-24 00:24 PST, Koan-Sin Tan
hyatt: review-
Details | Formatted Diff | Diff
updated patch (work in progress) (8.48 KB, patch)
2011-01-24 21:46 PST, Koan-Sin Tan
no flags Details | Formatted Diff | Diff
screenshot (166.62 KB, image/png)
2011-01-24 21:47 PST, Koan-Sin Tan
no flags Details
updated patch[solved a bug] (585 bytes, patch)
2012-04-16 03:14 PDT, yangc248
no flags Details | Formatted Diff | Diff
updated patch (support calculating width for vertical text ) (2.65 KB, patch)
2012-07-02 20:26 PDT, yangc248
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Koan-Sin Tan 2010-12-23 23:45:58 PST
As discussed in https://bugs.webkit.org/show_bug.cgi?id=48459, If you run fast/blockflow/japanese-*-text.html on QtWebKit you'll see that the glyphs are rotated 90 degrees clockwise from how they appear on WebKit Mac and Chromium Mac/Linux. The reason is that vertical text is not implemented on QtWeKit yet.
Comment 1 Koan-Sin Tan 2010-12-24 00:24:20 PST
Created attachment 77406 [details]
initial patch to make vertical text work

An initial patch to make vertical text work. There are many problems in this patch, but it works.

1. Qt doesn't export font/text matrix (yet), so we have to rotate the QPainter at least once for each QString
2. Qt doesn't export API to get GSUB tables (yet), it's not quite difficult to do vertical glyph substitution
3. Currently, glyph positions use QFontMetrics to measure the width a horizontal string. This is not quite. Should be able to measure the height of a string with vertical layout
Comment 2 Eric Seidel (no email) 2010-12-24 01:42:48 PST
Why does every platform have to solve this individually?
bug 48459, bug 51450 and bug 50619
Comment 3 Koan-Sin Tan 2010-12-24 02:12:48 PST
(In reply to comment #2)
> Why does every platform have to solve this individually?
> bug 48459, bug 51450 and bug 50619

Platform Independent part is done by Dave Hyatt, I think. These 
are platform dependent parts. I was intended to submitted
all the patches to bug 48459, but was told to file a new 
bug for Chromium Linux. It turns out these are quite different.

Chromium Linux uses Skia and has HarfBuzz exported.
Chromium Windows uses Windows GDI
Gtk uses Gtk+Cairo+freetype2
Qt uses its own QFont

Some use Glyphs API, some not.
Comment 4 Kenneth Rohde Christiansen 2010-12-24 02:15:33 PST
> Chromium Linux uses Skia and has HarfBuzz exported.
> Chromium Windows uses Windows GDI
> Gtk uses Gtk+Cairo+freetype2
> Qt uses its own QFont

Qt also uses HarfBuzz. In Qt 4.8 we will export the glyph API, thought I believe we will already export it privately in a 4.7.x subrelease.

> Some use Glyphs API, some not.
Comment 5 Koan-Sin Tan 2010-12-29 05:12:11 PST
(In reply to comment #4)
> > Chromium Linux uses Skia and has HarfBuzz exported.
> > Chromium Windows uses Windows GDI
> > Gtk uses Gtk+Cairo+freetype2
> > Qt uses its own QFont
> 
> Qt also uses HarfBuzz. In Qt 4.8 we will export the glyph API, thought I believe we will already export it privately in a 4.7.x subrelease.

Sounds good. But my Mac Ports uses Qt 4.7, my Ubuntu 10.04 still uses 4.6, I think :-)
Comment 6 Benjamin Poulain 2011-01-03 04:26:38 PST
Comment on attachment 77406 [details]
initial patch to make vertical text work

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

> WebCore/platform/graphics/qt/FontQt.cpp:222
> +                   drawVertical(p, pt, string);

A space is missing here for the indentation.
Comment 7 Dave Hyatt 2011-01-06 15:31:20 PST
Comment on attachment 77406 [details]
initial patch to make vertical text work

This code only sort of supports vertical text.  You still need code that will pick the correct vertical glyphs (punctuation glyphs are different for example).  It's not enough to just rotate the ideographs.
Comment 8 Koan-Sin Tan 2011-01-09 18:07:34 PST
(In reply to comment #7)
> (From update of attachment 77406 [details])
> This code only sort of supports vertical text.  You still need code that will pick the correct vertical glyphs (punctuation glyphs are different for example).  It's not enough to just rotate the ideographs.

Yes, but, Qt doesn't export API to get GSUB tables (yet), it's quite difficult to do vertical glyph substitution :-)
Comment 9 Koan-Sin Tan 2011-01-24 21:46:34 PST
Created attachment 80015 [details]
updated patch (work in progress)

need Jiang Jiang's patched Qt 4.8, http://qt.gitorious.org/~jiang/qt/jiangs-qt/commits/vertical
Comment 10 Koan-Sin Tan 2011-01-24 21:47:40 PST
Created attachment 80016 [details]
screenshot
Comment 11 yangc248 2012-04-16 03:14:56 PDT
Created attachment 137308 [details]
updated patch[solved a bug]

The old patch can't work well on other's vertion.(such as webkit R97664).
Because there is a bug on the old patch.
This new patch solve the bug.
Comment 12 yangc248 2012-04-16 03:37:46 PDT
(In reply to comment #1)
> Created an attachment (id=77406) [details]
> initial patch to make vertical text work
> 
> An initial patch to make vertical text work. There are many problems in this patch, but it works.
> 
> 1. Qt doesn't export font/text matrix (yet), so we have to rotate the QPainter at least once for each QString
> 2. Qt doesn't export API to get GSUB tables (yet), it's not quite difficult to do vertical glyph substitution
> 3. Currently, glyph positions use QFontMetrics to measure the width a horizontal string. This is not quite. Should be able to measure the height of a string with vertical layout

In my project ,
1、I had updated QFontMetrics'width interface to support vertical flag.
2、I had updated freetype font engine to get the gsub tables.
3、I had done some codes to support spectial process(For example,90 degree rotate 、 180 degree rotate for special glyph)
4、I had modified some bugs in Jiang Jiang's patch. 

I dont's know whether this patchs can be passed.
Comment 13 yangc248 2012-07-02 20:26:09 PDT
Created attachment 150521 [details]
updated patch (support calculating width for vertical text )
Comment 14 yangc248 2012-07-02 22:28:08 PDT
Comment on attachment 150521 [details]
updated patch (support calculating width for vertical text ) 

This patch have to work together with patchs in https://bugreports.qt-project.org/browse/QTBUG-529.
Comment 15 WebKit Commit Bot 2013-06-17 17:11:29 PDT
Attachment 150521 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1
Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 16 Jocelyn Turcotte 2014-02-03 03:17:05 PST
=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.