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

Refactoring for making the --use-facsimile more generic #128

Closed
wants to merge 357 commits into from

Conversation

lpugin
Copy link
Member

@lpugin lpugin commented Jan 8, 2024

This PR integrates a SyncFromFacsimileFunctor for applying the information provided in the facsimile in one place rather than in different places.

The functor is called from Toolkit::LoadData if the document as a <facsimile type="transcription">

https://github.com/rism-digital/verovio/blob/0b9ca33b0e4144dff2eb68438f3cbe3169d10515/src/toolkit.cpp#L795-L797

Eventually, the goal will be to call it when --use-facsimile is set. However, this will be changed only when it will work for neon too.

Another change brought by the PR is that the neon MEI need to have each <staff> wrapped into a <section type="neon-neume-line">. That way, the MEI also makes sense without facsimile.

Example:
https://gist.github.com/lpugin/dcf63d0c43c61df55486ce16030f6402

Rendering with --header none --footer none --page-margin-top 0 --page-margin-left 0 --use-facsimile (with the facsimile applied as in neon)

image

Rendering with --header none --footer none --page-margin-top 0 --page-margin-left 0 (with the new SyncFromFacsimileFunctor)

image

Rendering with @type="transcription" removed on <facsimile> (without applying the facsimile information)

image

As you can see, there is only a different with the horizontal placement of clef and the vertical placement of the custos. I haven't figured out what the issue is with the clef. For the custos, you can see that the placement is also wrong without the facsimile layout.

Once these two issues are fixed, we can proceed to the opposite functor (SyncToFacsimile).

@lpugin
Copy link
Member Author

lpugin commented Jan 9, 2024

I fixed the custos issue. It is now looking good (as as before) without --use-facsimile with both the facsimile applied with the functor or without applying the facsimile information

image

(PS some clefs appear twice without applying the facsimile information because they are given in the encoding and also in the staffDef. This is expected)

@lpugin
Copy link
Member Author

lpugin commented Jan 9, 2024

Output without --use-facsimile but with the facsimile applied throught @type="transcription"

image

@lpugin
Copy link
Member Author

lpugin commented Jan 9, 2024

I tested with neon and it works with the following adjustments

DDMAL/Neon@develop...lpugin:Neon:develop

image

@yinanazhou
Copy link
Member

I've confirmed that it works in Neon (https://github.com/DDMAL/Neon/tree/neume-line)

image

@lpugin could you merge this PR when it's ready?

@lpugin
Copy link
Member Author

lpugin commented Jan 10, 2024

Great. Did you check editing? I think the editor_neume.cpp needs to be adjusted to call SyncFromFacsimileDoc at the end of every editing action (before the page is re-rendered).

I will also remove the various places where the drawing position retrieved from the facsimile since these are not used any more. For example this will be removed:

https://github.com/rism-digital/verovio/blob/7fc81987554928670028784054b760a492ec3338/src/layerelement.cpp#L399-L406

Since the drawing x position has been previously synced from the facsimile and is retrieved from:

https://github.com/rism-digital/verovio/blob/7fc81987554928670028784054b760a492ec3338/src/layerelement.cpp#L408-L409

@yinanazhou
Copy link
Member

Hi @lpugin, I've been pushing changes to this branch. I'm a bit confused about the drawingY and logicalY/DeviceContextY in Verovio because I noticed that the coordinate is changed (?) somewhere in Verovio.

When I first load the page (no editor actions made), I had to remove or add ToLogicalY() or ToDeviceContextY() in DrawAccid(), DrawDivLine(), and DrawSyl() to fix their position on the page (they are flipped against the page).

But as soon as I drag an element, the coordinate is change: if the element is at the top of the page, it will jump to the bottom. I was wondering if you might know what is happening? Or where I should look into?

@lpugin
Copy link
Member Author

lpugin commented Jan 17, 2024

Yes, the coordinates in MEI facsimile is reverse from the Verovio internal. That is, from top to bottom in facsimile, while Verovio coordinates are from bottom to top. It is very important not to mixed them up So the rule is:

  • convert them ToLogicalX/Y in the SyncFromFacsimileFunctor
  • convert them ToDeviceContextX/Y in the SyncToFacsimileFunctor

This is another reason why we need to remove the facsimile interaction in the drawing code

@lpugin
Copy link
Member Author

lpugin commented Jan 18, 2024

Also, you should convert the facsimile coordinates in the editor using the EditorToolkit::m_view.ToLogicalX/Y. That way, all the calculations are done in Verovio coordinates.

lpugin and others added 28 commits May 7, 2024 17:56
cmake: win include fix for MSYS
MusicXML: allow short multi-measure rests to be blocks
Add support for neumes encoded line by line
…ile-neume-line

Revert "Add support for neumes encoded line by line"
Fix regrassion introduce by facsimile-neume-line
* Change ubuntu to 22.04 for g++11
@lpugin lpugin closed this May 21, 2024
@lpugin lpugin deleted the develop-facsimile-neume-line branch May 21, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.