bigdataviewer/bigdataviewer-playground

Resampled Source

Closed this issue · 14 comments

@NicoKiaru

Thanks a lot! It is working!

I'd have a couple of suggestions/wishes.


@Override
    public String getName() {
        return origin.getName()+"_ResampledAs_"+resamplingModel.getName();
    }

Could we please change that to

@Override
    public String getName() {
        return resamplingModel.getName();
    }

This would give us the flexibility to give the source any name we want (including: origin.getName()+"_ResampledAs_"+"something" as once could just give it this name).


Could we add a constructor like this:

EmptySourceAndConverterCreator( FinalRealInterval interval, Source source );

And this would use the source to figure out an optimal number of voxels, such that it would mimic the number of voxels in the source as good as possible?


Could we add the option to shift the ResampledSource to the origin? I guess adding a boolean in the constructor that would trigger a Views.zeroMin() at the right place?

Regarding the name, I think even better would be if the SourceResampler would get a String name argument into its constructor and that would be the name of the new Source.

@NicoKiaru would you be fine with implementing above suggestions? I can do it in a PR, but I don't want to work on it if you would not like that...

Hi @tischi,

No I think it's alright, I just don't have time to work on it, but I'm happy if you make a PR.

I will just check whether the serialization remains unaffected by this.

Regarding your points:

  • For the name change, I don't see any issue.

And this would use the source to figure out an optimal number of voxels, such that it would mimic the number of voxels in the source as good as possible?

I think that's difficult. Because there are too many ways to figure out the optimal number of voxels. Instead I would rather try to make a static function somewhere in SourceAndConverterHelper, which would figure out the voxel size you like ( the ones that 'mimics the source'). From the voxel size output of this function(vx, vy, vz), you could use the already existing constructor that's using vx, vy, vz.

But really the main issue I see is how to figure out the optimal number of voxels.

Could we add the option to shift the ResampledSource to the origin? I guess adding a boolean in the constructor that would trigger a Views.zeroMin() at the right place?

I'm not a super fan of this, because any extra option in the constructor needs a modification of the source serialization - breaking upstream stuff.

I'd rather shift the source by creating another source, shifted by the right offset, because this, we can already do.

I'd rather shift the source by creating another source, shifted by the right offset, because this, we can already do.

Using TransformedSource, right?

Using TransformedSource, right?

Yep!

But if we add a String name to the SourceResampler, would that also break your serialization?

But if we add a String name to the SourceResampler, would that also break your serialization?

Yes, but let's do it for this one.

@NicoKiaru I don't understand how to set the voxelDimensions of the ResampledSource. Can you help me there?

The voxelDimensions of the Resampled source are the ones of the model source used for the resampling:

return resamplingModel.getVoxelDimensions();

Usually, the model is created as an EmptySource which voxeldimensions are non optimally specified here:

public VoxelDimensions getVoxelDimensions() {

This EmptySource can be modified for a better support of voxel dimensions. How would you like it to be modified ?

How would you like it to be modified ?

I think we either need to give the voxelDimensions as an optional argument to the constructor of EmptySource or to the ResampledSource. As I am changing the constructor of the ResampledSourcealready anyway to accommodate thename`, I would add it there.

I am thinking now adding it to the constructor of EmptySource makes much more sense as this is the place where one also specifies the other dimension related properties.

@NicoKiaru Ok, for you? (see above)

Perfectly agree

All of this has been done already I think, in 132cd4d and d07a0cb

So I close the issue