helge17 / tuxguitar

Improve TuxGuitar and provide builds
Other
492 stars 42 forks source link

Unable to display certain accidentals #270

Open cernael opened 8 months ago

cernael commented 8 months ago

To reproduce:

Similar story for key 7 flats, Cb scale (0, 2, 4, 5, 7, 9, 11, 12 on the B string), Cb, Db, Eb, Fb, Gb, Ab, Bb, Cb expected, B, Db, Eb, E, Gb, Ab, Bb, B displayed.

Sure, a workaround here would be to pick the enharmonically equivalent key (5 flats/sharps), but it seems impossible to accurately display the key of Gb/F# major (either Cb is incorrectly rendered as B, or E# as F).

What I would like at minimum is a way to coerce the score display to toggle between enharmonic versions of the same sounding pitch. It appears that it is only able to display accidentals for the black keys, and only either flats OR sharps in a single measure, even when a combination is called for (for instance, the 3rd of a D7 chord in Gm).

guiv42 commented 8 months ago

Not a trivial issue: my understanding is that the evaluations of note position on the score and presence of an accidental symbol are not implemented at the same place. Some refactoring needed here.

I'll try to do something. First step is to fix existing bug (i.e. don't display F when E# is expected). Then, addition of user controls to choose between enharmonic versions of the same pitch could be added. Note that this would require an evolution of .tg file format.

cernael commented 8 months ago

It seems like the key selector directly determines two things - "should black keys be considered flat or sharp" (and by that assigns note names to all pitches) and "which note names are in the key, and which require accidentals". To fix this, the first decision needs to be a lot more nuanced, mapping out not a single note, but a set - ideally including double sharps/flats as well.

I haven't looked at the code though, my analysis is made solely from the user experience. I'm willing to help if I can, but I am not a java developer.

guiv42 commented 8 months ago

I'm starting to understand how this code works. And basically no evolution seems possible to me before a first step of rationalization is done. My current plan for this first step is to create one single reference in the code to choose how one note shall be represented (e.g. F or E#). At first just considering note pitch and measure key signature. As I mentioned above, yet the 2 decisions "F or E" and "natural or sharp" are completely de-correlated from implementation point of view...

Once this first step is done we may open the discussion for a future evolution. Stay tuned!

guiv42 commented 8 months ago

unexpected complexity: computation of note octave also needs to be adapted: Midi note 60 can be either a C4 or a B#3... For now my B# are displayed 1 octave too high and my Cb one octave too low :D Progressing

cernael commented 8 months ago

Oof. Yeah, that naïve approach's not gonna be able to support this feature. Looks like the method is only called from one other place, where the key is available? Could send that in and condition on the extremes for a first approximation, thought it's not the prettiest solution.

guiv42 commented 8 months ago

Just for your info: I'm the author of "this naive approach" ;) Or more precisely: I'm the person who re-implemented this naive approach at a single place in the code (it was replicated in many places before, see #37). Current issue is very similar: some notes manipulation functions are implemented at several places. So I'm trying to cleanup. That should take some time but I'm rather optimistic. Does something like this make sense ? (not mature enough to be published) Screenshot_20240311_231435

cernael commented 8 months ago

No disrespect intended - naïveté is preferable up until it's unable to support the increasing demands from feature-hungry users. If anything, I'm the madman here for wanting to use terrible keys. :-P

Yes, that looks pretty sane and sensible.

guiv42 commented 8 months ago

No disrespect intended

sure, no pb! And from a musical point of view, nothing's mad about your request, it makes sense :) Thanks for your feedback about the screenshot. Still progressing: I now have a correct display. I'm just struggling with "edition mode" (where you create/delete notes by clicking in the score area), but that should be feasible.

guiv42 commented 8 months ago

@cernael: could you please have a try with latest pre-release?

At least you should be able to work with these "terrible" keys ;) Currently the choice of the note to display (e.g. C or B#) is done considering only note pitch and measure key signature. I think that's the best I can do in this first step.

Next step should be to create new controls to enable user to add specific constraints (e.g. force "G#" instead of "Ab") on a note-per-note basis. This should be easier to implement now. Note that this adds new data to the song, so it requires an evolution of the .tg file format. So, not a short term evolution.

cernael commented 8 months ago

From a quick test, it looks good. My terrible keys are rendered correctly.

And that next step is great - it would also allow for picking silly stuff like double sharps and flats, which probably never need to be chosen by the system as such. Exiling that choice to the explicit per-note forcing control is likely to fit the mental model of anyone wanting to use them anyway.

guiv42 commented 8 months ago

OK, thanks for your feedback.

So I'm changing status of this issue from "bug" to "enhancement". In low priority, since it requires an evolution of the file format (this requires time).

cernael commented 8 months ago

@guiv42 Just so I have the concepts right - the bug parts, the incorrect display of terrible keys fixed in the prerelease above, are due for an official release soonish, while the enhancement part, the forcing of certain accidentsls, will wait for the file format evolution?

I also got an idea, but I'm about 70% sure it won't work. But if there were per-key tables detailing whether the non-key black notes should get flats or sharps (based on which is likelier in the given key), would that be an enhancement that could be pushed in without the file format evolution? (The top reason for why it might not work is that I suspect that the likelihood is affected by the mode, major/minor/[other], and since the program only works with [number of flats/sharps], needed info is missing.)

guiv42 commented 8 months ago

Just so I have the concepts right - the bug parts, the incorrect display of terrible keys fixed in the prerelease above, are due for an official release soonish, while the enhancement part, the forcing of certain accidentsls, will wait for the file format evolution?

Yes this is what I can do for now. There are several pending feature requests that need to create additional data in tuxguitar file, thus breaking compatibility. Since these new files would not be accessible to all users using previous versions of TuxGuitar (most probably a huge majority of users) this is not an easy decision, so that might take some time.

About your idea: I think you're right, there's probably not enough information available to make a better sharp/flat decision.

guiv42 commented 2 months ago

Development of new file format has made serious progress recently (see tuxguitar-next branch), so I reopen this topic.

I now have a working implementation in a dev branch: a new action is defined to toggle one note representation between enharmonic versions (see video)

https://github.com/user-attachments/assets/a3026d0a-6a3d-47c9-93ad-28f105417b98

Currently it's only accessible with a keyboard shortcut. @cernael : any idea where to put a menu entry and/or a button in a toolbar to trigger this action?

guiv42 commented 1 month ago

Feature is now fully implemented in tuxguitar-next branch, to be merged in master with next major release. See menu item "Beat / Alternative Enharmonic"