Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

141581 jianpu #2957

Closed
wants to merge 1 commit into from
Closed

141581 jianpu #2957

wants to merge 1 commit into from

Conversation

byan61
Copy link

@byan61 byan61 commented Jan 8, 2017

This is initial implementation of Jianpu staff display and adding a linked Jianpu staff from a Standard staff.
The implementation is just a start and is by no means a complete one yet.
Currently display of octave dots is limited to one dot.
Supports for duration augmenting dots, grace notes, spanners, etc., are not there yet.
There are also some TODO items marked in the code.

@lasconic
Copy link
Contributor

lasconic commented Jan 8, 2017

Hi, thank you for the PR. Two things:

  1. I need you to sign the MuseScore CLA in order to pull your code into master https://musescore.org/en/cla

  2. Your PR cannot be merged as it. It has several conflicts with master. You will need to rebase and fix the conflicts.

libmscore/CMakeLists.txt
libmscore/barline.cpp
libmscore/element.h
libmscore/layout.cpp
libmscore/measure.cpp
libmscore/staff.cpp
libmscore/staff.h
libmscore/stafftype.cpp
libmscore/system.cpp

@byan61
Copy link
Author

byan61 commented Jan 8, 2017

Done with #1.
For #2, I merged Jianpu changes with latest from master branch.
But, after the merge, Jianpu barlines and Jianpu beams do not work anymore.
I'm now working to fix these.

@byan61
Copy link
Author

byan61 commented Jan 9, 2017

Merge conflicts have been resolved. It's ready for review.
Reading/writing Jianpu staff from/to the score file is not implemented yet.
So, if you test Jianpu staff display by adding a linked Jianpu staff, do not save your score file. Otherwise your score file may get corrupted.

@lasconic
Copy link
Contributor

lasconic commented Jan 9, 2017

It seems your code cannot be compiled, see https://travis-ci.org/musescore/MuseScore/jobs/190151221#L1926

@byan61
Copy link
Author

byan61 commented Jan 9, 2017

Sorry, some files were left out accidentally.
They have been pushed now.

@lasconic
Copy link
Contributor

lasconic commented Jan 9, 2017

No problem. The CI is running. Next thing will be to squash all your commits together to not pollute the history of the repo. See http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html

@lasconic
Copy link
Contributor

lasconic commented Jan 9, 2017

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Jan 9, 2017

Some mtests are failing, namely tst_midimapping, see https://travis-ci.org/musescore/MuseScore/jobs/190164161#L5105
And I think I know why, see my other comments

@@ -1384,6 +1386,7 @@ void StaffType::initStaffTypes()
StaffType(StaffGroup::PERCUSSION, "perc1Line", QObject::tr("Perc. 1 line"), 1, -4, 1, true, true, false, true, false, true),
StaffType(StaffGroup::PERCUSSION, "perc3Line", QObject::tr("Perc. 3 lines"), 3, 0, 2, true, true, false, true, false, true),
StaffType(StaffGroup::PERCUSSION, "perc5Line", QObject::tr("Perc. 5 lines"), 5, 0, 1, true, true, false, true, false, true),
StaffType(StaffGroup::JIANPU, "jianpu", QObject::tr("Jianpu"), 0, 0, 1, false, true, false, true, false, false),
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz Jan 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is why the mtest fail, here you insert Jianpu (better append it IMHO)

@@ -155,6 +155,7 @@ enum class StaffTypes : signed char {
TAB_6SIMPLE, TAB_6COMMON, TAB_6FULL,
TAB_4SIMPLE, TAB_4COMMON, TAB_4FULL,
TAB_UKULELE, TAB_BALALAJKA, TAB_ITALIAN, TAB_FRENCH,
JIANPU,
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz Jan 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whereas here you append jianpu (IMHO a good choice)

@@ -243,6 +244,11 @@ class StaffType {
static QList<TablatureDurationFont> _durationFonts;
static std::vector<StaffType> _presets;

// Jianpu: configurable properties
// TODO: read them from configuration file.
QFont _jianpuNoteFont = QFont("Times New Roman", 11.0, QFont::Bold); // font used to draw Jianpu notes/rests.
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz Jan 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better use FreeSerif or FreeSans, these are delivered with MuseScore, Times New Roman is not and not available on some platforms

@byan61
Copy link
Author

byan61 commented Jan 10, 2017

Thanks, Jojo.
I made the corrections and pushed the commit.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Jan 10, 2017

Looks better ;-)
Now you'd just need to squash these commits (see @lasconic's hint above) , preferably with a commit title in the form of "fix #141581: implement Jianpu staff display", so the corresponding issue gets marked fixed automagically when this PR gets merged.
Then again that squashing may wait until read/write has been implemented too

@byan61
Copy link
Author

byan61 commented Jan 11, 2017

I'll be working on read/write part before squashing the commits.

@byan61
Copy link
Author

byan61 commented Jan 16, 2017

After adding read/write and saving the score file with Jianpu linked staff, staves that are below Jianpu staff are not displayed at all when the file is opened. Display of Jianpu staff and staves above it are working fine. The layout functions of elements from the staves in question are called but the draw functions do not get called. Verified with debugger that these elements are added to the BspTree by scanElements function. Any idea why draw function of these elements do not get called?

@Jojo-Schmitz
Copy link
Contributor

A rebase is needed. And I guess this PR should get tagged as Work in Progress

@lasconic lasconic added the work in progress not finished work or not addressed review label Feb 1, 2017
@lasconic
Copy link
Contributor

lasconic commented Feb 5, 2017

I quickly try the PR. I created a score for Flute, pressed i, and added a Jianpu linked staff. Then I entered some note in the standard flute staff. And I got this

image

@byan61
Copy link
Author

byan61 commented Feb 6, 2017

Note entry and editing for Jianpu has not been implemented yet, so this will not work.
Only display of Jianpu from existing Standard notation score file is implemented so far.
Try opening an existing score file and adding a linked Jianpu Staff to an existing Standard staff.

@byan61
Copy link
Author

byan61 commented Feb 6, 2017

Arr... I just tried opening an existing score file and adding Jianpu staff and the display of Jianpu staff does not work anymore after merging with latest from the base branch. I'll work to fix that.

@lasconic
Copy link
Contributor

lasconic commented Feb 6, 2017

Sorry for that. It's true that the current master is a bit of a moving target right now. Feel free to ask for help if needed.

@byan61
Copy link
Author

byan61 commented Feb 14, 2017

Sorry that I've been busy at work and at home and have not had much time to focus on this yet. Any help is welcome including pointing out what might be causing the Jianpu display issue.

@byan61
Copy link
Author

byan61 commented Apr 19, 2017

Issue of Jianpu staff display has been fixed now.

@byan61
Copy link
Author

byan61 commented Apr 21, 2017

@lasconic and @Jojo-Schmitz: Can you take a look and merge the PR if it's acceptable? Thanks.

@lasconic
Copy link
Contributor

@byan61 sure, give us some time though, we are trying to release MuseScore 2.1 asap.

@byan61
Copy link
Author

byan61 commented May 19, 2017

I have fixed the remaining compiler warnings and pushed a new commit, hopefully the last one for this PR. Please take a look.

@byan61
Copy link
Author

byan61 commented May 24, 2017

@lasconic and @Jojo-Schmitz: Can you take a look and merge the PR if it's acceptable so that I can move on to the next step? Thanks.

@Jojo-Schmitz
Copy link
Contributor

@byan61: code looks OK to me, I haven't actually tested any of it though. And I can't merge.

@byan61
Copy link
Author

byan61 commented Jun 23, 2017

@lasconic and @Jojo-Schmitz: It's been a month now. Any chance for testing and to get it merged?

@sirsoweird
Copy link

@lasconic @Jojo-Schmitz @byan61 I'd love to help test this, however I had a hard time getting a build going for either Windows or Linux. Could anyone put something together for Windows so a few of us could test and give feedback? Sorry I'm not an expert programmer, but I'll help where I can.

