lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Split glyph contours in up/down segments for skylines (issue 5697000


From: hanwenn
Subject: Re: Split glyph contours in up/down segments for skylines (issue 569700043 by address@hidden)
Date: Fri, 08 May 2020 07:03:23 -0700

https://codereview.appspot.com/569700043/diff/582060043/lily/freetype.cc
File lily/freetype.cc (right):

https://codereview.appspot.com/569700043/diff/582060043/lily/freetype.cc#newcode105
lily/freetype.cc:105: bool is_tt = (0 == strcmp ("TrueType",
FT_Get_Font_Format (face)));
On 2020/05/08 08:15:58, hahnjo wrote:
> Can we have this as a comparison of std::string to avoid strcmp?

Done.

https://codereview.appspot.com/569700043/diff/582060043/lily/freetype.cc#newcode126
lily/freetype.cc:126: else if (outline->tags[j] & 1)
On 2020/05/08 08:15:58, hahnjo wrote:
> is there a define for this magic number?

no. See
https://www.freetype.org/freetype2/docs/reference/ft2-outline_processing.html
- I don't really understand how the description of the tags work with
the code here, but if Werner says it's OK, who am I to doubt?

https://codereview.appspot.com/569700043/diff/582060043/lily/freetype.cc#newcode135
lily/freetype.cc:135: else if (outline->tags[j] & 2)
On 2020/05/08 08:15:59, hahnjo wrote:
> same

Acknowledged.

https://codereview.appspot.com/569700043/diff/582060043/lily/freetype.cc#newcode162
lily/freetype.cc:162: }
On 2020/05/08 08:15:59, hahnjo wrote:
> This code looks very familiar. Don't we already have it in
> make_draw_bezier_boxes?

yes.

https://codereview.appspot.com/569700043/diff/582060043/lily/freetype.cc#newcode164
lily/freetype.cc:164: else
On 2020/05/08 08:15:58, hahnjo wrote:
> I'd advocate another else if and have an assert in the else branch
(but I know
> it's been this way in the old code)

sure. But what would we assert? FT reserves the right to put other
things in tag bits.

https://codereview.appspot.com/569700043/diff/582060043/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):

https://codereview.appspot.com/569700043/diff/582060043/lily/stencil-integral.cc#newcode237
lily/stencil-integral.cc:237: points[i + 1]);
On 2020/05/08 08:15:59, hahnjo wrote:
> AFAICS this is switching from CCW to CW - is there an advantage for
this?

the previous code didn't have an ordering, afaict.

https://codereview.appspot.com/569700043/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]