Re: Fix dashed line errors (issue 320320043 by david.nales...@gmail.com)

2017-03-07 Thread lemzwerg

LGTM.

https://codereview.appspot.com/320320043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Fix dashed line errors (issue 320320043 by david.nales...@gmail.com)

2017-03-07 Thread Urs Liska
I currently can't compile so I can't test the patch, and I can only
assess (1) from reading the code.

But it's good to fix this. It's a glitch but nevertheless "feels"
awkward when tweaking a score.
Urs


Am 07.03.2017 um 05:46 schrieb david.nales...@gmail.com:
> Reviewers: ,
>
> Message:
> Please review.  Thanks!
>
> Description:
> Fix dashed line errors
>
> There are two errors in lily/line-interface.cc which cause
> dashed lines to be drawn inaccurately.
>
> (1) Line-thickness is improperly subtracted from the "off"
> value in Line_interface::make_dashed_line.  This was done,
> presumably, to account for the line cap of each dash.  In fact,
> PostScript discounts the line cap when constructing the pattern.
>
> (2) Dashed lines are constructed so that they begin and end
> with a dash.  Thus, a dashed line is built of dash + whitespace
> units and a last lone dash.  Period is not adjusted in
> Line_interface::line for this last dash, which can lead to a
> noticeable difference in its length compared to other dashes.
>
> Correcting these flaws ensures that dashed lines end with a
> dash rather than whitespace, and that terminating dashes are
> not clipped.  Also, the number of dashes drawn reflects the
> number calculated.
>
> Please review this at https://codereview.appspot.com/320320043/
>
> Affected files (+13, -8 lines):
>   M lily/line-interface.cc
>
>
> Index: lily/line-interface.cc
> diff --git a/lily/line-interface.cc b/lily/line-interface.cc
> index
> bc0895339fea3c996e08a71891c0090cbea2e96c..9c16ece4106f956af5cb3a30cb4fc667065abab8
> 100644
> --- a/lily/line-interface.cc
> +++ b/lily/line-interface.cc
> @@ -123,8 +123,8 @@ Line_interface::make_dashed_line (Real thick,
> Offset from, Offset to,
>Real dash_period, Real dash_fraction)
>  {
>dash_fraction = min (max (dash_fraction, 0.0), 1.0);
> -  Real on = dash_fraction * dash_period;
> -  Real off = max (0.0, dash_period - on - thick);
> +  Real on = dash_fraction * dash_period;
> +  Real off = max (0.0, dash_period - on);
>
>SCM at = scm_list_n (ly_symbol2scm ("dashed-line"),
> scm_from_double (thick),
> @@ -226,16 +226,21 @@ Line_interface::line (Grob *me, Offset from,
> Offset to)
>  return Stencil ();
>
>Real len = (to - from).length ();
> -
> -  int n = (int) rint ((len - period * fraction) / period);
> -  n = max (0, n);
> -  if (n > 0)
> +  /*
> +Dashed lines should begin and end with a dash.  Therefore,
> +there will be one more dash than complete dash + whitespace
> +units (full periods).
> +  */
> +  int full_period_count =
> +(int) rint ((len - period * fraction) / period);
> +  full_period_count = max (0, full_period_count);
> +  if (full_period_count > 0)
>  {
>/*
>  TODO: figure out something intelligent for really short
>  sections.
> -   */
> -  period = ((to - from).length () - period * fraction) / n;
> +  */
> +  period = len / (fraction + full_period_count);
>  }
>stencil = make_dashed_line (thick, from, to, period, fraction);
>  }
>
>
>
> ___
> lilypond-devel mailing list
> lilypond-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/lilypond-devel

-- 
u...@openlilylib.org
https://openlilylib.org
http://lilypondblog.org


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Fix dashed line errors (issue 320320043 by david.nales...@gmail.com)

2017-03-06 Thread david . nalesnik

Reviewers: ,

Message:
Please review.  Thanks!

Description:
Fix dashed line errors

There are two errors in lily/line-interface.cc which cause
dashed lines to be drawn inaccurately.

(1) Line-thickness is improperly subtracted from the "off"
value in Line_interface::make_dashed_line.  This was done,
presumably, to account for the line cap of each dash.  In fact,
PostScript discounts the line cap when constructing the pattern.

(2) Dashed lines are constructed so that they begin and end
with a dash.  Thus, a dashed line is built of dash + whitespace
units and a last lone dash.  Period is not adjusted in
Line_interface::line for this last dash, which can lead to a
noticeable difference in its length compared to other dashes.

Correcting these flaws ensures that dashed lines end with a
dash rather than whitespace, and that terminating dashes are
not clipped.  Also, the number of dashes drawn reflects the
number calculated.

Please review this at https://codereview.appspot.com/320320043/

Affected files (+13, -8 lines):
  M lily/line-interface.cc


Index: lily/line-interface.cc
diff --git a/lily/line-interface.cc b/lily/line-interface.cc
index  
bc0895339fea3c996e08a71891c0090cbea2e96c..9c16ece4106f956af5cb3a30cb4fc667065abab8  
100644

--- a/lily/line-interface.cc
+++ b/lily/line-interface.cc
@@ -123,8 +123,8 @@ Line_interface::make_dashed_line (Real thick, Offset  
from, Offset to,

   Real dash_period, Real dash_fraction)
 {
   dash_fraction = min (max (dash_fraction, 0.0), 1.0);
-  Real on = dash_fraction * dash_period;
-  Real off = max (0.0, dash_period - on - thick);
+  Real on = dash_fraction * dash_period;
+  Real off = max (0.0, dash_period - on);

   SCM at = scm_list_n (ly_symbol2scm ("dashed-line"),
scm_from_double (thick),
@@ -226,16 +226,21 @@ Line_interface::line (Grob *me, Offset from, Offset  
to)

 return Stencil ();

   Real len = (to - from).length ();
-
-  int n = (int) rint ((len - period * fraction) / period);
-  n = max (0, n);
-  if (n > 0)
+  /*
+Dashed lines should begin and end with a dash.  Therefore,
+there will be one more dash than complete dash + whitespace
+units (full periods).
+  */
+  int full_period_count =
+(int) rint ((len - period * fraction) / period);
+  full_period_count = max (0, full_period_count);
+  if (full_period_count > 0)
 {
   /*
 TODO: figure out something intelligent for really short
 sections.
-   */
-  period = ((to - from).length () - period * fraction) / n;
+  */
+  period = len / (fraction + full_period_count);
 }
   stencil = make_dashed_line (thick, from, to, period, fraction);
 }



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel