Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1773403002:
Update SVG text layout to use shaped glyph data & go fast (O(n^2)->O(n)) (Closed)
|
Created:
4 years, 9 months ago by pdr. Modified:
4 years, 9 months ago Reviewers:
fs CC:
blink-reviews, blink-reviews-layout_chromium.org, blink-reviews-platform-graphics_chromium.org, Rik, chromium-reviews, danakj+watch_chromium.org, dshwang, drott+blinkwatch_chromium.org, krit, eae+blinkwatch, f(malita), fs, gyuyoung2, jbroman, jchaffraix+rendering, Justin Novosad, kinuko+watch, kouhei+svg_chromium.org, leviw+renderwatch, pdr+graphicswatchlist_chromium.org, pdr+renderingwatchlist_chromium.org, pdr+svgwatchlist_chromium.org, rwlbuis, Stephen Chennney, szager+layoutwatch_chromium.org, zoltan1 Base URL:
https://chromium.googlesource.com/chromium/src.git@1773353003 Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionUpdate SVG text layout to use shaped glyph data & go fast (O(n^2)->O(n))
When HarfBuzz shapes a run of text it caches the resulting glyph widths
and glyph advances. This patch uses the per-glyph output directly in
SVG's text layout which improves both rendering quality and speed.
Shaping is the process where glyphs are positioned and sized, including
forming combined glyphs such as 'f' and 'i' forming 'fi'.
SVG's text layout used an O(n^2) algorithm for calculating glyph
positions where an entire run would be shaped for each character, and
the differences would be used for glyph widths. This approach is slow
and produces poor glyph data. Using the shaper's glyph advances, we get
improved positioning (see the 'e' in lengthAdjust-text-metrics.html).
By only shaping text once we also get a nice speed improvement. This
patch improves the SVGCubics benchmark by 30%.
BUG=589525
Committed: https://crrev.com/304ec1544273ed8d62c693da6dd2c63727805cdd
Cr-Commit-Position: refs/heads/master@{#380534}
Patch Set 1 #Patch Set 2 : Minor cleanup of comments and tests #
Total comments: 11
Patch Set 3 : Rebase after dependant patch landed, no other changes. #Patch Set 4 : ffix ligature bug, address reviewer comments, add rebaseline entries #Patch Set 5 : Make the grapheme position synthesis loop longer but easier to grok, fix test indentation #Patch Set 6 : Add a missed rebaseline to test expectations #
Total comments: 2
Patch Set 7 : static_cats #Patch Set 8 : Fix TestExpectations collision #Messages
Total messages: 47 (24 generated)
Description was changed from ========== Update SVG text layout to use shaped glyph data and go fast When HarfBuzz shapes a run of text it caches the resulting glyph widths and glyph advances. This patch uses the per-glyph output directly in SVG's text layout which improves both rendering quality and speed. As a reminder, shaping is the process where glyphs are positioned, including forming combined glyphs such as 'f' and 'i' forming 'fi'. SVG's text layout used an O(n^2) algorithm for calculating glyph positions where the entire run would be laid out for each character and the differences would be used for glyph widths. For example: 1) shape 'T', we now know the width of 'T' 2) shape 'Te', then subtract the previous, so we know the width of 'e' 3) shape 'Tex', then subtract the previous, so we know the width of 'x' ... etc This approach is slow and produces poor glyph data. For example, the 'e' in 'Text' is slightly under the 'T' and the above approach will result in an 'e' position too far to the right. Using shaper's glyph advances we get better positioning (see: lengthAdjust-text-metrics.html). In addition, by using the shaper's glyph data we only need to shape text once to get all of the glyph information. As a result, this patch results in a 30% improvement on the SVGCubics benchmark. BUG=589525 ========== to ========== Update SVG text layout to use shaped glyph data and go fast When HarfBuzz shapes a run of text it caches the resulting glyph widths and glyph advances. This patch uses the per-glyph output directly in SVG's text layout which improves both rendering quality and speed. As a reminder, shaping is the process where glyphs are positioned, including forming combined glyphs such as 'f' and 'i' forming 'fi'. SVG's text layout used an O(n^2) algorithm for calculating glyph positions where an entire run would be laid out for each character and the differences would be used for glyph widths. For example: 1) shape 'T', we now know the width of 'T' 2) shape 'Te', then subtract the previous, so we know the width of 'e' 3) shape 'Tex', then subtract the previous, so we know the width of 'x' ... etc This approach is slow and produces poor glyph data. E.g., The 'e' in 'Text' is actually slightly under the 'T' but the above approach will position 'e' too far to the right. Using shaper's glyph advances, we get better positioning (see: lengthAdjust-text-metrics.html). By using the shaper's glyph data we only need to shape text once. As a result, this patch is a 30% improvement on the SVGCubics benchmark. BUG=589525 ==========
Description was changed from ========== Update SVG text layout to use shaped glyph data and go fast When HarfBuzz shapes a run of text it caches the resulting glyph widths and glyph advances. This patch uses the per-glyph output directly in SVG's text layout which improves both rendering quality and speed. As a reminder, shaping is the process where glyphs are positioned, including forming combined glyphs such as 'f' and 'i' forming 'fi'. SVG's text layout used an O(n^2) algorithm for calculating glyph positions where an entire run would be laid out for each character and the differences would be used for glyph widths. For example: 1) shape 'T', we now know the width of 'T' 2) shape 'Te', then subtract the previous, so we know the width of 'e' 3) shape 'Tex', then subtract the previous, so we know the width of 'x' ... etc This approach is slow and produces poor glyph data. E.g., The 'e' in 'Text' is actually slightly under the 'T' but the above approach will position 'e' too far to the right. Using shaper's glyph advances, we get better positioning (see: lengthAdjust-text-metrics.html). By using the shaper's glyph data we only need to shape text once. As a result, this patch is a 30% improvement on the SVGCubics benchmark. BUG=589525 ========== to ========== Update SVG text layout to use shaped glyph data and go fast When HarfBuzz shapes a run of text it caches the resulting glyph widths and glyph advances. This patch uses the per-glyph output directly in SVG's text layout which improves both rendering quality and speed. As a reminder, shaping is the process where glyphs are positioned, including forming combined glyphs such as 'f' and 'i' forming 'fi'. SVG's text layout used an O(n^2) algorithm for calculating glyph positions where an entire run would be laid out for each character and the differences would be used for glyph widths. For example: 1) shape 'T', we now know the width of 'T' 2) shape 'Te', then subtract the previous, so we know the width of 'e' 3) shape 'Tex', then subtract the previous, so we know the width of 'x' ... etc This approach is slow and produces poor glyph data. E.g., the 'e' in 'Text' is actually slightly under the 'T' but the above approach will position 'e' too far to the right. Using shaper's glyph advances, we get better positioning (see: lengthAdjust-text-metrics.html). By using the shaper's glyph data we only need to shape text once. As a result, this patch is a 30% improvement on the SVGCubics benchmark. BUG=589525 ==========
pdr@chromium.org changed reviewers: + fs@opera.com
Description was changed from ========== Update SVG text layout to use shaped glyph data and go fast When HarfBuzz shapes a run of text it caches the resulting glyph widths and glyph advances. This patch uses the per-glyph output directly in SVG's text layout which improves both rendering quality and speed. As a reminder, shaping is the process where glyphs are positioned, including forming combined glyphs such as 'f' and 'i' forming 'fi'. SVG's text layout used an O(n^2) algorithm for calculating glyph positions where an entire run would be laid out for each character and the differences would be used for glyph widths. For example: 1) shape 'T', we now know the width of 'T' 2) shape 'Te', then subtract the previous, so we know the width of 'e' 3) shape 'Tex', then subtract the previous, so we know the width of 'x' ... etc This approach is slow and produces poor glyph data. E.g., the 'e' in 'Text' is actually slightly under the 'T' but the above approach will position 'e' too far to the right. Using shaper's glyph advances, we get better positioning (see: lengthAdjust-text-metrics.html). By using the shaper's glyph data we only need to shape text once. As a result, this patch is a 30% improvement on the SVGCubics benchmark. BUG=589525 ========== to ========== Update SVG text layout to use shaped glyph data and go fast When HarfBuzz shapes a run of text it caches the resulting glyph widths and glyph advances. This patch uses the per-glyph output directly in SVG's text layout which improves both rendering quality and speed. Shaping is the process where glyphs are positioned and sized, including forming combined glyphs such as 'f' and 'i' forming 'fi'. SVG's text layout used an O(n^2) algorithm for calculating glyph positions where an entire run would be laid out for each character and the differences would be used for glyph widths. This approach is slow and produces poor glyph data. Using shaper's glyph advances, we get improved positioning (see the 'e' in lengthAdjust-text-metrics.html). By only shaping text once we also get a nice speed improvement. This patch improves the SVGCubics benchmark by 30%. BUG=589525 ==========
Description was changed from ========== Update SVG text layout to use shaped glyph data and go fast When HarfBuzz shapes a run of text it caches the resulting glyph widths and glyph advances. This patch uses the per-glyph output directly in SVG's text layout which improves both rendering quality and speed. Shaping is the process where glyphs are positioned and sized, including forming combined glyphs such as 'f' and 'i' forming 'fi'. SVG's text layout used an O(n^2) algorithm for calculating glyph positions where an entire run would be laid out for each character and the differences would be used for glyph widths. This approach is slow and produces poor glyph data. Using shaper's glyph advances, we get improved positioning (see the 'e' in lengthAdjust-text-metrics.html). By only shaping text once we also get a nice speed improvement. This patch improves the SVGCubics benchmark by 30%. BUG=589525 ========== to ========== Update SVG text layout to use shaped glyph data and go fast When HarfBuzz shapes a run of text it caches the resulting glyph widths and glyph advances. This patch uses the per-glyph output directly in SVG's text layout which improves both rendering quality and speed. Shaping is the process where glyphs are positioned and sized, including forming combined glyphs such as 'f' and 'i' forming 'fi'. SVG's text layout used an O(n^2) algorithm for calculating glyph positions where an entire run would be shaped for each character, and the differences would be used for glyph widths. This approach is slow and produces poor glyph data. Using the shaper's glyph advances, we get improved positioning (see the 'e' in lengthAdjust-text-metrics.html). By only shaping text once we also get a nice speed improvement. This patch improves the SVGCubics benchmark by 30%. BUG=589525 ==========
Need to wait for https://codereview.chromium.org/1773353003/ to get the test updates, but this is otherwise ready for review.
Yes, I like! LGTM w/ some comments. https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp (right): https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp:48: // TODO(pdr): The surrogate pair concept should be refactored to a single I think we should rather strive to move in the direction of using something more resembling a "typographic character unit" (https://drafts.csswg.org/css-text-3/#typographic-character-unit), and hopefully not need to care about surrogates or not. (Currently the only reason for this is because of the x/y/dx/dy indexing using "glyphs" - and using TCUs would be even better and more correct.) https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp:143: // code below synthesizes a position when two characters share the same Well technically "two or more" (i.e ligatures like 'ffi' and 'ffl'). The code below doesn't deal with that though. I don't know what result you'll get for diacritics, but hopefullly they won't have width == 0. https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp:150: previousRange.end = previousRange.start + midpoint; (previousRange.start + previousRange.end) / 2 for a slightly shorter version (with less FOPS; and * 0.5f even better =)) https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp:193: // TODO(pdr): This loop is too tightly coupled to SVGTextMetricsCalculator. Should we file a (separate) bug for this, or will it be handled in the one referenced here? (It does sound like a good plan - should probably also create a "dummy" BidiCharacterRun for the case where none are created currently.)
The CQ bit was checked by pdr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1773403002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1773403002/80001
Thanks! PTAL Here are the updated tests from the 3rd patchset: https://storage.googleapis.com/chromium-layout-test-archives/linux_chromium_r... https://storage.googleapis.com/chromium-layout-test-archives/mac_chromium_rel... https://storage.googleapis.com/chromium-layout-test-archives/win_chromium_rel... The svg/text/bidi-text-query.svg test now matches firefox and looks much nicer. The updated lengthAdjust-text-metrics results added some Fail lines which also now match firefox. If everything looks good, feel free to cq it. https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp (right): https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp:48: // TODO(pdr): The surrogate pair concept should be refactored to a single On 2016/03/09 at 11:31:44, fs wrote: > I think we should rather strive to move in the direction of using something more resembling a "typographic character unit" (https://drafts.csswg.org/css-text-3/#typographic-character-unit), and hopefully not need to care about surrogates or not. (Currently the only reason for this is because of the x/y/dx/dy indexing using "glyphs" - and using TCUs would be even better and more correct.) Nice! I didn't know the name for this. I've rewritten this comment and filed a separate bug (https://crbug.com/593570) because this is a pretty large project. https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp:143: // code below synthesizes a position when two characters share the same On 2016/03/09 at 11:31:44, fs wrote: > Well technically "two or more" (i.e ligatures like 'ffi' and 'ffl'). The code below doesn't deal with that though. I don't know what result you'll get for diacritics, but hopefullly they won't have width == 0. +1, good catch. I've ffixed this loop and added a test (svg/text/ligature-queries.html). The loop will still incorrectly split combining diacritic markers though. I think this is better than not splitting fi and ffi since combining diacritic markers seem more rare (english-centrism... :/). This is only for combining diacritics so Söderquist with a single-character ö is safe. FWIW, Firefox takes the opposite approach and shows overlapping glyphs for ffi. With a little extra glyph data out of HarfBuzz, I think we'll be able to fix this with https://crbug.com/473476. https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp:150: previousRange.end = previousRange.start + midpoint; On 2016/03/09 at 11:31:44, fs wrote: > (previousRange.start + previousRange.end) / 2 > > for a slightly shorter version (with less FOPS; and * 0.5f even better =)) The new loop now requires a divide 😭 https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp:193: // TODO(pdr): This loop is too tightly coupled to SVGTextMetricsCalculator. On 2016/03/09 at 11:31:44, fs wrote: > Should we file a (separate) bug for this, or will it be handled in the one referenced here? (It does sound like a good plan - should probably also create a "dummy" BidiCharacterRun for the case where none are created currently.) Yeah, I plan to do this one in a followup but wanted to keep this patch simple.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by pdr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1773403002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1773403002/100001
https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp (right): https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp:143: // code below synthesizes a position when two characters share the same On 2016/03/10 at 04:55:15, pdr wrote: > On 2016/03/09 at 11:31:44, fs wrote: > > Well technically "two or more" (i.e ligatures like 'ffi' and 'ffl'). The code below doesn't deal with that though. I don't know what result you'll get for diacritics, but hopefullly they won't have width == 0. > > +1, good catch. I've ffixed this loop and added a test (svg/text/ligature-queries.html). > > The loop will still incorrectly split combining diacritic markers though. I think this is better than not splitting fi and ffi since combining diacritic markers seem more rare (english-centrism... :/). This is only for combining diacritics so Söderquist with a single-character ö is safe. FWIW, Firefox takes the opposite approach and shows overlapping glyphs for ffi. With a little extra glyph data out of HarfBuzz, I think we'll be able to fix this with https://crbug.com/473476. I looked at this some more and we're violating the spec by not using combined glyphs like Firefox. I'll request clarification from the SVGWG but I'd like to continue with the approach here for now.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM w/ uNit https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp (right): https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp:143: // code below synthesizes a position when two characters share the same On 2016/03/10 at 07:21:48, pdr wrote: > On 2016/03/10 at 04:55:15, pdr wrote: > > On 2016/03/09 at 11:31:44, fs wrote: > > > Well technically "two or more" (i.e ligatures like 'ffi' and 'ffl'). The code below doesn't deal with that though. I don't know what result you'll get for diacritics, but hopefullly they won't have width == 0. > > > > +1, good catch. I've ffixed this loop and added a test (svg/text/ligature-queries.html). > > > > The loop will still incorrectly split combining diacritic markers though. I think this is better than not splitting fi and ffi since combining diacritic markers seem more rare (english-centrism... :/). This is only for combining diacritics so Söderquist with a single-character ö is safe. FWIW, Firefox takes the opposite approach and shows overlapping glyphs for ffi. With a little extra glyph data out of HarfBuzz, I think we'll be able to fix this with https://crbug.com/473476. Yes, it more common for us weird people with diacritics to write the composed form - we could as well use the decomposed form though =). > I looked at this some more and we're violating the spec by not using combined glyphs like Firefox. I'll request clarification from the SVGWG but I'd like to continue with the approach here for now. Yeah, I think even the early revisions of the spec somehow spelled out cases like ligatures wrt 'x'/'y' position et.c (and there are tests for it in the official testsuite - text-text-<somenumber> - one of those with lots of blue boxes, short red lines and other weird stuff.) The new ("under construction" *insert GIF with digging person here*) text layout algorithm, some parts of a TCU would be marked "middle": https://svgwg.org/svg2-draft/text.html#TextLayoutAlgorithm And for the record - I'm ok with a "small progressive steps" approach here, so no need to get rid of all the old bagage at once. https://codereview.chromium.org/1773403002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp:193: // TODO(pdr): This loop is too tightly coupled to SVGTextMetricsCalculator. On 2016/03/10 at 04:55:15, pdr wrote: > On 2016/03/09 at 11:31:44, fs wrote: > > Should we file a (separate) bug for this, or will it be handled in the one referenced here? (It does sound like a good plan - should probably also create a "dummy" BidiCharacterRun for the case where none are created currently.) > > Yeah, I plan to do this one in a followup but wanted to keep this patch simple. Roger that. https://codereview.chromium.org/1773403002/diff/100001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/1773403002/diff/100001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/TestExpectations:1259: crbug.com/589525 [ Mac ] svg/W3C-SVG-1.1/text-text-06-t.svg [ NeedsRebaseline ] This is the test that was, originally intended, to test ligature formation in combination with 'x'/'y' positioning. It's much weaker without the custom font though. https://codereview.chromium.org/1773403002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp (right): https://codereview.chromium.org/1773403002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/svg/SVGTextMetricsBuilder.cpp:146: for (int rangeIndex = m_subrunRanges.size() - 1; rangeIndex >= 0; --rangeIndex) { Nit: People like to yell about "implementation-defined"-ness things, so maybe explicitly cast to size() to int here.
The CQ bit was checked by pdr@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com Link to the patchset: https://codereview.chromium.org/1773403002/#ps120001 (title: "static_cats")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1773403002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1773403002/120001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by pdr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1773403002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1773403002/120001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by pdr@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com Link to the patchset: https://codereview.chromium.org/1773403002/#ps140001 (title: "Fix TestExpectations collision")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1773403002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1773403002/140001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by pdr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1773403002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1773403002/140001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by pdr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1773403002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1773403002/140001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
It looks like there is an infrastructure issue: https://bugs.chromium.org/p/chromium/issues/detail?id=593891
The CQ bit was checked by pdr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1773403002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1773403002/140001
Description was changed from ========== Update SVG text layout to use shaped glyph data and go fast When HarfBuzz shapes a run of text it caches the resulting glyph widths and glyph advances. This patch uses the per-glyph output directly in SVG's text layout which improves both rendering quality and speed. Shaping is the process where glyphs are positioned and sized, including forming combined glyphs such as 'f' and 'i' forming 'fi'. SVG's text layout used an O(n^2) algorithm for calculating glyph positions where an entire run would be shaped for each character, and the differences would be used for glyph widths. This approach is slow and produces poor glyph data. Using the shaper's glyph advances, we get improved positioning (see the 'e' in lengthAdjust-text-metrics.html). By only shaping text once we also get a nice speed improvement. This patch improves the SVGCubics benchmark by 30%. BUG=589525 ========== to ========== Update SVG text layout to use shaped glyph data & go fast (O(n^2)->O(n)) When HarfBuzz shapes a run of text it caches the resulting glyph widths and glyph advances. This patch uses the per-glyph output directly in SVG's text layout which improves both rendering quality and speed. Shaping is the process where glyphs are positioned and sized, including forming combined glyphs such as 'f' and 'i' forming 'fi'. SVG's text layout used an O(n^2) algorithm for calculating glyph positions where an entire run would be shaped for each character, and the differences would be used for glyph widths. This approach is slow and produces poor glyph data. Using the shaper's glyph advances, we get improved positioning (see the 'e' in lengthAdjust-text-metrics.html). By only shaping text once we also get a nice speed improvement. This patch improves the SVGCubics benchmark by 30%. BUG=589525 ==========
Message was sent while issue was closed.
Committed patchset #8 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== Update SVG text layout to use shaped glyph data & go fast (O(n^2)->O(n)) When HarfBuzz shapes a run of text it caches the resulting glyph widths and glyph advances. This patch uses the per-glyph output directly in SVG's text layout which improves both rendering quality and speed. Shaping is the process where glyphs are positioned and sized, including forming combined glyphs such as 'f' and 'i' forming 'fi'. SVG's text layout used an O(n^2) algorithm for calculating glyph positions where an entire run would be shaped for each character, and the differences would be used for glyph widths. This approach is slow and produces poor glyph data. Using the shaper's glyph advances, we get improved positioning (see the 'e' in lengthAdjust-text-metrics.html). By only shaping text once we also get a nice speed improvement. This patch improves the SVGCubics benchmark by 30%. BUG=589525 ========== to ========== Update SVG text layout to use shaped glyph data & go fast (O(n^2)->O(n)) When HarfBuzz shapes a run of text it caches the resulting glyph widths and glyph advances. This patch uses the per-glyph output directly in SVG's text layout which improves both rendering quality and speed. Shaping is the process where glyphs are positioned and sized, including forming combined glyphs such as 'f' and 'i' forming 'fi'. SVG's text layout used an O(n^2) algorithm for calculating glyph positions where an entire run would be shaped for each character, and the differences would be used for glyph widths. This approach is slow and produces poor glyph data. Using the shaper's glyph advances, we get improved positioning (see the 'e' in lengthAdjust-text-metrics.html). By only shaping text once we also get a nice speed improvement. This patch improves the SVGCubics benchmark by 30%. BUG=589525 Committed: https://crrev.com/304ec1544273ed8d62c693da6dd2c63727805cdd Cr-Commit-Position: refs/heads/master@{#380534} ==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/304ec1544273ed8d62c693da6dd2c63727805cdd Cr-Commit-Position: refs/heads/master@{#380534} |
