Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(256)

Issue 4539045: [PDF] Fix ending condition for font advance construction. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 6 months ago by Steve VanDeBogart
Modified:
13 years, 6 months ago
Reviewers:
Chris Guillory
CC:
skia-review_googlegroups.com, arthurhsu
Base URL:
https://skia.googlecode.com/svn/trunk
Visibility:
Public.

Description

[PDF] Fix ending condition for font advance construction. The old code always ended (the last sequence of glyphs) on a range, even if there was a very long run at the end. This fixes that. Committed: http://code.google.com/p/skia/source/detail?r=1293

Patch Set 1 #

Patch Set 2 : Change to a fix that doesn't copy a big chunk of code #

Patch Set 3 : Fix bug #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+18 lines, -4 lines) Patch
M src/core/SkAdvancedTypefaceMetrics.cpp View 1 2 5 chunks +18 lines, -4 lines 2 comments Download

Messages

Total messages: 5
Chris Guillory
Ready for review?
13 years, 6 months ago (2011-05-10 19:51:15 UTC) #1
Steve VanDeBogart
Ready for review.
13 years, 6 months ago (2011-05-10 22:54:09 UTC) #2
Chris Guillory
LGTM. One comment. http://codereview.appspot.com/4539045/diff/3002/src/core/SkAdvancedTypefaceMetrics.cpp File src/core/SkAdvancedTypefaceMetrics.cpp (right): http://codereview.appspot.com/4539045/diff/3002/src/core/SkAdvancedTypefaceMetrics.cpp#newcode122 src/core/SkAdvancedTypefaceMetrics.cpp:122: if (curRange->fStartId == num_glyphs) { Should ...
13 years, 6 months ago (2011-05-10 23:19:11 UTC) #3
Steve VanDeBogart
http://codereview.appspot.com/4539045/diff/3002/src/core/SkAdvancedTypefaceMetrics.cpp File src/core/SkAdvancedTypefaceMetrics.cpp (right): http://codereview.appspot.com/4539045/diff/3002/src/core/SkAdvancedTypefaceMetrics.cpp#newcode122 src/core/SkAdvancedTypefaceMetrics.cpp:122: if (curRange->fStartId == num_glyphs) { On 2011/05/10 23:19:12, Chris ...
13 years, 6 months ago (2011-05-10 23:24:03 UTC) #4
Chris Guillory
13 years, 6 months ago (2011-05-10 23:24:32 UTC) #5
On 2011/05/10 23:24:03, Steve VanDeBogart wrote:
>
http://codereview.appspot.com/4539045/diff/3002/src/core/SkAdvancedTypefaceMe...
> File src/core/SkAdvancedTypefaceMetrics.cpp (right):
> 
>
http://codereview.appspot.com/4539045/diff/3002/src/core/SkAdvancedTypefaceMe...
> src/core/SkAdvancedTypefaceMetrics.cpp:122: if (curRange->fStartId ==
> num_glyphs) {
> On 2011/05/10 23:19:12, Chris Guillory wrote:
> > Should this also verify that prevRange is non-NULL (edge case where
num_glyphs
> > is 0).
> 
> If num_glyphs is 0, then something before this is probably going to break
> anyway.
Fair enough.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b