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

Issue 566080043: Clean up and fix glyph contour generation

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months, 1 week ago by hanwenn
Modified:
1 week, 3 days ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Clean up and fix glyph contour generation Use FT_Outline_Decompose to do the real work. This fixes at least two bugs in the existing code: * Glyphs with multiple contours (e.g. "O" or "i") should walk the contours separately * Contours can wrap around to the start. This leads to OOB reads and/or incorrect outlines in some circumstances.

Patch Set 1 #

Total comments: 2

Patch Set 2 : Use FT_Outline_Decompose #

Total comments: 8

Patch Set 3 : jonas #

Patch Set 4 : const& #

Unified diffs Side-by-side diffs Delta from patch set Stats (+99 lines, -51 lines) Patch
M lily/freetype.cc View 1 2 3 3 chunks +99 lines, -51 lines 0 comments Download

Messages

Total messages: 21
hanwenn
Werner, can you confirm that this looks correct? I'm especially not certain about the wrapping.
4 months, 1 week ago (2020-05-09 10:36:02 UTC) #1
lemzwerg
I don't fully understand this code. Please add more comments. A quite well documented routine ...
4 months, 1 week ago (2020-05-09 14:23:04 UTC) #2
hahnjo
Why is this separate from https://codereview.appspot.com/569700043 ?
4 months, 1 week ago (2020-05-09 17:51:39 UTC) #3
hanwenn
it's separate because it causes formatting changes. I like to keep the bugfixes and performance ...
4 months, 1 week ago (2020-05-09 20:38:12 UTC) #4
hanwenn
On 2020/05/09 14:23:04, lemzwerg wrote: > I don't fully understand this code. that is an ...
4 months, 1 week ago (2020-05-09 20:42:36 UTC) #5
Dan Eble
On 2020/05/09 20:38:12, hanwenn wrote: > it's separate because it causes formatting changes. I like ...
4 months, 1 week ago (2020-05-09 21:00:01 UTC) #6
lemzwerg
> > I don't fully understand this code. > > that is an ominous comment ...
4 months, 1 week ago (2020-05-10 04:15:49 UTC) #7
hanwenn
Use FT_Outline_Decompose
4 months, 1 week ago (2020-05-10 08:25:35 UTC) #8
hahnjo
On 2020/05/09 20:38:12, hanwenn wrote: > it's separate because it causes formatting changes. I like ...
4 months, 1 week ago (2020-05-10 08:38:12 UTC) #9
hanwenn
On Sun, May 10, 2020 at 10:38 AM <jonas.hahnfeld@gmail.com> wrote: > > On 2020/05/09 20:38:12, ...
4 months, 1 week ago (2020-05-10 08:58:10 UTC) #10
hahnjo
https://codereview.appspot.com/566080043/diff/560020046/lily/freetype.cc File lily/freetype.cc (right): https://codereview.appspot.com/566080043/diff/560020046/lily/freetype.cc#newcode143 lily/freetype.cc:143: }; Not sure if FT developers plan to change ...
4 months, 1 week ago (2020-05-10 09:16:59 UTC) #11
hanwenn
jonas
4 months, 1 week ago (2020-05-10 09:29:45 UTC) #12
hanwenn
https://codereview.appspot.com/566080043/diff/560020046/lily/freetype.cc File lily/freetype.cc (right): https://codereview.appspot.com/566080043/diff/560020046/lily/freetype.cc#newcode143 lily/freetype.cc:143: }; On 2020/05/10 09:16:58, hahnjo wrote: > Not sure ...
4 months, 1 week ago (2020-05-10 09:29:48 UTC) #13
hahnjo
On 2020/05/10 09:29:48, hanwenn wrote: > https://codereview.appspot.com/566080043/diff/560020046/lily/freetype.cc > File lily/freetype.cc (right): > > https://codereview.appspot.com/566080043/diff/560020046/lily/freetype.cc#newcode143 > ...
4 months, 1 week ago (2020-05-10 10:00:54 UTC) #14
hanwenn
On 2020/05/10 10:00:54, hahnjo wrote: > On 2020/05/10 09:29:48, hanwenn wrote: > > https://codereview.appspot.com/566080043/diff/560020046/lily/freetype.cc > ...
4 months, 1 week ago (2020-05-10 10:02:27 UTC) #15
hahnjo
On 2020/05/10 10:02:27, hanwenn wrote: > On 2020/05/10 10:00:54, hahnjo wrote: > > On 2020/05/10 ...
4 months, 1 week ago (2020-05-10 10:03:57 UTC) #16
hanwenn
On 2020/05/10 10:00:54, hahnjo wrote: > On 2020/05/10 09:29:48, hanwenn wrote: > > https://codereview.appspot.com/566080043/diff/560020046/lily/freetype.cc > ...
4 months, 1 week ago (2020-05-10 10:04:44 UTC) #17
hanwenn
const&
4 months, 1 week ago (2020-05-10 10:07:27 UTC) #18
hanwenn
On 2020/05/10 10:03:57, hahnjo wrote: > > > > yes. I find it easier to ...
4 months, 1 week ago (2020-05-10 10:08:37 UTC) #19
lemzwerg
*Much* better to read and understand, thanks! LGTM now.
4 months, 1 week ago (2020-05-10 11:12:37 UTC) #20
aaireill111
1 week, 3 days ago (2020-09-09 02:50:00 UTC) #21
On 2020/05/10 11:12:37, lemzwerg wrote:
> *Much* better to read and understand, thanks!  LGTM now.
Sign in to reply to this message.

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