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

Integrate lates changes from DDMAL #3564

Merged
merged 113 commits into from
Dec 28, 2023
Merged

Integrate lates changes from DDMAL #3564

merged 113 commits into from
Dec 28, 2023

Conversation

lpugin
Copy link
Contributor

@lpugin lpugin commented Dec 23, 2023

No description provided.

yinanazhou added 30 commits May 16, 2023 14:23
Allow accid/clef/divLine to be child of syllable
MatchHeight action for bbox
Fix ungroup when firstIsSyl
Remove redundant new syl for insert action
@yinanazhou
Copy link
Contributor

@yinanazhou could you try to rebase the DDMAL branch?

Hi, I've merged the changes to this branch, but I'm getting a negative width/height value error in the console. Could you please guide me on where I should look into? Thanks!

@lpugin
Copy link
Contributor Author

lpugin commented Dec 24, 2023

Sure. Please post the file that is causing the issue, and all not non-default parameters you are setting to call the toolkit.

@yinanazhou
Copy link
Contributor

yinanazhou commented Dec 24, 2023

Hi, I haven't identified the specific file causing the issue yet. Below is a snippet of the SVG output, where the x/y/width/height value can be negative and the number is quite large. I wonder if it is related to any changes regarding drawing and logical system? I've checked src/view_element.cpp and src/svgdevicecontext.cpp but haven't find anything.

image

This is the command line I used to build verovio for neon:
./buildToolkit -x "Gootville,Petaluma" -DHPX

And these are the options neon uses:

this.toolkit.setOptions({
      from: 'mei',
      footer: 'none',
      header: 'none',
      pageMarginLeft: 0,
      pageMarginTop: 0,
      font: 'Bravura',
      useFacsimile: true,
    });

Any insights would be appreciated. Merry Christmas :)

@lpugin
Copy link
Contributor Author

lpugin commented Dec 26, 2023

Please send the input file you are using

@yinanazhou
Copy link
Contributor

Sorry, forgot the most important file...
005r.xml.zip

@lpugin
Copy link
Contributor Author

lpugin commented Dec 27, 2023

The problem is that there facs@lry and facs@lrx were not read. I don't know how this used to work in your branch because this causes the staff size to have an unrealistic value here:

verovio/src/staff.cpp

Lines 177 to 181 in d89dc94

Zone *zone = this->GetZone();
assert(zone);
int yDiff
= zone->GetLry() - zone->GetUly() - (zone->GetLrx() - zone->GetUlx()) * tan(abs(rotate) * M_PI / 180.0);
m_drawingStaffSize = 100 * yDiff / (doc->GetOptions()->m_unit.GetValue() * 2 * (m_drawingLines - 1));

I fixed it here d89dc94.

Rendering your test file works on this branch:
image

I am going to suggest some additional changes in the facsimile handling since we are going to use it in other contexts. I will open an issue on the ddmal/verovio repository. Once resolve, please do make sure to always make a PR to the rism-digital/verovio everytime you merge something in your develop branch. Pinging @fujinaga

@lpugin lpugin merged commit dab41c5 into rism-digital:develop Dec 28, 2023
5 checks passed
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.

2 participants