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.

Sign up for GitHub

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

Jump to bottom

factor hook offset into stem slash calculations #19559

Merged
merged 1 commit into from
Oct 5, 2023
Merged

factor hook offset into stem slash calculations #19559

merged 1 commit into from
Oct 5, 2023

Conversation

miiizen
Copy link
Contributor

@miiizen miiizen commented Sep 29, 2023

Resolves: #19481

  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

@oktophonie oktophonie added the vtests This PR produces approved changes to vtest results label Sep 29, 2023
@miiizen miiizen changed the title factor hook offset into stam slash calculations factor hook offset into stem slash calculations Sep 29, 2023
@zacjansheski
Copy link
Contributor

Tested on Mac OS 11.6.5, Ubuntu 20.04.6, Windows 11
Approved (pending @mike-spa 's approval)

cbjeukendrup
cbjeukendrup reviewed Sep 29, 2023
View reviewed changes
src/engraving/rendering/dev/tlayout.cpp Outdated
@@ -4821,7 +4821,7 @@ static void layoutStemSlash(const StemSlash* item, const LayoutContext& ctx, Ste
&& (musicFont == "Bravura" || musicFont == "Finale Maestro" || musicFont == "Gonville")) {
angle *= angleIncrease;
}
endX = hook->layoutData()->bbox().translated(hook->pos()).right(); // always ends at the right bbox margin of the hook
endX = hook->layoutData()->bbox().translated(hook->pos()).right() - hook->offset().x(); // always ends at the right bbox margin of the hook
Copy link
Contributor

@cbjeukendrup cbjeukendrup Sep 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe it would be better to replace hook->pos() with hook->layoutData()->pos() instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes - I hadn't noticed that hook->pos() included the offset, thanks!

@zacjansheski
Copy link
Contributor

Is this waiting approval from @mike-spa or is it ready to merge?

mike-spa
mike-spa approved these changes Oct 4, 2023
View reviewed changes
Copy link
Contributor

@mike-spa mike-spa left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@cbjeukendrup
Copy link
Contributor

cbjeukendrup commented Oct 5, 2023 •
edited
Loading

(rebased, because of layoutData/ldata conflict)

Will merge after the checks pass!

@cbjeukendrup cbjeukendrup merged commit a23459c into musescore:master Oct 5, 2023
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oktophonie oktophonie oktophonie approved these changes

@cbjeukendrup cbjeukendrup cbjeukendrup approved these changes

@mike-spa mike-spa mike-spa approved these changes

Assignees

@zacjansheski zacjansheski

Labels
vtests This PR produces approved changes to vtest results
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Horizonal offset of flag on acciaccatura extends slash
5 participants
@miiizen @zacjansheski @cbjeukendrup @oktophonie @mike-spa