Bug 48085 - Make basic vertical text painting work.
Summary: Make basic vertical text painting work.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks: 46123
  Show dependency treegraph
 
Reported: 2010-10-21 12:10 PDT by Dave Hyatt
Modified: 2010-10-21 18:15 PDT (History)
2 users (show)

See Also:


Attachments
Patch (126.63 KB, patch)
2010-10-21 12:12 PDT, Dave Hyatt
darin: review+
Details | Formatted Diff | Diff
Patch that adds new results from Takano's change. (152.92 KB, patch)
2010-10-21 14:14 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
New ugly results from Takano's and my patch combining. (148.27 KB, patch)
2010-10-21 14:28 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 2010-10-21 12:10:14 PDT
Make basic vertical text painting work.  This includes the text itself, underlines, overlines, line-throughs and shadows.

Selection would draw correctly if hit testing worked well or if invalidation worked well. :)
Comment 1 Dave Hyatt 2010-10-21 12:12:28 PDT
Created attachment 71469 [details]
Patch
Comment 2 Darin Adler 2010-10-21 12:39:37 PDT
Comment on attachment 71469 [details]
Patch

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

> WebCore/WebCore.xcodeproj/project.pbxproj:-21155
> -			developmentRegion = English;

You need to update your Xcode with Software Update. The newer version doesn’t remove this, and you should not check in this project change.

> WebCore/rendering/InlineFlowBox.cpp:-708
> -            paintTextDecorations(paintInfo, tx, ty, false);

How does this work now? Is there other code that always paints text decorations now that used to do it only conditionally?

> WebCore/rendering/InlineTextBox.cpp:433
> +        context->rotate(deg2rad(90.));

We normally would just use 90 here instead of "90." but maybe that will cause overloading problems with the deg2rad function. Also, this will call deg2rad at runtime -- maybe we should include a constant you can use for this. Maybe a helper function to rotate 90 degrees would be cleaner?
Comment 3 Dave Hyatt 2010-10-21 13:16:28 PDT
(In reply to comment #2)
>
> > WebCore/rendering/InlineFlowBox.cpp:-708
> > -            paintTextDecorations(paintInfo, tx, ty, false);
> 
> How does this work now? Is there other code that always paints text decorations now that used to do it only conditionally?
> 

We have two complete text decoration drawing code paths.  One is for quirks mode and one is for strict mode.  CSS2.1 has defined a third model that is not compatible with either of our models (yay).  However our quirks mode model is much closer to what CSS2.1 wants.  In addition CSS2.1 states that "older UAs that predate CSS2.1 are still conformant" if we implement text-decoration the way our quirks mode model does.

This patch is removing the strict mode text decoration code path and just making the quirks mode one the only path we use.
Comment 4 Dave Hyatt 2010-10-21 14:14:25 PDT
Created attachment 71489 [details]
Patch that adds new results from Takano's change.
Comment 5 Dave Hyatt 2010-10-21 14:17:24 PDT
Fixed in r70263.
Comment 6 Dave Hyatt 2010-10-21 14:28:01 PDT
Created attachment 71493 [details]
New ugly results from Takano's and my patch combining.

Patch
Comment 7 Eric Seidel (no email) 2010-10-21 14:49:07 PDT
This broke snow leopard.
Comment 8 Ryosuke Niwa 2010-10-21 18:15:36 PDT
(In reply to comment #7)
> This broke snow leopard.

Fixed in http://trac.webkit.org/changeset/70281.  Please verify the fix.