DISTRHO / Cardinal

Virtual modular synthesizer plugin
https://cardinal.kx.studio/
GNU General Public License v3.0
2.22k stars 154 forks source link

VCA-1 produces visual glitches #9

Closed falkTX closed 2 years ago

falkTX commented 2 years ago

Not sure if issue with VCV or Cardinal. But having this module on the board makes the entire plugin window green. Moving VCA out of the way mitigates the issue. Seems like the module is not closing/stopping a drawing operation properly, so the green used for its bars gets set for all modules.

zezic commented 2 years ago

For me it fills entire window with green only if its value is set below 0.005f, in another words, it does it when this condition is false: https://github.com/VCVRack/Fundamental/blob/v2/src/VCA.cpp#L242

But also, the following separators drawing procedure works in a strange way: https://github.com/VCVRack/Fundamental/blob/v2/src/VCA.cpp#L256-L262 The area it fills with black is too big:

And what's interesting, it renders correctly in the plugin browser, but not in the live mode.

falkTX commented 2 years ago

You are correct that this only happens if value is below 0.005f. I think the code from VCA is wrong, there is a nvgFillColor + nvgFill being called where there is a potential for no nvgRect (when all gains are below that value)

Before digging deeper into this, we need to see if the same happens with VCV as well. The v2 branch.

zezic commented 2 years ago

At least the latest binary build of Rack v2 is not affected by this issue. I think it has something to do with the way NanoVG operates in the Cardinal (GL?) environment. Maybe debugging the wrong drawing of the separators will give some answers.

falkTX commented 2 years ago

good to know, thanks for checking.

I did some quick testing, and the area within the "meter" is simply not being drawn. if we extend the green area we can see the middle point where it should be drawn is the section where it is not. Seems to me some logic is inverted somewhere.

falkTX commented 2 years ago

To make more clear what I mean, I extended the green rectangle 50px on each side.

Screenshot_20211018_230318

But on the plugin browser it looks as intended.

Screenshot_20211018_230422

falkTX commented 2 years ago

Oh, interestingly commenting out the separators makes the whole area bring drawn as expected.

Screenshot_20211018_230603

zezic commented 2 years ago

Yes. It's omething wrong with them. I have some ideas to test. Gonna try to debug it.

falkTX commented 2 years ago

already found something, this code for the separators works:

        // Invisible separators
        const int segs = 25;
        for (int i = 1; i <= segs; i++) {
            nvgBeginPath(args.vg);
            nvgRect(args.vg,
                    r.pos.x - 1.0,
                    r.pos.y + r.size.y * i / segs,
                    r.size.x + 2.0,
                    1.0);
            nvgFillColor(args.vg, color::BLACK);
            nvgFill(args.vg);
        }

basically drawing each path separately.

so this is just joining each path together when drawing...? there is an opengl config for this right?

zezic commented 2 years ago

You found it! :) I will try to search for anything related. UPD: No luck for me anymore.

falkTX commented 2 years ago

I also had no luck despite trying many things. Because this is a very popular module and the bug makes the cardinal plugin unusable, I added a workaround in b3fd44c2cb23d84939094f1a3d2d537450706367

falkTX commented 2 years ago

Patch-fix has been submitted upstream in https://github.com/VCVRack/Fundamental/pull/129

falkTX commented 2 years ago

Since I take it that upstream is pretty much never going to respond to that, I just setup a fork https://github.com/CardinalModules/Fundamental

No more workarounds needed, plus we need to start forking a few modules to send patches upstream. Otherwise my falkTX account will be flooded with forks. And we want to link/reference the submodules to good repositories with fixes, not original ones that have yet to see PRs merged. So https://github.com/CardinalModules is born :tada: