Whitebackground

[nam-van-nguyen]

Would you please merge my changes here: https://github.com/nam-van-nguyen/PianoBooster on the "whitebackground" branch? I updated the code so that the UI preference dialog allowing people to set the red/green/blue integer code (0-255) for various colors. Everything appears to work quite well. In the future, it would help to add a color picker to help with the usability.

[pianobooster]

I would like to first run up and test your code before merging. However I have got other important things happening at moment, and so I will not be be able to look at this until the week after next. Sorry for the delay.

[nam-van-nguyen]

I would like to first run up and test your code before merging. However I have got other important things happening at moment, and so I will not be be able to look at this until the week after next. Sorry for the delay.

Nice to see your message. This is not urgent, so whenever you have time would be great. Thanks!

[Martchus]

Thanks for implementing this. Not being able to configure colors is the limitation which annoyed me most. So I've just tested it out and it works generally.

One problem I've noticed is that some colors (e.g. the background) are not updated immediately after closing the preferences (only after restarting the app). Maybe that's also only because I'm using Wayland¹.

I like that the different colors are all customizable individually although I found the defaults not very nice. I suppose a completely white background and completely back notes would be better than grayish colors (for the sake of contrast and also for consistency with "normal" sheet music).

---

That's how it can look like after playing a little bit with the preferences:

I suppose that's kind of pretty (well, it still isn't Musecore). Only the keyboard at the bottom looks a bit off with no lines to distinguish the keys.

---

I must also say that the preferences dialog looks quite noisy and could have some convenience feature. I'd definitely just put one "Colors:" label at the top instead of repeating it in every row within the list view.

Just to give you some inspiration, that's how I did once something similar in my project Syncthing Tray (the most relevant source code is actually in qtutilities):

And:

Not having the RGB values directly in the list keeps the dialog much cleaner but of course it is a matter of taste. I also added a button to select some presets in my dialogs. Adding a small preview was actually also not that difficult and looks quite nice in my opinion. I'm aware that the matrix-layout is likely not going to work for PianoBooster but some cells could be disabled if they don't apply. Note that the dialog from the 2nd screenshot allows one to pick a single color to compute everything else from it. That saves some time but still allows one to tweak details. Both dialogs also use a special "ColorButton" (also in the grid view) which looks quite nice in my opinion. The one in your dialog has some noisy looking rectangle inside (at least with Breeze style, see screenshot at the bottom²).

Note that all of this is just for inspiration. I think the dialog is good enough to consider the PR mergable.

---

¹ Using KDE. To make PianoBooster draw something I had to run it via XWayland by passing the CLI args -platform xcb.

²

[nam-van-nguyen]

Here's how the image looks like on my computer. The center of the buttons is a character. Probably your system doesn't have the font. I will look at making an image for it.

Although not as good as a preview, you can play a midi, then open the preference dialog, change some settings and click on the bottom left corner "Apply" button to see the change.

The background should change when the "Apply" button is clicked. If not, see if you can get the latest code, or if you already did, then it's some peculiarity of Qt framework that may cause it, or the way you run it under KDE.

The noisy integer fields maybe removed completely. It was written originally, so people can at least change the color using external color picker. But I then added the color picker making the fields a bit redundant.

I was thinking about having presets also, or allow users to open/save settings or just color settings. I don't know. My time is very limited, and I don't know how much that is wanted. A simple way is to just to post a list of them on GitHub as text, and then users just copy and paste to the "Piano Booster.ini" file. I know it's not optimal, but at least we now have that ability.

The default color maybe not optimal. I basically took what was already there (in the forked branch), and change the background a little since I stare at screen more than 10 hours a day, and the bright white background hurts my eyes. Each has their own preference.

I appreciate you took time to review and providing feedback. With limited time, I will see what I can do.

[nam-van-nguyen]

It looks like the menu color and menu selected colors are not used anywhere. Someone please confirm it so I can remove them. I searched and didn't see any reference to them.

There appears to be segmentation fault after the program exits.

[nam-van-nguyen]

Here's an updated preference dialog:

[Martchus]

The center of the buttons is a character.

It is a bit unfortunate that the character doesn't exist in KDE's default font and probably also not in other fonts. Maybe just keep the buttons empty as the simplest solution?

But I'd say the new dialog looks already much better.

There appears to be segmentation fault after the program exits.

I also noticed that - so that's then not a problem specific to my setup. GDB (e.g. used via Qt Creator) tells one exactly where it happens so debugging it shouldn't be that hard.

I'll try your updated code later this evening.

[nam-van-nguyen]

I tried to debug the code with QT Creator, there is no Debug profile, only deployment. I couldn't figure out where to add the debug flag for the build. This must be a noob question, and someone please let me know since I don't use QT creator before.

Anyway, I changed the line 70 of QtMain.cpp locally, to call exit(0) versus return, and it works. Not sure if this is the right way, so I don't commit it.

exit(value);
//return value;

[Martchus]

Qt Creator should definitely allow you to do a debug build. At least for me it works like for any other CMake project. Not sure whether I'll be able to get to it again this evening but next time I can make a screenshot of my Qt Creator config if that's helpful. Of course you can also just invoke the build from the command line and use GDB directly (which has a built-in TUI) or use any other GDB fronted.

And no, using exit is not a correct solution. It simply means bypassing invocation of destructors but there's likely some UB in the code which needs fixing.

[Martchus]

That's the drop-down menu you would add a build config:

Or you simply set CMAKE_BUILD_TYPE to Debug manually. I'd also recommend to use the latest version of Qt Creator because they recently improved the CMake integration a lot.

[Martchus]

On my system the segfault happens in btw:

if (font!=nullptr) delete font;

Not the source of the issue, but I'd like to note that delete does a null-check on its own so the if is completely unnecessary.

[Martchus]

Looks like font is an object that comes from here:

FTFont * setupFont() {
    static FTFont * cacheFont = NULL;

So calling delete on it later is likely problematic because the object might have already been deleted elsewhere if setupFont() is invoked multiple times.

[nam-van-nguyen]

I made another branch for supporting background images. I really like it, although I don't have enough time to adjust theme colors and finding images that would be good.

I could possibly close this request and submit a pull request from that branch instead.

https://github.com/nam-van-nguyen/PianoBooster/tree/imagebackground