Bug 151051 - tate-chu-yoko should shrink to fit when it exceeds the available space
Summary: tate-chu-yoko should shrink to fit when it exceeds the available space
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
: 93832 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-11-09 13:10 PST by Dave Hyatt
Modified: 2016-01-05 00:56 PST (History)
9 users (show)

See Also:


Attachments
Patch (141.78 KB, patch)
2015-11-09 13:14 PST, Dave Hyatt
no flags Details | Formatted Diff | Diff
Patch (136.30 KB, patch)
2015-11-09 13:26 PST, Dave Hyatt
mmaxfield: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-mavericks (1.12 MB, application/zip)
2015-11-09 13:57 PST, Build Bot
no flags Details
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (1.09 MB, application/zip)
2015-11-09 14:08 PST, Build Bot
no flags Details
Patch for EWS testing (135.89 KB, patch)
2015-11-10 11:33 PST, Dave Hyatt
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (1.04 MB, application/zip)
2015-11-10 12:08 PST, Build Bot
no flags Details
Archive of layout-test-results from ews100 for mac-mavericks (878.11 KB, application/zip)
2015-11-10 12:19 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 2015-11-09 13:10:01 PST
tate-chu-yoko needs to shrink to fit (within reason) if it exceeds the amount of available space.
Comment 1 Dave Hyatt 2015-11-09 13:14:37 PST
Created attachment 265089 [details]
Patch
Comment 2 Dave Hyatt 2015-11-09 13:26:11 PST
Created attachment 265092 [details]
Patch
Comment 3 Myles C. Maxfield 2015-11-09 13:50:24 PST
Comment on attachment 265092 [details]
Patch

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

It seems to me that this is going to squish the tatechuyoko vertically as well as horizontally. Are we sure that fits the bill? I expected you to do something with matrix transformations to squish in only a single direction.

> Source/WebCore/rendering/RenderCombineText.cpp:121
> +    

Please remove this whitespace.

> Source/WebCore/rendering/RenderCombineText.cpp:133
> +            glyphOverflow.left = glyphOverflow.top = glyphOverflow.right = glyphOverflow.bottom = 0;

Is this necessary? Shouldn't the callee be doing this?

> Source/WebCore/rendering/RenderCombineText.cpp:159
> +            FontCascade compressedFont(bestFitDescription, style().fontCascade().letterSpacing(), style().fontCascade().wordSpacing());

Shouldn't we be picking between style() and firstLineStyle() appropriately?

> Source/WebCore/rendering/RenderCombineText.cpp:169
> +            computedSize--;

I don't think we should be looping on the order of the original font size. Isn't there a way to take bigger steps the further out we are?
Comment 4 Myles C. Maxfield 2015-11-09 13:53:44 PST
Comment on attachment 265092 [details]
Patch

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

> LayoutTests/fast/text/text-combine-crash-expected.txt:-5
> -foo   

What? Why did this change?

> LayoutTests/fast/text/international/spaces-combined-in-vertical-text-expected.txt:-1
> -PASS if no ASSERT fail or crash in debug build.

What? Why did this change?
Comment 5 Build Bot 2015-11-09 13:57:41 PST
Comment on attachment 265092 [details]
Patch

Attachment 265092 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/407053

New failing tests:
fast/text/international/text-combine-image-test.html
Comment 6 Build Bot 2015-11-09 13:57:45 PST
Created attachment 265095 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 7 Myles C. Maxfield 2015-11-09 14:00:10 PST
Comment on attachment 265092 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Covered by existing tests

We should make sure that RenderCombineText::width() uses the correct (shrunken) font too (it looks like it does from the .pngs but it isn't obvious)

> LayoutTests/platform/mac/fast/text/decorations-with-text-combine-expected.txt:-40
> -              text run at (6,288) width 71: "12345"

I'm not sure if combining 12345 here is best. But I'm also not sure if keeping it uncombined here is best. Hrm.
Comment 8 Myles C. Maxfield 2015-11-09 14:01:55 PST
Comment on attachment 265092 [details]
Patch

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

> Source/WebCore/rendering/RenderCombineText.cpp:170
> +        } while (computedSize >= 6.0f);

Hardcoding this 6 here is probably bad, given the original font size could be humongous, making 6px way way too small relatively.
Comment 9 Myles C. Maxfield 2015-11-09 14:08:21 PST
Comment on attachment 265092 [details]
Patch

After some research, it appears that shrinking in both dimensions is fine.
Comment 10 Build Bot 2015-11-09 14:08:30 PST
Comment on attachment 265092 [details]
Patch

Attachment 265092 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/407081

New failing tests:
fast/text/international/text-combine-image-test.html
Comment 11 Build Bot 2015-11-09 14:08:35 PST
Created attachment 265096 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 12 Dave Hyatt 2015-11-10 11:33:58 PST
Created attachment 265205 [details]
Patch for EWS testing
Comment 13 Build Bot 2015-11-10 12:08:47 PST
Comment on attachment 265205 [details]
Patch for EWS testing

Attachment 265205 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/411302

New failing tests:
fast/text/international/text-combine-image-test.html
Comment 14 Build Bot 2015-11-10 12:08:51 PST
Created attachment 265213 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 15 Build Bot 2015-11-10 12:19:06 PST
Comment on attachment 265205 [details]
Patch for EWS testing

Attachment 265205 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/411353

New failing tests:
fast/text/international/text-combine-image-test.html
Comment 16 Build Bot 2015-11-10 12:19:10 PST
Created attachment 265217 [details]
Archive of layout-test-results from ews100 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 17 Dave Hyatt 2015-11-10 14:05:12 PST
Fixed in r192269.
Comment 18 Koji Ishii 2015-11-20 00:00:35 PST
*** Bug 93832 has been marked as a duplicate of this bug. ***
Comment 19 Csaba Osztrogonác 2016-01-05 00:56:52 PST
(In reply to comment #17)
> Fixed in r192269.

The commit log was repeated 47 times.  :(  Dave, could you check your script?