LilyIssues / Issues / #4355 Subdivision of beams is wrong

#4355 Subdivision of beams is wrong

Status: Verified

Owner: nobody

Labels: Fixed_2_19_20 (19)

Needs:

Patch:

Type: Defect

Updated: 2015-05-28

Created: 2015-04-24

Creator: Anonymous

Private: No

Originally created by: *anonymous

Originally created by: Carl.D.S...@gmail.com
Originally owned by: Carl.D.S...@gmail.com

The beam subdivision code in lilypond currently does the wrong thing. Subdivided groups of beams are always connected by a single beam, when they should be connected by the appropriate number of beams to represent the length of the subgroup.

In the attached file, the groups should be connected by two beams, rather than by one beam.

1 Attachments

BadSubdivision.png

Discussion

Google Importer - 2015-04-24

Originally posted by: Carl.D.S...@gmail.com

(No comment was entered for this change.)

Labels: Type-Defect

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-04-24

Originally posted by: tdanielsmusic

See also Issue 2361

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-04-24

Originally posted by: Carl.D.S...@gmail.com

Fix issue 4355 -- broken beam subdivision

When beams are subdivided, the number of beams joining the subdivisions
should match the length of the subdivision. Previously, the subdivisions
were joined by a single beam.

http://codereview.appspot.com/226700043

Labels: Patch-new
Owner: Carl.D.S...@gmail.com
Status: Started

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-04-25

Originally posted by: pkx1...@gmail.com

Patchy the autobot says: passes tests. Includes a full make doc

Labels: -Patch-new Patch-review

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-04-25

Originally posted by: Carl.D.S...@gmail.com

Respond to comments. Update regtest and really fix the beaming.

http://codereview.appspot.com/226700043

Labels: -Patch-review Patch-new

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-04-25

Originally posted by: Carl.D.S...@gmail.com

Issue 2361 has been merged into this issue.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-04-26

Originally posted by: pkx1...@gmail.com

Patchy the autobot says: passes tests. Includes a full make doc

Labels: -Patch-new Patch-review

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-04-30

Originally posted by: pkx1...@gmail.com

Patch on countdown for May 3rd

Labels: -Patch-review Patch-countdown

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-05-03

Originally posted by: pkx1...@gmail.com

Patch counted down - please push

Labels: -Patch-countdown Patch-push

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-05-03

Originally posted by: Carl.D.S...@gmail.com

Pushed to staging as 7456fb7..8fa2d85

Status: Fixed

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-05-03

Originally posted by: tdanielsmusic

(No comment was entered for this change.)

Labels: -Patch-push Fixed_2_19_20

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-05-07

Originally posted by: dak@gnu.org

The code seems to be a bit too simplistic as it can only deal with power-of-two meters. As an example,

{
\time 12/16
\set subdivideBeams = ##t
\repeat unfold 24 c''32
}

is changed for the worse.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-05-07

Originally posted by: dak@gnu.org

I think LilyPond should never subdivide at levels that don't divide baseMoment.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-05-07

Originally posted by: Carl.D.S...@gmail.com

The behavior is clearly not right for non-power-of-two meters. In David's example, we subdivde and join a group of duration 1/8 with a group of duration 1/16. This should not happen.

I will split this issue from issue 2361 again.

I will fix this issue so that the subdivision is correct.

However, the fix will eliminate the ability to have multiple levels of subdivision.

Multiple levels of subdivision will remain an enhancement request.

Labels: -Fixed_2_19_20
Status: Started

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-05-11

Originally posted by: Carl.D.S...@gmail.com

Adjust beam subdivision to work properly on subdividing at baseMoment

http://codereview.appspot.com/226700043

Labels: Patch-new

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-05-11

Originally posted by: pkx1...@gmail.com

Patchy the autobot says: passes tests. includes a full make doc

Labels: -Patch-new Patch-review

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-05-15

Originally posted by: pkx1...@gmail.com

Patch on countdown for May 19th

Labels: -Patch-review Patch-countdown

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-05-18

Originally posted by: pkx1...@gmail.com

Patch counted down - please push

Labels: -Patch-countdown Patch-push

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-05-20

Originally posted by: Carl.D.S...@gmail.com

Fixed with commit 4b78ea6..0382ed8

Labels: -Patch-push Fixed_2_19_21
Status: Fixed

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-05-25

Originally posted by: Carl.D.S...@gmail.com

(No comment was entered for this change.)

Status: Fixed

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2015-05-28

Originally posted by: fedel...@gmail.com

It seems fixed in 2.19.20:
[r8fa2d858e492fabbe966cc6a3a4daa7533dfd193]

Author: Carl Sorensen <carl.d.sorensen@gmail.com> 2015-04-25 05:07:51
Committer: Carl Sorensen <carl.d.sorensen@gmail.com> 2015-05-03 15:56:27
Parent: [r7456fb7f523cfd77e628eab0824c530c5b758ccd] (LSR local import)
Child: [r638ccfde1bf3069ab320beacbb734bed1494f3fa] (Issue 4358: Set DynamicLineSpanner direction for \partcombine.)
Branches: master, remotes/origin/dev/dak/issue4422, remotes/origin/master, remotes/origin/origin/staging, remotes/origin/release/unstable, remotes/origin/staging, remotes/origin/translation
Follows: release/2.19.19-1
Precedes: release/2.19.20-1

    Fix issue 4355 -- broken beam subdivision

    When beams are subdivided, the number of beams joining the subdivisions
    should match the length of the subdivision. Previously, the subdivisions
    were joined by a single beam.

Labels: -Fixed_2_19_21 Fixed_2_19_20
Status: Verified

Related

Issues: ~~#4422~~

If you would like to refer to this comment somewhere else in this project, copy and paste the following link: