Use layouts for the instrument sound shaping tab / Extract classes for Envelope and LFO graphs #7193
Conversation
Move the rendering of the envelope graph into it's own class. This is a preparation step to use layouts in `EnvelopeAndLfoView`. It does not work with the mixed in painting in the paint event.
Move the LFO graph into its own class. This finally enables the removal of the mouse event and paint event code in `EnvelopeAndLfoView`. Make the enum `LfoShape` in `EnvelopeAndLfoParameters` public so that it can be used without friend declarations. Add accessor methods for the model of the LFO.
Use layouts to align the elements of the `EnvelopeAndLfoView`. This removes lots of hard-coded values. Add helper lambdas for the repeated creation of `Knob` and `PixmapButton` instances. The spacing between the envelope and LFO should be removed once there is a more open layout.
Use layouts to align the elements of the `InstrumentSoundShapingView`.
Draw the info text at around 20% of the LFO graph's height. This prepares the dialog to be scaled later. Write "1000 ms/LFO" instead of "ms/LFO: 1000" with a larger gap.
|
|
|
@Veratil, I think this is an optical illusion. If I remove three of the knobs you can see that the layout is centered by default:
So with six knobs it should also be centered. Once the dialog is more scalable we should give the layouts more breathing space anyway and also consider to give a new layout to some of the elements, e.g. by putting all the DAHDSR knobs right under the envelope graph, making the envelope graph larger, etc. I am currently working on adjusting the rendering of the envelope graph so that it becomes scalable and is not tied to the size of the background pixmap anymore. However, we have to get started somewhere and one of the first step is to use layouts wherever possible. So moving step by step into the right direction, improving things step by step even if they are not "perfect" yet. Once this is merged I would merge it into my feature branch with the instrument redesign (see here) and adjust the layouts accordingly. Currently the |
|
I can agree with this though. If possible to get a better alignment without much extra work, we should. Otherwise it can be left to a future PR. |
Looks like you've got a golden eye. 😄
It's very likely an effect of the So, I'd like to keep this for a later PR when everything will be "decompressed" anyway. Otherwise we might be hunting ghost or artifacts of Qt's layout algorithm under size constraints. And it's not like the previous hard-coded layout was perfect anyway. By the way, I also have the changes ready to make the envelope graph resizable so that it renders the graph correctly at different sizes. Shall I put it on top of this PR or shall we first merge the layout changes and then do the envelope graph changes? |
If this PR affects the rendering of the envelope then we should fix it in this PR, otherwise we can do it in a separate.
Sounds good, we don't need chase the margins at this time. 👍 |
In the current state the rendering of the envelope is not changed. It's rendered as a separate widget but the widget has a fixed size and renders everything in fixed coordinates with regards to that size. So everything like before. I will therefore create a separate PR for the dynamic rendering once this is merged.
Ok, great. Do you want to do an "official" review or did you already check the code and it's good to go? |
…apingWithLayout Conflicts: * src/gui/instrument/EnvelopeAndLfoView.cpp * src/gui/instrument/InstrumentSoundShapingView.cpp
Whitespace and formatting.
* Remove some "pragma once" for now * Adjust include orders * Variable initialization in headers * Prevention of most vexing parses
…r Envelope and LFO graphs (LMMS#7193) Move the envelope and LFO graphs into their own classes. Besides the improved code organization this step had to be done to be able to use layouts in `EnvelopeAndLfoView`. The class previously had fixed layouts mixed with custom rendering in the paint event. Mouse events are now also handled in both new classes instead of in `EnvelopeAndLfoView`. ## Layouts in EnvelopeAndLfoView Use layouts to align the elements of the `EnvelopeAndLfoView`. This removes lots of hard-coded values. Add helper lambdas for the repeated creation of `Knob` and `PixmapButton` instances. The spacing that is explicitly introduced between the envelope and LFO should be removed once there is a more open layout. ## Layouts for InstrumentSoundShapingView Use layouts to align the elements of the `InstrumentSoundShapingView`. ## Info text improvements in LFO graph Draw the info text at around 20% of the LFO graph's height. This prepares the dialog to be scaled later. Write "1000 ms/LFO" instead of "ms/LFO: 1000" with a larger gap. ## Accessors for EnvelopeAndLfoParameters Make the enum `LfoShape` in `EnvelopeAndLfoParameters` public so that it can be used without friend declarations. Add accessor methods for the model of the LFO. ## Other improvements * Adjust include orders * Variable initialization in headers * Prevention of most vexing parses
Layouts for instrument sound shaping
Use layouts to align the elements of the instrument sound shaping page and its children. Here's a comparison before/after:
This can also be considered as further preparation for a later adjustment of the instrument window to make it more scalable and to give the elements more "breathing space" (see this comment). Most of the other tabs already use layouts.
New widgets to render the envelope and LFO graphs
This pull requests also extracts the rendering of the envelope and the LFO graphs into their own widget classes. This needed to be done to get rid of the individual mouse handling and painting code in
EnvelopeAndLfoView. Both events only worked with hard-coded layouts which are removed by this PR.This also prepares both graphs to be implemented in a scalable and potentially more interactive way, e.g. the envelope graph could be used to directly manipulate the AHDSR curve. For now both graphs are set to the fixed size of their background pixmap so that they have the exact same appearance as before.
Adjusted rendering of the LFO info text
As can be seen in the screenshot the rendering of the LFO's info text has also been improved. It now displays "2000 ms/LFO" instead of "ms/LFO: 2000". The text is now always rendered at 20% of the widget height in preparation to make the widget resizable.
TODOs (for later PRs)
EnvelopeAndLfoParametersinto two classesEnvelopeParametersandLfoParametersso that each graph will only operate on the data that's it concerned with.