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

2020-04-28 Thread Kevin Barry
Hi Jonas,

> Done (hopefully).

Thank you very much; it looks like it worked!

Kevin



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

2020-04-28 Thread Jonas Hahnfeld
Am Dienstag, den 28.04.2020, 18:31 +0100 schrieb Kevin Barry:
> Hi James,
> 
> > We do have a process, see: 
> > http://lilypond.org/doc/v2.19/Documentation/contributor-big-page#summary-for-experienced-developers
> > 
> 
> I have followed this as best I can, but it seems I need to have write
> access to the issue tracker before I can send patches. If that is the
> case then can someone please arrange that for me? My username is
> barrykp.

Done (hopefully).

Jonas


signature.asc
Description: This is a digitally signed message part


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

2020-04-28 Thread Kevin Barry
Hi James,

> We do have a process, see: 
> http://lilypond.org/doc/v2.19/Documentation/contributor-big-page#summary-for-experienced-developers

I have followed this as best I can, but it seems I need to have write
access to the issue tracker before I can send patches. If that is the
case then can someone please arrange that for me? My username is
barrykp.

Kevin



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

2020-04-28 Thread James

Hello Kevin,

On 27/04/2020 18:07, Kevin Barry wrote:

Hi James,

Thank you for responding.

On Mon, Apr 27, 2020 at 11:09:50AM +0100, James wrote:

Is this patch read for full testing or is it work-in-progress you just want
someone to comment on it?

It's not work in progress no - unless someone has objections or
observations, but I don't mean to circumvent the review process or
anything. If there's a better way to go about this then I'm happy to
follow a process.

Kevin


We do have a process, see: 
http://lilypond.org/doc/v2.19/Documentation/contributor-big-page#summary-for-experienced-developers


this will get you going.

James




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

2020-04-27 Thread Kevin Barry
Hi James,

Thank you for responding.

On Mon, Apr 27, 2020 at 11:09:50AM +0100, James wrote:
> Is this patch read for full testing or is it work-in-progress you just want
> someone to comment on it?

It's not work in progress no - unless someone has objections or
observations, but I don't mean to circumvent the review process or
anything. If there's a better way to go about this then I'm happy to
follow a process.

Kevin



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

2020-04-27 Thread James

Kevin,

Is this patch read for full testing or is it work-in-progress you just 
want someone to comment on it?


James

On 25/04/2020 08:42, Kevin Barry wrote:

Patch is attached to this mail as a file if that is more convenient.

Kevin




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

2020-04-25 Thread Kevin Barry
Patch is attached to this mail as a file if that is more convenient.

Kevin
>From 1c5715ad52139aab936ee7dffb2cdef3d123b369 Mon Sep 17 00:00:00 2001
From: Kevin Barry 
Date: Fri, 24 Apr 2020 19:26:26 +0100
Subject: [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 a skyline determined only from the non-utf-8-string
stencils. This sometimes causes the text portion of such mixed grobs
(e.g.  metronome marks) to collide with other grobs.

While looping over the stencils, check for utf-8-string and if found,
clear the skylines and break out of the loop. Empty skylines forces a
fallback to the grob's bounding box, which restores the behaviour from
before the patch to improve skyline approximations (issue 2148). This
does not fix the issue that there is no routine for determining skylines
for utf-8-strings when the backend is svg, but it does at least remove
the collisions without changing the behaviour in non-broken situations.

Add a suitable (svg backend) regression test.
---
 input/regression/svg-metronome-mark-skylines.ly | 15 +++
 lily/stencil-integral.cc| 10 +-
 2 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 input/regression/svg-metronome-mark-skylines.ly

diff --git a/input/regression/svg-metronome-mark-skylines.ly 
b/input/regression/svg-metronome-mark-skylines.ly
new file mode 100644
index 00..844e4a8081
--- /dev/null
+++ b/input/regression/svg-metronome-mark-skylines.ly
@@ -0,0 +1,15 @@
+\header {
+texidoc = "In svg output, a @code{MetronomeMark} or any grob mixing text
+and other glyphs does not collide with other grobs.
+"
+}
+
+\version "2.21.1"
+
+#(ly:set-option 'backend 'svg)
+
+{
+  \tempo "Allegro" 4 = 120
+  r1
+  r4 f''' e''' b''
+}
diff --git a/lily/stencil-integral.cc b/lily/stencil-integral.cc
index 7b12eaf17b..8ac386a10e 100644
--- a/lily/stencil-integral.cc
+++ b/lily/stencil-integral.cc
@@ -1097,7 +1097,15 @@ Stencil::skylines_from_stencil (SCM sten, Real pad, SCM 
rot, Axis a)
   vector boxes;
   vector > buildings;
   for (SCM s = scm_reverse_x (data, SCM_EOL); scm_is_pair (s); s = scm_cdr (s))
-stencil_dispatcher (boxes, buildings, scm_caar (s), scm_cdar (s));
+{
+  // If any stencils are utf-8-string, fall back on the bounding box
+  if (scm_is_eq (scm_cadar (s), ly_symbol2scm ("utf-8-string")))
+{
+  boxes.clear();
+  break;
+}
+  stencil_dispatcher (boxes, buildings, scm_caar (s), scm_cdar (s));
+}
 
   // we use the bounding box if there are no boxes
   // FIXME: Rotation?
-- 
2.25.3



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

2020-04-24 Thread Kevin Barry
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 a skyline determined only from the non-utf-8-string
stencils. This sometimes causes the text portion of such mixed grobs
(e.g.  metronome marks) to collide with other grobs.

While looping over the stencils, check for utf-8-string and if found,
clear the skylines and break out of the loop. Empty skylines forces a
fallback to the grob's bounding box, which restores the behaviour from
before the patch to improve skyline approximations (issue 2148). This
does not fix the issue that there is no routine for determining skylines
for utf-8-strings when the backend is svg, but it does at least remove
the collisions without changing the behaviour in non-broken situations.

Add a suitable (svg backend) regression test.
---
 input/regression/svg-metronome-mark-skylines.ly | 15 +++
 lily/stencil-integral.cc| 10 +-
 2 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 input/regression/svg-metronome-mark-skylines.ly

diff --git a/input/regression/svg-metronome-mark-skylines.ly 
b/input/regression/svg-metronome-mark-skylines.ly
new file mode 100644
index 00..844e4a8081
--- /dev/null
+++ b/input/regression/svg-metronome-mark-skylines.ly
@@ -0,0 +1,15 @@
+\header {
+texidoc = "In svg output, a @code{MetronomeMark} or any grob mixing text
+and other glyphs does not collide with other grobs.
+"
+}
+
+\version "2.21.1"
+
+#(ly:set-option 'backend 'svg)
+
+{
+  \tempo "Allegro" 4 = 120
+  r1
+  r4 f''' e''' b''
+}
diff --git a/lily/stencil-integral.cc b/lily/stencil-integral.cc
index 7b12eaf17b..8ac386a10e 100644
--- a/lily/stencil-integral.cc
+++ b/lily/stencil-integral.cc
@@ -1097,7 +1097,15 @@ Stencil::skylines_from_stencil (SCM sten, Real pad, SCM 
rot, Axis a)
   vector boxes;
   vector > buildings;
   for (SCM s = scm_reverse_x (data, SCM_EOL); scm_is_pair (s); s = scm_cdr (s))
-stencil_dispatcher (boxes, buildings, scm_caar (s), scm_cdar (s));
+{
+  // If any stencils are utf-8-string, fall back on the bounding box
+  if (scm_is_eq (scm_cadar (s), ly_symbol2scm ("utf-8-string")))
+{
+  boxes.clear();
+  break;
+}
+  stencil_dispatcher (boxes, buildings, scm_caar (s), scm_cdar (s));
+}
 
   // we use the bounding box if there are no boxes
   // FIXME: Rotation?
-- 
2.25.3