ConverterSetup creation could support more cases

[tischi]

I think the logic here could be more permissive:

 public static ConverterSetup createConverterSetup(SourceAndConverter sac, int legacyId) {
        //return BigDataViewer.createConverterSetup(sac, legacyId);
        ConverterSetup setup;
        if (sac.getSpimSource().getType() instanceof RealType) {
            setup = createConverterSetupRealType(sac);
        } else if (sac.getSpimSource().getType() instanceof ARGBType) {
            setup = createConverterSetupARGBType(sac);
        } else {
            errlog.accept("Cannot create convertersetup for Source of type "+sac.getSpimSource().getType().getClass().getSimpleName());
            setup = null;
        }
        //setup.setViewer(() -> requestRepaint.run());
        return setup;
    }

For example, the else case could be replace with:

            log.accept( "Unsupported ConverterSetup for Converters of class " +  source.getConverter().getClass() );
            if (source.asVolatile() != null) {
                setup = new UnmodifiableConverterSetup( source.getConverter(), source.asVolatile().getConverter());
            } else {
                setup = new UnmodifiableConverterSetup( source.getConverter());
            }

In addition, looking createConverterSetupRealType(..) into there is:

static private ConverterSetup createConverterSetupRealType(SourceAndConverter source) {
        final ConverterSetup setup;
        if (source.getConverter() instanceof ColorConverter) {
            setup = BigDataViewer.createConverterSetup(source, -1);

BigDataViewer.createConverterSetup(source, does not require the source to be of any specific type; thus the case source.getConverter() instanceof ColorConverter) could be handled independent of the type of the source itself.
In fact the return converter will not necessarily be of RealType.

Would it be OK if I created a PR to potentially improve this?

[NicoKiaru]

Sure, make a PR.

The lines of code for reference:

bigdataviewer-playground/src/main/java/sc/fiji/bdvpg/sourceandconverter/SourceAndConverterHelper.java

Lines 218 to 231 in 6a7e17d

	public static ConverterSetup createConverterSetup(SourceAndConverter sac, int legacyId) {
	//return BigDataViewer.createConverterSetup(sac, legacyId);
	ConverterSetup setup;
	if (sac.getSpimSource().getType() instanceof RealType) {
	setup = createConverterSetupRealType(sac);
	} else if (sac.getSpimSource().getType() instanceof ARGBType) {
	setup = createConverterSetupARGBType(sac);
	} else {
	errlog.accept("Cannot create convertersetup for Source of type "+sac.getSpimSource().getType().getClass().getSimpleName());
	setup = null;
	}
	//setup.setViewer(() -> requestRepaint.run());
	return setup;
	}

Also there was an interface (ICloneableConverter) which was made to handle particular converters in case that's useful:

bigdataviewer-playground/src/main/java/sc/fiji/bdvpg/sourceandconverter/SourceAndConverterHelper.java

Lines 182 to 212 in 6a7e17d

	public static Converter cloneConverter(Converter converter, SourceAndConverter sac) {
	if (converter instanceof ICloneableConverter) { // Extensibility of converters which implements ICloneableConverter
	return ((ICloneableConverter) converter).duplicateConverter(sac);
	} else if (converter instanceof ScaledARGBConverter.VolatileARGB) {
	return new ScaledARGBConverter.VolatileARGB(((ScaledARGBConverter.VolatileARGB) converter).getMin(), ((ScaledARGBConverter.VolatileARGB) converter).getMax());
	} else if (converter instanceof ScaledARGBConverter.ARGB) {
	return new ScaledARGBConverter.ARGB(((ScaledARGBConverter.ARGB) converter).getMin(),((ScaledARGBConverter.ARGB) converter).getMax());
	} else if (converter instanceof RealLUTConverter) {
	return new RealLUTConverter(((RealLUTConverter) converter).getMin(),((RealLUTConverter) converter).getMax(),((RealLUTConverter) converter).getLUT());
	} else {
	Converter clonedConverter = BigDataViewer.createConverterToARGB((NumericType)sac.getSpimSource().getType());
	if (clonedConverter!=null)
	{
	if ((converter instanceof ColorConverter)&&(clonedConverter instanceof ColorConverter))
	{
	((ColorConverter)clonedConverter).setColor(((ColorConverter)converter).getColor());
	((ColorConverter)clonedConverter).setMin(((ColorConverter)converter).getMin());
	((ColorConverter)clonedConverter).setMax(((ColorConverter)converter).getMax());
	}
	return clonedConverter;
	}
	else
	{
	errlog.accept( "Could not clone the converter of class " + converter.getClass().getSimpleName() );
	return null;
	}
	}
	}

And why not taking BigDataViewer.createConverterSetup(source directly ?

[tischi]

I think BigDataViewer.createConverterSetup() only works for ColorConverter...

[tischi]

PR: #255

[NicoKiaru]

PR merged. Closing this issue