@sirsoweird
Copy link

I finally got my build up and running. Just giving it a few quick tests.
capture

A few thoughts: in the top line, the Jianpu underline went down when the notes went above B
Standard Jianpu would have the under-dots below the under-lines.
Measure 7 shows a dotted half; the dot should not show for halves or longer notes. The length of the note is only indicated by the number of dashes after the note. For the dotted half, it should have two dashes after the note.

@lasconic
Copy link
Contributor

Did you rebase on master and fix the conflicts? if yes, can you open a new PR ?

@sirsoweird
Copy link

No, I didn't rebase with master. I'm still very interested in this project, however I'm not going to have much time to work on it for a while here. Not sure where @byan61 is with this, he doesn't seem to be very active currently. If we want to get back on track with this project, what would the steps be?

@lasconic
Copy link
Contributor

Hi,
First step would be to rebase to current master. Then fix the issues you mentioned, preferably with reference to an authoritative source. The problem with Jianpu is that there are several ways to notate it. If MuseScore includes one way, we will need a source to defend the implementation. Then the implementation needs to be clean up. The use of friend class makes it hard to read.

@dannysalim
Copy link

Hi, I am Danny Salim and am interested to continue this project.

@lasconic
Copy link
Contributor

lasconic commented Apr 6, 2018

See PR #3614 for a rebased version.

@odhot
Copy link

odhot commented Dec 31, 2018

I finally got my build up and running. Just giving it a few quick tests.
capture

A few thoughts: in the top line, the Jianpu underline went down when the notes went above B
Standard Jianpu would have the under-dots below the under-lines.
Measure 7 shows a dotted half; the dot should not show for halves or longer notes. The length of the note is only indicated by the number of dashes after the note. For the dotted half, it should have two dashes after the note.

hi, i have working on your version but for indonesian numbered notation .. where can i contact you sir ?

@odhot
Copy link

odhot commented Dec 31, 2018

ss
here's the sample .. but our sharp and flat is different ..

@Jojo-Schmitz
Copy link
Contributor

@odhot better go to #3614, as that is still open (but does need a rebase meanwhile)

@Buronan13
Copy link

@odhot boleh minta nomor wa atau ig atau nama fb kmu tidak?
Saya juga lagi cari cara membuat not angka di musescore tpi gak ketemu2?

@David-C0pperfield
Copy link

A year later...

@Jojo-Schmitz
Copy link
Contributor

this is still closed and #3614 still open...

@sherlockholmestech
Copy link

smusic.zip
Consider this for jianpu notation only, unlinked.

@odhot
Copy link

odhot commented Apr 16, 2020

smusic.zip
Consider this for jianpu notation only, unlinked.

different ... our numeric notation has different flag position

Screen Shot 2020-04-16 at 09 11 22

@Thethnan
Copy link

@lasconic @Jojo-Schmitz @byan61 I'd love to help test this, however I had a hard time getting a build going for either Windows or Linux. Could anyone put something together for Windows so a few of us could test and give feedback? Sorry I'm not an expert programmer, but I'll help where I can.

How to get this jianpu view note sheet.

@Thethnan
Copy link

I would like to get jianpu staff note to numbered note auto convert, please help me.

@Jojo-Schmitz
Copy link
Contributor

this PR has been closed 2 1/2 years ago and work continued in #3614.
That in turn had been closed meanwhile too though, but if anything that'd be the one to continue on.

@oorfeo
Copy link

oorfeo commented Sep 2, 2021

Last comment until now was Oct. 2020.
I'm Don Rechtman, a U.S. citizen currently residing in Shenzhen. More than one billion people here in China read, compose and perform using numbered Jianpu notation. If this project can be successfully completed, the Musescore community should receive a dramatic boost in size.
I'm available to beta and alpha any plugin work, and have resources to comment on suggestions for items for inclusion.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Sep 2, 2021

See #3614, that one is at least quite a bit newer

@oorfeo
Copy link

oorfeo commented Sep 2, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work in progress not finished work or not addressed review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet