Re: Rewrite Skyline code (issue 547980044 by hanw...@gmail.com)

On 2020/04/19 22:35:39, dak wrote: > On 2020/04/19 21:43:26, hanwenn wrote: > > (this still needs some work, but the speedup might be worth an early look) > > * Buildings store Y coordinate of the left edge, rather than the > intercept at x==0.0. This avoid excessive rounding problems when X >

Re: Minor cleanups in stencil-integral.cc (issue 579630043 by hanw...@gmail.com)

On 2020/04/24 21:33:12, Carl wrote: > On 2020/04/24 21:19:57, dak wrote: > > On 2020/04/24 21:18:12, dak wrote: > > > > > > https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc > > > File lily/stencil-integral.cc (right): > > > > > > > > >

Re: Minor cleanups in stencil-integral.cc (issue 579630043 by hanw...@gmail.com)

On 2020/04/24 21:19:57, dak wrote: > On 2020/04/24 21:18:12, dak wrote: > > > https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc > > File lily/stencil-integral.cc (right): > > > > >

Re: Minor cleanups in stencil-integral.cc (issue 579630043 by hanw...@gmail.com)

https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc File lily/stencil-integral.cc (right): https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc#newcode465 lily/stencil-integral.cc:465: // more convoluted, but it's fairly hot path. Sorry

Re: Minor cleanups in stencil-integral.cc (issue 579630043 by hanw...@gmail.com)

On 2020/04/24 21:18:12, dak wrote: > https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc > File lily/stencil-integral.cc (right): > > https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc#newcode465 > lily/stencil-integral.cc:465: // more

[PATCH v1] Issue 3778: Use bounding box as skylines for markup in svg backend

As there is no routine for determining skylines for utf-8-string stencils, they normally fall back to the grob's bounding box, which is fine. However, when there is a mixture of utf-8-string and other types of stencil (which have associated skyline functions) in a single grob, the entire grob gets

Re: make_draw_bezier_boxes: save work if thickness == 0.0 (issue 551730043 by hanw...@gmail.com)

On Fri, Apr 24, 2020 at 8:56 PM wrote: > > > https://codereview.appspot.com/551730043/diff/583770043/lily/stencil-integral.cc > File lily/stencil-integral.cc (right): > > https://codereview.appspot.com/551730043/diff/583770043/lily/stencil-integral.cc#newcode496 > lily/stencil-integral.cc:496:

Re: [PATCH] Issue 3778: Use bounding box as skylines for markup in svg backend

On Fri, Apr 24, 2020 at 09:25:19PM +0200, Han-Wen Nienhuys wrote: > Just add a test file under input/regression/ , following the style of > other files there. Thanks for the pointer. The only other svg backend file there looks like it has been deliberately disabled, so apologies if I have done

Re: Fix warnings related to po (issue 551830043 by jonas.hahnf...@gmail.com)

Reviewers: lemzwerg, Message: On 2020/04/24 18:25:33, lemzwerg wrote: > LGTM, thanks a lot! > > https://codereview.appspot.com/551830043/diff/573740043/po/GNUmakefile > File po/GNUmakefile (right): > > https://codereview.appspot.com/551830043/diff/573740043/po/GNUmakefile#newcode14 >

Re: [PATCH] Issue 3778: Use bounding box as skylines for markup in svg backend

On Fri, Apr 24, 2020 at 09:18:10PM +0200, Han-Wen Nienhuys wrote: > Can you add a regression test that shows the problem? Yes. I'll go read the docs to see how to do that and do one up (but feel free to point me in the right direction...) Kevin

Re: [PATCH] Issue 3778: Use bounding box as skylines for markup in svg backend

On Fri, Apr 24, 2020 at 9:21 PM Kevin Barry wrote: > > On Fri, Apr 24, 2020 at 09:18:10PM +0200, Han-Wen Nienhuys wrote: > > Can you add a regression test that shows the problem? > > Yes. I'll go read the docs to see how to do that and do one up (but feel > free to point me in the right

Re: \mask and \unmaskWithTag

