-
Notifications
You must be signed in to change notification settings - Fork 186
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
Initial refactoring for making the use of facsimile more generic #3565
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Same check is performed in Surface::GetMaxY
* Also remove call to Read/WriteFacsimileInterface in LayerElement child classes
* Placeholder for sb but not tested nor enabled
* To be reverted
* Clang formatting
* Not MEI valid structure
* Also add call to FascimileInterface::ResetDataInterface
This reverts commit 4cc1e83.
* Read page size from the surface attributes * Convert to logical units
rettinghaus
reviewed
Jan 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refactors the handling of facsimile handling following modelling discussions for the @BeethovensWerkstatt with @kepper.
For producing a rendering where the position of the elements in provided in the encoding, we use now
<facsimile>
instead of using Verovio' internal page-based model with coordinates. This is the same as in @DDMAL's neon.js and the PR should not break anything for neon. @yinanazhou, please double check.The PR is a fist step for making the handling of facsimile in neon.js more generic. I will follow up with a dedicated issue (tagging @fujinaga). One problem is that neon changes the music data or the facsimile data in various places. This needs to be cleaned up.
With the current PR, if
facsimile
has a@type="transcription"
and--breaks encoded
is set, the facsimile layout is applied. This is happening with a call toDoc::SyncFromFacsimileDoc
that sets thezone@ulx
asm_drawingFacsX
values instead of calculating the layout. Eventually, we do not want to rely on the@type
attribute but instead on the--use-facsimile
option. However, this will be changed only when code used by neon will have been made generic.The PR also makes it possible to generate a
facsimile
from a calculated layout with the-t mei-facs
output option together with--pages-all
and--breaks encoded
. This is happening inDoc::SyncToFacsimileDoc
. At this stage, this is very basic and only implements what is currently possible with the coordinates in the page-based output. That is, possitioning systems, measures, staves, and notes.Example with the following standard score-based MEI file:
Output with a facsimile generated with
-t mei-facs
:Rendering using the facsimile (detected throught
facsimile@type="transcription"
)For reference, the page-based MEI generated with
-t mei-pb
: