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

Articulation tweaks #13522

Merged
merged 5 commits into from
Oct 25, 2022
Merged

Articulation tweaks #13522

merged 5 commits into from
Oct 25, 2022

Conversation

mike-spa
Copy link
Contributor

Three separate jobs:

  • A small modification of default styles.
  • Correcting the alignment of composite staccato/tenuto marks.
  • Improving the efficiency when fetching the category of a given articulation mark (it doesn't makes sense to perform all those comparisons every time we need to query it, makes more sense to compute them once and store them).

Before:
image17055
After:
image18575

src/engraving/libmscore/articulation.cpp Outdated Show resolved Hide resolved
Comment on lines 125 to 130
bool _isDouble = false;
bool _isTenuto = false;
bool _isStaccato = false;
bool _isAccent = false;
bool _isMarcato = false;
bool _isLuteFingering = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

To reduce memory usage, you could store these using a single field of flags. But I'd first ask @vpereverzev if he thinks that's better.

Anyway, to do that, you would create an enum like this:

enum class ArticulationCategory: char { // char makes sure that this will use only 1 byte, 
                                        // which consists of 8 bits, 
                                        // which is enough to store these 6 flags
    None = 0x00
    Double = 0x01
    Tenuto = 0x02
    Staccato = 0x04
    Accent = 0x08
    Marcato = 0x10
    LuteFingering = 0x20
}

Then create a Flags from that enum: (you need to #include "flags.h" for this)

DECLARE_FLAGS(ArticulationCategories, ArticulationCategory)
DECLARE_OPERATORS_FOR_FLAGS(ArticulationCategories)

Then replace all these members with:

Suggested change
bool _isDouble = false;
bool _isTenuto = false;
bool _isStaccato = false;
bool _isAccent = false;
bool _isMarcato = false;
bool _isLuteFingering = false;
ArticulationCategories m_categories = ArticulationCategory::None;

Then replace the is... methods with:

    bool isDouble() const { return m_categories & ArticulationCategory::Double; }

etc.

Then modify computeCategories like this:

m_categories.setFlag(ArticulationCategory::Double, 
                     _symId == SymId::articMarcatoStaccatoAbove || _symId == SymId::articMarcatoStaccatoBelow
                     || _symId == SymId::articTenutoStaccatoAbove || …)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I'll try!

@mike-spa mike-spa force-pushed the articulationTweaks branch 4 times, most recently from 98a914a to 233e277 Compare September 30, 2022 07:31
@oktophonie oktophonie added the vtests This PR produces approved changes to vtest results label Sep 30, 2022
bool isStaccato() const { return m_categories & ArticulationCategory::STACCATO; }
bool isAccent() const { return m_categories & ArticulationCategory::ACCENT; }
bool isMarcato() const { return m_categories & ArticulationCategory::MARCATO; }
bool isLuteFingering() { return m_categories & ArticulationCategory::LUTE_FINGERING; }
Copy link
Contributor

Choose a reason for hiding this comment

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

const got lost here :)

correction


correction


cleanup


cleanup


cleanup
correction


efficiency


correction
added vtest


vtest corr
@vpereverzev vpereverzev merged commit cd4ffec into musescore:master Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vtests This PR produces approved changes to vtest results
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants