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

Add inverted noteBackgroundColor #22486

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Jojo-Schmitz
Copy link
Contributor

@@ -210,7 +210,7 @@ Color EngravingConfiguration::thumbnailBackgroundColor() const

Color EngravingConfiguration::noteBackgroundColor() const
{
return Color::WHITE;
return notationConfiguration()->foregroundColor();
Copy link
Contributor

Choose a reason for hiding this comment

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

This solution is not ideal... It creates bad dependencies between the modules. The notation module should depend on the engraving module, and not vice versa

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(no such concerns had been raised with the original PR)
How to change?

@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Apr 19, 2024

The vtest diffs show that a 'different shade of white' is used as background for the notehead parentheses.
Most probably the difference between QColor("#f9f9f9") (the default foreground color)) and Color::WHITE (defined as inline const Color Color::WHITE { 255, 255, 255 };
Not sure this is tolerable.

Seems to match my comments #16715 (comment) and #16830 (comment)

@Jojo-Schmitz Jojo-Schmitz force-pushed the invert-note-backgrounds branch 2 times, most recently from aa91b13 to 1c263a2 Compare April 19, 2024 13:05
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.

None yet

3 participants