Re: Clean up and fix glyph contour generation nits. (issue 566080043 by hanw...@gmail.com)

> > I don't fully understand this code. > > that is an ominous comment to come from the Freetype > author. I had assumed you reviewed the existing code > as well. I have two difficulties: I'm not good at C++ and Guile stuff, and it's not completely clear to me what the code wants to achieve.

Re: Fix #5964: MM Rests shouldn’t segfault when there’s no StaffSymbol. (issue 576090043 by v.villen...@gmail.com)

On 2020/05/09 18:30:04, Dan Eble wrote: > Yes, that's what I was trying to explain. > internal_line_count() is relevant to one branch only. Oh, I see your point now. I thought I could get away with line_count only, but indeed this fails to account for (hypothetical) cases where the line-positions

Re: Clean up and fix glyph contour generation nits. (issue 566080043 by hanw...@gmail.com)

On 2020/05/09 14:23:04, lemzwerg wrote: > I don't fully understand this code. that is an ominous comment to come from the Freetype author. I had assumed you reviewed the existing code as well. > Please add more comments. > > A quite well documented routine to walk over a contour is function >

Re: Clean up and fix glyph contour generation nits. (issue 566080043 by hanw...@gmail.com)

it's separate because it causes formatting changes. I like to keep the bugfixes and performance fixes separate. (Performance fixes should not cause regtest differences) https://codereview.appspot.com/566080043/

Re: Remove deprecated context properties (issue 560030044 by v.villen...@gmail.com)

Not sure about chordNameExceptionsFull and chordNameExceptionsPartial. I thought our deprecating policy would be to wait for 1 or even 2 stable versions before deleting from source. Well, the snippet demonstrates a user-level reimplementation, but this makes little sense to me. Also, banter-style

Re: migrating to GitLab

Am Samstag, den 09.05.2020, 21:13 +0200 schrieb David Kastrup: > Carl Sorensen writes: > > On 5/9/20, 12:13 PM, "lilypond-devel on behalf of Jonas Hahnfeld" > > > hah...@hahnjo.de> wrote: > > > > > > So what's the feeling about the migration? go / no-go for tomorrow? > > > > ->CS Do we have a

Re: migrating to GitLab

On May 9, 2020, at 15:13, David Kastrup wrote: > Carl Sorensen writes: > >> ->CS At any rate, I think that we should have appropriate CG >> instructions at the time we make the switch. They don't have to be >> perfect (the CG has a much lower editing bar than the NR), but they >> need to be in

Re: migrating to GitLab

Carl Sorensen writes: > On 5/9/20, 12:13 PM, "lilypond-devel on behalf of Jonas Hahnfeld" > hah...@hahnjo.de> wrote: > > > So what's the feeling about the migration? go / no-go for tomorrow? > > ->CS Do we have a revision to the CG to go with the migration? I > haven't seen any red flags that

Re: migrating to GitLab

On 5/9/20, 12:13 PM, "lilypond-devel on behalf of Jonas Hahnfeld" wrote: So what's the feeling about the migration? go / no-go for tomorrow? ->CS Do we have a revision to the CG to go with the migration? I haven't seen any red flags that cause me to oppose the migration. I love the idea

Re: Fix #5964: MM Rests shouldn’t segfault when there’s no StaffSymbol. (issue 576090043 by v.villen...@gmail.com)

On 2020/05/09 18:14:40, Valentin Villenave wrote: > I do, however, worry about the comment before (what used to be) the > internal_line_count() definition: > > // Get the line-count property directly. This is for internal use when it is > // known that the line-positions property is not

Remove deprecated context properties (issue 560030044 by v.villen...@gmail.com)

LGTM. This issue also adds some scheme code and a conversion rule – they are different commits, right? https://codereview.appspot.com/560030044/diff/557800043/scm/define-context-properties.scm File scm/define-context-properties.scm (right):

Re: Fix #5964: MM Rests shouldn’t segfault when there’s no StaffSymbol. (issue 576090043 by v.villen...@gmail.com)

On 2020/05/09 12:47:29, Dan Eble wrote: > https://codereview.appspot.com/576090043/diff/569740045/lily/staff-symbol.cc > File lily/staff-symbol.cc (right): > > https://codereview.appspot.com/576090043/diff/569740045/lily/staff-symbol.cc#newcode116 > lily/staff-symbol.cc:116: return

Re: migrating to GitLab

Am Freitag, den 08.05.2020, 19:10 +0100 schrieb James Lowe: > On 08/05/2020 12:21, Jonas Hahnfeld wrote: > > Am Freitag, den 08.05.2020, 13:07 +0200 schrieb David Kastrup: > > > Jonas Hahnfeld writes: > > > > > > > 3) The idea is to have the "main" repository at GitLab, next to the > > > >

Re: Clean up and fix glyph contour generation nits. (issue 566080043 by hanw...@gmail.com)

Why is this separate from https://codereview.appspot.com/569700043 ? https://codereview.appspot.com/566080043/

Re: Stop smobifying Transform (issue 567580043 by hanw...@gmail.com)

On 2020/05/09 16:19:59, hanwenn wrote: > On Sat, May 9, 2020 at 4:56 PM wrote: > > > > On 2020/05/09 12:00:01, hanwenn wrote: > > > On 2020/05/09 11:03:12, dak wrote: > > > > Rationale? This negates work done as part of issue 5347 with the > > long-term > > > goal > > > > of

Re: Stop smobifying Transform (issue 567580043 by hanw...@gmail.com)

On Sat, May 9, 2020 at 4:56 PM wrote: > > On 2020/05/09 12:00:01, hanwenn wrote: > > On 2020/05/09 11:03:12, dak wrote: > > > Rationale? This negates work done as part of issue 5347 with the > long-term > > goal > > > of making transforms a better integrated and accessible part of > stencils. >

Re: Stop smobifying Transform (issue 567580043 by hanw...@gmail.com)

On 2020/05/09 12:00:01, hanwenn wrote: > On 2020/05/09 11:03:12, dak wrote: > > Rationale? This negates work done as part of issue 5347 with the long-term > goal > > of making transforms a better integrated and accessible part of stencils. > This > > will be increasingly important when we move

Re: Clean up and fix glyph contour generation nits. (issue 566080043 by hanw...@gmail.com)

I don't fully understand this code. Please add more comments. A quite well documented routine to walk over a contour is function `FT_Outline_Decompose` in file `src/base/ftoutln.c` of the FreeType source code. https://codereview.appspot.com/566080043/diff/560030043/lily/freetype.cc File

Re: Prevent race condition in `-dfont-ps-resdir` (issue 561810045 by truer...@gmail.com)

Thank you for your reviewing. https://codereview.appspot.com/561810045/diff/551840043/scm/backend-library.scm File scm/backend-library.scm (right): https://codereview.appspot.com/561810045/diff/551840043/scm/backend-library.scm#newcode143 scm/backend-library.scm:143: ;; When the directory

Make Scheme_hash_table just use the native hash table type. This (issue 561810043 by d...@gnu.org)

LGTM. Thanks. https://codereview.appspot.com/561810043/

Re: Issue 4182: avoid checking the offset of cross-staff stems too early (issue 554030043 by barr...@gmail.com)

https://codereview.appspot.com/554030043/diff/583870043/lily/note-head.cc File lily/note-head.cc (right): https://codereview.appspot.com/554030043/diff/583870043/lily/note-head.cc#newcode114 lily/note-head.cc:114: if (stem and ! ly_scm2bool (get_property (stem, "cross-staff"))) Please use &&

Prevent race condition in `-dfont-ps-resdir` (issue 561810045 by truer...@gmail.com)

LGTM, with minor nits. https://codereview.appspot.com/561810045/diff/551840043/scm/backend-library.scm File scm/backend-library.scm (right): https://codereview.appspot.com/561810045/diff/551840043/scm/backend-library.scm#newcode143 scm/backend-library.scm:143: ;; When the directory already

Re: output-distance snafu

> I sped up output-distance, but broke quality of the graphics > rendering. This is fixed in > https://codereview.appspot.com/560020043/diff/561820043/scripts/build/output-distance.py. > > This problem makes the regtests hard to use, so I want to skip > countdown for this change. +1

Re: Fix #5964: MM Rests shouldn’t segfault when there’s no StaffSymbol. (issue 576090043 by v.villen...@gmail.com)

https://codereview.appspot.com/576090043/diff/569740045/lily/staff-symbol.cc File lily/staff-symbol.cc (right): https://codereview.appspot.com/576090043/diff/569740045/lily/staff-symbol.cc#newcode116 lily/staff-symbol.cc:116: return ly_scm2floatvector (line_positions); The suggested patch to

Re: Stop smobifying Transform (issue 567580043 by hanw...@gmail.com)

Reviewers: dak, Message: On 2020/05/09 11:03:12, dak wrote: > Rationale? This negates work done as part of issue 5347 with the long-term goal > of making transforms a better integrated and accessible part of stencils. This > will be increasingly important when we move to Cairo since Cairo

Stop smobifying Transform (issue 567580043 by hanw...@gmail.com)

Rationale? This negates work done as part of issue 5347 with the long-term goal of making transforms a better integrated and accessible part of stencils. This will be increasingly important when we move to Cairo since Cairo cannot represent rotations by multiples of 90 degrees cleanly other than

Re: Fix #5964: MM Rests shouldn’t segfault when there’s no StaffSymbol. (issue 576090043 by v.villen...@gmail.com)

On 2020/05/09 01:42:54, Dan Eble wrote: > I looked around. This is the only place I found using just the size of the > vector. I don't think it's worth adding and maintaining a > Staff_symbol::line_count just for this case. Well, it wouldn’t be "adding and maintaining" since it already exists

Re: Shortcut Rational addition if either operand is zero (issue 551690046 by hanw...@gmail.com)

commit fb98d6be7ed06969a8eeab7b974e8734d8cd21b1 Author: Han-Wen Nienhuys Date: Sun Apr 12 13:32:01 2020 +0200 Shortcut Rational addition if either operand is zero https://codereview.appspot.com/551690046/

Re: Use binary search in Axis_group_interface::combine_pure_heights. (issue 573730043 by hanw...@gmail.com)

commit 7d4ddb25cec526d9785aa041bdb1d499d3209333 Author: Han-Wen Nienhuys Date: Mon Apr 20 10:31:30 2020 +0200 Use binary search in Axis_group_interface::combine_pure_heights. https://codereview.appspot.com/573730043/

Clean up and fix glyph contour generation nits. (issue 566080043 by hanw...@gmail.com)

Reviewers: lemzwerg, Message: Werner, can you confirm that this looks correct? I'm especially not certain about the wrapping. Description: Clean up and fix glyph contour generation nits. There are two bugs: * Glyphs with multiple contours (e.g. "O" or "i") should compute the contours

output-distance snafu

I sped up output-distance, but broke quality of the graphics rendering. This is fixed in https://codereview.appspot.com/560020043/diff/561820043/scripts/build/output-distance.py. This problem makes the regtests hard to use, so I want to skip countdown for this change. -- Han-Wen Nienhuys -

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

https://codereview.appspot.com/548030043/diff/559960069/scm/ps-to-png.scm File scm/ps-to-png.scm (right): https://codereview.appspot.com/548030043/diff/559960069/scm/ps-to-png.scm#newcode184 scm/ps-to-png.scm:184: (if (not (= 1 anti-alias-factor)) please submit this as is, but for a follow-up,

PATCHES - Countdown for May 9th

Hello, Here is the current patch countdown list. The next countdown will be on May 11th. A quick synopsis of all patches currently in the review process can be found here: http://philholmes.net/lilypond/allura/ *** Push: 5963 Remove define causing a warning - Jonas Hahnfeld