On Apr 23, 2020, at 19:25, Dan Eble wrote: > >(let ((masked-music (ly:music-property m 'void))) > (if (and (ly:music? masked-music) (tags-keep-predicate tags)) ^^ Oops. This is bogus and the output of the score is correct in

Re: [PATCH] Issue 3778: Use bounding box as skylines for markup in svg backend

Can you add a regression test that shows the problem? On Fri, Apr 24, 2020 at 8:50 PM Kevin Barry wrote: > > As there is no routine for determining skylines for utf-8-string > stencils, they normally fall back to the grob's bounding box, which is > fine. However, when there is a mixture of

Re: make_draw_bezier_boxes: save work if thickness == 0.0 (issue 551730043 by hanw...@gmail.com)

https://codereview.appspot.com/551730043/diff/583770043/lily/stencil-integral.cc File lily/stencil-integral.cc (right): https://codereview.appspot.com/551730043/diff/583770043/lily/stencil-integral.cc#newcode496 lily/stencil-integral.cc:496: break; I think that is less readable than desirable.

Re: Rewrite Skyline code (issue 547980044 by hanw...@gmail.com)

https://codereview.appspot.com/547980044/diff/567490043/lily/include/skyline.hh File lily/include/skyline.hh (right): https://codereview.appspot.com/547980044/diff/567490043/lily/include/skyline.hh#newcode152 lily/include/skyline.hh:152: } On 2020/04/24 17:12:48, hanwenn wrote: > On 2020/04/24

Re: Extracting approximate outlines from FT ?

On Fri, Apr 24, 2020 at 5:54 PM Han-Wen Nienhuys wrote: > > On Fri, Apr 17, 2020 at 7:56 PM Werner LEMBERG wrote: > > > I've been looking into performance, and one big source of slowness > > > is calculating skylines. I found that we calculate exact skylines > > > based on glyph shapes for some

[PATCH] Issue 3778: Use bounding box as skylines for markup in svg backend

As there is no routine for determining skylines for utf-8-string stencils, they normally fall back to the grob's bounding box, which is fine. However, when there is a mixture of utf-8-string and other types of stencil (which have associated skyline functions) in a single grob, the entire grob gets

Re: make_draw_bezier_boxes: save work if thickness == 0.0 (issue 551730043 by hanw...@gmail.com)

Reviewers: Dan Eble, Message: The percentages between different have looked fairly repeatable, but as a less invasive measure is to just tweak the final 'if' Description: make_draw_bezier_boxes: save work if thickness == 0.0 This drop make_draw_bezier_boxes from 0.73% to 0.44% in the profile

make_draw_bezier_boxes: save work if thickness == 0.0 (issue 551730043 by hanw...@gmail.com)

I see I'm late to the party with this comment, but what are the error bars on that 0.29%? I'm not sure the decreased readability is worth it. https://codereview.appspot.com/551730043/

Fix warnings related to po (issue 551830043 by jonas.hahnf...@gmail.com)

LGTM, thanks a lot! https://codereview.appspot.com/551830043/diff/573740043/po/GNUmakefile File po/GNUmakefile (right): https://codereview.appspot.com/551830043/diff/573740043/po/GNUmakefile#newcode14 po/GNUmakefile:14: sed-makestuff = -e 's/[a-zA-Z_/]*make\[[0-9]*\].*//' shouldn't this be

Re: Rewrite Skyline code (issue 547980044 by hanw...@gmail.com)

On 2020/04/24 17:12:48, hanwenn wrote: > https://codereview.appspot.com/547980044/diff/567490043/lily/include/skyline.hh > File lily/include/skyline.hh (right): > > https://codereview.appspot.com/547980044/diff/567490043/lily/include/skyline.hh#newcode152 > lily/include/skyline.hh:152: } > On

Re: Rewrite Skyline code (issue 547980044 by hanw...@gmail.com)

https://codereview.appspot.com/547980044/diff/567490043/lily/include/skyline.hh File lily/include/skyline.hh (right): https://codereview.appspot.com/547980044/diff/567490043/lily/include/skyline.hh#newcode152 lily/include/skyline.hh:152: } On 2020/04/24 16:33:04, hahnjo wrote: > Why do you need

Re: Rewrite Skyline code (issue 547980044 by hanw...@gmail.com)

https://codereview.appspot.com/547980044/diff/567490043/lily/include/skyline.hh File lily/include/skyline.hh (right): https://codereview.appspot.com/547980044/diff/567490043/lily/include/skyline.hh#newcode152 lily/include/skyline.hh:152: } Why do you need all of this in the header file? As far

Re: Extracting approximate outlines from FT ?

On Fri, Apr 17, 2020 at 7:56 PM Werner LEMBERG wrote: > > I've been looking into performance, and one big source of slowness > > is calculating skylines. I found that we calculate exact skylines > > based on glyph shapes for some symbols. > > > > For example, for the F clef, we compute an outline

Re: Annoying 'langdefs.py' warning

Am Sonntag, den 19.04.2020, 17:22 +0200 schrieb Werner LEMBERG: > >> Is there a way to suppress them? > > > > Well, submitting fixes? > > I looked into both issues some time ago, but couldn't find out a good > solution. Thanks to your clean-ups I can imagine that it's now easier > to handle

Re: Optimize get_path_list. (issue 579570043 by hanw...@gmail.com)

On 2020/04/13 19:11:22, hanwenn wrote: > https://codereview.appspot.com/579570043/diff/577750045/lily/stencil-integral.cc > File lily/stencil-integral.cc (right): > > https://codereview.appspot.com/579570043/diff/577750045/lily/stencil-integral.cc#newcode115 > lily/stencil-integral.cc:115: &&

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

Han-Wen Nienhuys writes: > On Fri, Apr 24, 2020 at 3:15 PM David Kastrup wrote: >> > >> > If you want me to use "auto" instead of >> > "vector::const_iterator", can you just point to the line in >> > the code and say "could you use auto instead of an iterator here"? >> >> Wouldn't "everywhere"

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

On Fri, Apr 24, 2020 at 3:15 PM David Kastrup wrote: > > > If you want me to use "auto" instead of > > "vector::const_iterator", can you just point to the line in > > the code and say "could you use auto instead of an iterator here"? > > Wouldn't "everywhere" be sort of obvious? yes, it would.

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

On 2020/04/24 13:09:53, hanwenn wrote: > On Fri, Apr 24, 2020 at 3:04 PM wrote: > > > > Han-Wen, any reason not to use range-based loops in the places I pointed > > to in the last patch set? > > I'm rewriting a lot of this code anyway in >

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

hanw...@gmail.com writes: > Here is how I have experienced this discussion: > > DAK: this is micro-optimization that causes memory fragmentation. > > HW: No, it's not; here is a benchmark that shows decreased memory use as > well. > > DAK: We could use even different data structures in the

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

On Fri, Apr 24, 2020 at 3:04 PM wrote: > > Han-Wen, any reason not to use range-based loops in the places I pointed > to in the last patch set? I'm rewriting a lot of this code anyway in https://codereview.appspot.com/547980044/ so changing it to range loops involves more wasted editing. I can

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

Han-Wen, any reason not to use range-based loops in the places I pointed to in the last patch set? https://codereview.appspot.com/583750043/diff/545930043/lily/skyline.cc File lily/skyline.cc (right): https://codereview.appspot.com/583750043/diff/545930043/lily/skyline.cc#newcode184

Re: Use vectors rather than lists for skylines. (issue 572040043 by hanw...@gmail.com)

mixup, sorry. https://codereview.appspot.com/572040043/

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

https://codereview.appspot.com/583750043/diff/577780043/lily/skyline.cc File lily/skyline.cc (right): https://codereview.appspot.com/583750043/diff/577780043/lily/skyline.cc#newcode183 lily/skyline.cc:183: vector::iterator i; On 2020/04/24 07:08:50, hahnjo wrote: > move into for loop and make

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

Here is how I have experienced this discussion: DAK: this is micro-optimization that causes memory fragmentation. HW: No, it's not; here is a benchmark that shows decreased memory use as well. DAK: We could use even different data structures in the future. HW: This seems very philosophical

Re: Verifying fixes?

On 23/04/2020 18:32, Valentin Villenave wrote: On Thursday, April 23, 2020, Carl Sorensen wrote:. Do you believe we should Verify the fixes, or should we just leave the issues closed without verification? I'm willing to spend some time doing the verification, if we think it's valuable to do

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

On 2020/04/24 08:15:09, Valentin Villenave wrote: > On 2020/04/24 07:20:55, hahnjo wrote: > > If there are no technical objections, I think we should move > > forward and let something as big sit around for too long. > > Just for clarification, did you miss a "not" in that sentence? Ehm yes.

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

On 2020/04/24 07:20:55, hahnjo wrote: > If there are no technical objections, I think we should move > forward and let something as big sit around for too long. Just for clarification, did you miss a "not" in that sentence? Cheers, - V. https://codereview.appspot.com/573670043/

Re: Make doc currently broken

On 4/23/20, Federico Bruni wrote: > I confirm it's a stale dependency file. > I had the same problem while testing my patch before pushing and IIRC > removing Documentation/it/out-www/web.texi fixed it. OK, thanks! I should have thought about doc-clean. Cheers, - V.

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

So I've largely kept out of the discussion for now, mostly because I'm not overly familiar with the code. However I fully agree with Dan that a macro pretending to be a member function is neither obvious nor C++ style. As such I'm in favor of doing the conversion as it better meets the

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

I generally agree with David that having the code agnostic of the container would be an improvement. This doesn't need to be a typedef yet as reserve() is unique to std::vector (at least not in std::list) and makes sense in the places it is used here. A rather obvious change (that I'm very