LMMS
Improved Waveshaper / Spline-based graph replacement
Fruity Waveshaper is one of the plugins I use the most in FL. It's a fantastic general-purpose digital distortion. The Waveshaper plugin by Diizy is good and definitely has its place, but it is lacking in a few key areas:
- It is incredibly difficult to create a smooth curve. If you want a smooth curve and then a harsh edge it becomes even harder.
- Diizy's waveshaper uses graph.h, which uses 200 discrete points to store the curve. This means resolution is limited, and this limits the steepest curve possible in the shaper.
The original plugin is very similar to the 8-bit shaper by Xfer. Again, not a bad thing, but I certainly think another option would provide a better workflow and overall general-purpose plugin.
The changes I plan to make are two-fold:
- Replace the graph. This will be a fairly large project. In the long run I am interested in improving automation to some extent, and my hope is that this will help me get a feel for how to do that and make things easier for me in the future.
- Overhaul the DSP. This is a necessary side effect of the first change. Because the DSP is so simple, switching from a discrete to a vector-based graph will change pretty much everything in the calculations.
Todo:
- [ ] Vector Graph
- [x] Add bare functioning graph replacement
- [x] Create data structure for internal data storage
- [x] Add function to get output float based on input
- [x] Add initial revision of drawing code based on internal data
- [x] Connect click/drag events
- [x] Allow points to be created via right click
- [x] Make point positions adjustable via click + drag
- [x] Add context menu on point for deleting point
- [x] Allow curve type to be changed
- [x] Make curves adjustable via click + drag
- [x] Allow curve to be reset with right click
- [x] Add more curve types
- [x] Clean up
- [x] ~Refactor with usability in mind~
- [x] Allow snap to grid
- [x] Add ~comments~ documentation
- [x] Remake UI in new style
- [ ] Implement saving and loading
- [ ] DSP
- [x] Process samples by calling function from vector graph
- [ ] (VectorGraph) Attempt optimizations
- [ ] Waveshaper 2
- [x] Remove unnecessary code from the original Waveshaper
- [ ] Add asymmetric feature
- [ ] Clean up
- [ ] Add comments
- [ ] Implement saving and loading
Does anybody know why the circleci build is failing? This error seems cryptic to me.
Makefile:151: recipe for target 'all' failed
make: *** [all] Error 2
Exited with code 2
Edit: It's always after updating Xpressive. I wonder if I accidentally updated the submodules... Edit 2: Nah, I don't think that's it.
I want to remake the UI using the new style, but I want to do it right with widgets and all, and that will require adding a new separate new widgets. Since this PR is approaching 2000 lines, I think it would be best to save that for another PR.
Also, tagging @budislav for feedback on the design. I was thinking about making the tension handle grow when it gets moused over or something. I copied your point width on the automation track design but I feel these could be smaller.
Also, tagging @budislav for feedback on the design. I was thinking about making the tension handle grow when it gets moused over or something. I copied your point width on the automation track design but I feel these could be smaller.
@SecondFlight Thanks for asking! Well yes that handler should be smaller, but this can be changed after testing it. So for now just make it little smaller. Much important are these three things:
- We need one gui kit with all components so they can be used in every part of lmms.
- Everything need to be scalable so have on mind high display resolutions. @fundamental made vector based gui framework for new Zyn Fusion so maybe he can tell you more about it.
- Use paterns. If you are going to make only plugin changes, make it exactly like in my designs. Plugin height, title bar with power button and side panel with three knobs are used in all plugins so everything should look the same. https://budislavtvp.deviantart.com/art/LMMS-UI-UX-Design-Concept-Single-Window-523696539
@budislav Thanks for the feedback! The graph is intended as the first part of that toolkit. It's completely separate from the plugin and is scalable at least in theory. It doesn't use any images. I'll be sure to ask fundamental about his vector UI.
In the next PR for the plugin UI I want to start building that toolkit.
What do you envision in terms of scaling? At the moment I'm going off of pixel measurements. I want to start thinking about how to architect it to make it scalable. Should I be basing my scaling on DPI and keeping widths the same based on that? That may complicate my drawing code a bit for those one-pixel lines you have. They don't work with antialiasing, but they will need to change in width. I want to keep them in multiples of one pixel without Qt trying to interpolate.
That was a bit of a ramble, so let me rephrase just the question. As I understand it, you want relative widths to remain the same, but you want the scaling to be able to be changed depending on DPI, is that correct? And should the DPI be able to be anything or are we only allowing certain multiples? I can work with whatever you think is best but I want to start thinking about these things before I get too far into the design process.
Since this PR is approaching 2000 lines
One of the many reasons why I ended up making a custom toolkit for zyn-fusion in a scripting language was due to just how much code GUIs can end up being. For Zyn-Fusion there's ~30kloc of code and that's excluding ~13,000 lines of metadata used to populate values within the interface. If it was implemented only in C or C++ I could easily imagine the total size would be closer to 100kloc.
So, be prepared for a lot of code if you're going down this path.
What do you envision in terms of scaling?
For Zyn the GUI is all vector drawing, so rescaling it is pretty easy, but rescaling it while making it look good is a separate thing entirely. The initial scaling you describe would be constant aspect-ratio scaling which still has its challenges, but isn't the full case where the aspect ratio of the window can change to more or less arbitrary values. In that case, then you have to worry about custom layout engines and even perhaps multiple modes of layout/rendering based upon the more extreme cases.
For the constant-aspect ratio case you still need to worry about perturbing some elements to fall on pixel boundaries (to prevent blurring), scaling lines based upon pixel boundaries, and perhaps even changing the level-of-detail based upon large/small renderings. You can start with saying the design can be AW by AH where A is a known scaling factor (0.5,1,2,4,...) and W by H is the original design, but I personally like the more challenging case where you don't know in advance what the scaling factor may be.
I'll be sure to ask fundamental about his vector UI.
Feel free to ask questions. Anything last minute could in theory make it into the presentation at LAC in a few days.
@fundamental What's the advantage of using metadata over just making a really solid and specific set of classes? Qt already does a lot of the work for you in terms of compartmentalizing widgets. I had in mind a low-level framework that could be called on by the plugins in a generic sort of way to arrange the UI according to the design spec. Using metadata would be a completely different workflow I would imagine, though not necessarily a worse or better one.
Also, the actual drawing code for the graph is less than 150 lines out of 500 in the cpp. I could see this growing even with the proper compartmentalization, but so far it seems to be quite manageable.
For Zyn the metadata is used to avoid repeating information that's used elsewhere within the synth. This includes parameter units, ranges, tooltips, etc. One thing that happens frequently in audio apps is that the section of the code which works with audio processing/generation has a laundry list of assumptions about the parameters handed to them and those assumptions are informally duplicated and rewritten in several layers including the GUI. So for Zyn you just say "This knob is connected to the envelope attack" and then the rest of the information is populated from the metadata without you needing to specify all that much.
For an idea of what this means in practice, see https://github.com/mruby-zest/osc-bridge/blob/master/schema/test.json#L741-L763 or https://github.com/mruby-zest/osc-bridge/blob/master/schema/test.json#L832-L841
I had in mind a low-level framework that could be called on by the plugins in a generic sort of way to arrange the UI according to the design spec.
Sounds like the half way point between a layout engine and what I would call an 'inner-toolkit' which is to say a GUI toolkit built on top of or within a pre-existing one.
Also, the actual drawing code for the graph is less than 150 lines out of 500 in the cpp.
GUIs are made out of edge cases, so the drawing code will continue to be fairly simple, but then you have custom event handling, lots of widget settings/configuration, layout, defining widget groups, defining how they glue onto the underlying DSP, etc, etc, etc. Code bloat is pretty inevitable within this domain.
Right, that makes sense. I don't think that a decision here is something I should be making myself then, it sounds like it will span across the entire software and affect most future development. Perhaps this is something I should create an issue for to start a discussion.
The question in my mind is, how much do we want to rip up the current system to implement these UI changes? Having something external that says "this knob hooks up here" would be great, but it's a completely different paradigm from what we currently have, and implementing it would involve a lot of well-thought-out core changes in a number of areas.
But in the end I don't know what's best and I'm going off of assumptions. I think I'll make an issue for discussion later today.
My own opinion as an observer is that with the current LMMS development community and the current internal LMMS architecture an organized large scale UI upgrade isn't feasible at this time. Any work would need to be much smaller scale otherwise the trend of individuals building moderate sized goals, getting frustrated, and subsequently leaving the project, will likely continue. For Zyn the used approach was to upgrade the internal architecture to make large scale GUI changes feasible with restricted developer resources, so that may be an option for LMMS, though keep in mind that's a very large project within itself.
And I really don't feel equipped to do that kind of overhaul, both from lack of general experience as well as a lack of knowledge of the inner workings of the software. My current plan is to slowly start developing widgets that could be useful later so as to save time for whoever puts it all together, be it me or somebody else. Still, I completely agree that implementing the UI for an individual plugin or feature shouldn't take weeks and should be doable by someone fairly new to the codebase.
@SecondFlight Just to answer on your questions, when I said gui scaling I meant that it should look crisp and pixel perfect on all resolutions even high display resolutions like "retina". FL studio have some nice settings for scaling gui so you if user feel that his gui is too small he can just scale it up. https://www.image-line.com/support/flstudio_online_manual/html/envsettings_general.htm This is why I think there should not be images in gui at all, I think you understand :)
/root/project/src/gui/widgets/VectorGraph.cpp:632:64: error: 'rightPoint' may be used uninitialized in this function [-Werror=maybe-uninitialized]
else if (checkRight && potentialSnappedValue > rightPoint->x())
Oh come on, checkRight will only be true if rightPoint is initialized. sigh I'll fix this tonight.
@PhysSong, Thanks for reviewing this. I'll wrap up this PR and address the comments once 1.3 becomes a development focus.
I've resolved merge conflicts and applied minor fixes. Modernization and other reviews are not applied yet. I'll try to continue working on this.
@bratpeki It was to fix the MSVC build. The error only occurred because we are treating warnings as errors. It was technically fine already
Overall I really like the functionality offered by this WaveShaper. For the most part it works pretty intuitively and is easy to use. I was able to figure out all the basics myself within a couple minutes. It also looks pretty nice.
My biggest gripe with this PR is how this is a WaveShaper replacement which leaves the old WaveShaper plugin intact.
Here is how I believe upgrades like this should work: In the best case scenario, you replace the old behavior with the new behavior while finding ways of upgrading projects to the new behavior without breaking backwards compatibility. This is a one-way upgrade. In the 2nd best scenario, you allow both the old and new behavior to co-exist but make the new behavior the default and make the old behavior inaccessible from the GUI.
This PR goes the 2nd route, except it leaves the old WaveShaper completely intact. I would strongly prefer if this PR instead went the 1st route by replacing the old WaveShaper and providing an upgrade routine for old projects.
Also, this WaveShaper does not offer any means of automating the points or tension handles. This is a significant missing feature.
There is no "reset" button like the old WaveShaper has, which is a bit inconvenient. And the window cannot be resized.
I also have some issues with the controls...
This is how the mouse controls work right now:
- When mouse is over empty space:
- LMB (left mouse button) does nothing (bad)
- RMB (right mouse button) creates a new point (unintuitive since the automation editor uses LMB for that)
- When mouse is over a point:
- LMB moves the point around (good!)
- RMB opens context menu with curve types (could be better, but at least a "Delete" option is present which makes it partially consistent with the automation editor)
- When mouse is over a tension handle:
- LMB adjusts the curve (good!)
- RMB resets the curve (could be better)
And here are my proposed changes to the controls:
- Use LMB over empty space to create points, just like in the automation editor
- Use RMB over a point to delete it (no context menu). This is more consistent with the automation editor.
- Use RMB over a tension handle to open the context menu with curve types. The option to reset the curve should be in this menu. This makes more sense to me since the tension handle is positioned directly in the center of the curve it pertains to, which makes it a more obvious choice for the context menu than right-clicking the point to its right.
With these adjustments, the controls would be perfect IMO.
And in the future:
- Holding Ctrl + LMB dragging a point or tension handle could create an automation connection for automating it, just like how the automation connections for knobs and other controls are created
- Using the RMB over empty space could potentially be reserved for a menu with options like saving/loading the vector graph points to/from the clipboard
My biggest gripe with this PR is how this is a WaveShaper replacement which leaves the old WaveShaper plugin intact.
Could you elaborate? Is this a new plugin, now replacing the old one?
Reading through @messmerd's comment right now!
I would strongly prefer if this PR instead went the 1st route by replacing the old WaveShaper and providing an upgrade routine for old projects.
Given the strained dev resources, I'd say we keep both and drop Waveshaper1 in favor of Waveshaper2 in some future release. Drawing a waveshaper by hand is a weird feature in general.
Also, we should do the same for Dynamics Processor after merging this feature.
There is no "reset" button like the old WaveShaper has, which is a bit inconvenient. And the window cannot be resized.
Agreed. This is pretty inconvenient.
Adding to this, my comments are in italics:
-
When mouse is over empty space:
- LMB does nothing (bad) (it's okay, we can fill this in the docs and rework it in future PRs; if we switch to this we should use a different cursor over points)
- RMB creates a new point (unintuitive since the automation editor uses LMB for that) (I talked about this to some extent in the point above)
-
When mouse is over a point:
- LMB moves the point around (good!) (agreed)
- RMB opens context menu with curve types (could be better, but at least a "Delete" option is present which makes it partially consistent with the automation editor) (double-LMB for a menu, RMB for deletion!)
-
When mouse is over a tension handle:
- LMB adjusts the curve (good!) (agreed)
- RMB resets the curve (could be better) (I wouldn't change that, it makes total sense; maybe a double-LMB instead would make more sense for you?)
How I think it should work:
- Double-LMB makes a point and also opens the context menu for that point, which is more in line with the automation editor
- LMB moves the point, RMB deletes it
If we wanna go down the automation route, there's certainly more talk involved.
I could take this over and keep developing the new waveshaper, but a backwards compat with "Waveshaper1" would be a bit overkill for me.
When do you plan to implement saving and loading functionality?
By the way, there seems to be a bug where when you try to drag the end point of the curve outside the grid area and right-click, the point goes outside the grid area (it seems to also create a new point at the side of it).
I'm down for this to be made as Waveshaper2, unironically. No backwards compat. Then in the future, when possible, we dump the OG Waveshaper.
Alternatively, IF someone is willing to do it, we could have a button or context menu option called "Draw Mode" which toggles the legacy behavior. This would probably be better but take more time and effort.
@szeli1 we could merge this into a branch and then have you push to that branch u til it's ready, kinda like what @SpomJ is doing for detachable windows now?