supercollider-quarks/JITLibExtensions

MainFXGui ignores specs from NamedControl

Opened this issue · 8 comments

Hi ! Stumbled on a small issue when using NamedControl style specs/arguments with the ProxyChain/MainFXGui classes. The specs are ignored unless input in the specs argument explicitly:

// Does not work:
ProxyChain.add3(
    srcName: \jpverb, 
	source: \filter -> { |in| 
		JPverb.ar(
			in: in,  
			t60: \time.kr(10, spec: [0.0001,100.0,\exp]),  
		);
	}
);

// Does work:
ProxyChain.add3(
    srcName: \jpverb, 
	source: \filter -> { |in| 
		JPverb.ar(
			in: in,  
			t60: \time.kr(10),  
		);
	},
    specs: (time: [0.0001,100.0,\exp])
)

Thanks! Unfortunately, this happens even lower:

Ndef(\x, { SinOsc.ar(\freq1.kr(400, spec: [30, 3000, \exp])) * 0.1 }).play.gui;

will check ASAP,
adc

LFSaw commented

another reproducer:

Ndef(\foo, { 
	SinOsc.ar(\freq.kr(spec:ControlSpec(200, 20000))) 
});

// specs are stored in specs variable
Ndef(\foo).specs

// but this (used in all NodeProxy-related stuff) returns default spec, resp. a spec defined in the Halo
Ndef(\foo).getSpec(\freq)

// hence, this does not know about the spec
Ndef(\foo).edit

any ideas?

I'm happy to help

bgola commented

I was taking a look at this and the issue seems to be when getSpec is called, it will try to find an spec using Halo, and if it doesn't find it will fallback to the global specs by doing name.asSpec.

When NamedControls are used the NamedControl class has no access to the proxy so it can't add the spec to the proxies' Halo. It uses UGen.buildSynthDef to add itself to the SynthDef.specs array, which is ignored by getSpec (which is used for building GUIs and so on). Checking the this.specs array before fallback to the global specs would fix that:

        getSpec { |name|
		var spec;
		var specs = Halo.at(this, \spec);
		if (name.isNil) { ^specs };
		if (specs.notNil) { spec = specs.at(name) };
		if (spec.isNil and: { name == '#' }) {
			name = this.key;
			if (specs.notNil) { spec = specs.at(name) };
		};
		^spec ?? { this.specs.at(name) ?? { name.asSpec } }
	}

Hope this makes sense / help :)

LFSaw commented

Thanks, @bgola

Do we need to implement this only for Object or also for other Classes?
It is a little difficult to understand where what is used...

also, since not all objects that support Halo also have a spec method, I'd propose

+ Object {
        getSpec { |name|
		var spec;
		var specs = Halo.at(this, \spec);
		if (name.isNil) { ^specs };
		if (specs.notNil) { spec = specs.at(name) };
		if (spec.isNil and: { name == '#' }) {
			name = this.key;
			if (specs.notNil) { spec = specs.at(name) };
		};
		if (this.respondsTo(\specs), {
			^spec ?? { this.specs.at(name) ?? { name.asSpec } }
		}, {
			^spec ?? { name.asSpec }
		})
	}
}

Additionally, the question is really, if we need two concurrent implementations for specs anyhow...

bgola commented

Definitely it is a bit confusing both .specs and .getSpec, they are used in different scenarios apparently. .specs is defined in SynthDef (so mainstream SC code) and .getSpec is JITLibExtensions stuff... I guess the right approach, as it is implemented now, is that JITLib takes care of using "official" SC stuff in the back. And probably we should add some explanation to the docs somehow :)

From what I can see simply implementing it for Object is enough.

LFSaw commented

my proposal would be to remove the Halo implementation of spec handling in favour of mainstream SC but keep backwards compatibility by changing the impelemtation of getSpec and addSpec to use specs(_) internally.

@adcxyz what do you think?

adcxyz commented

Hm,
I am not entirely sure that addSpec and getSpec can be moved transparently to use .specs instead of Halo.
would be good to sketch it and see if anything breaks.

adcxyz commented

the other option would be to add inline-defined specs to the tree that getSpec currently uses.