bigdataviewer / bigdataviewer-core

ImgLib2-based viewer for registered SPIM stacks and more
BSD 2-Clause "Simplified" License
33 stars 35 forks source link

Source transform for missing timepoints? #140

Open tischi opened 2 years ago

tischi commented 2 years ago

Even though my Source returns false for isPresent( 0 ), the below code still tries to get the source transform at time point 0. And in my case crashed because my Source only has source transforms for existing time points.

    at bdv.tools.transformation.TransformedSource.getSourceTransform(TransformedSource.java:239)
    at bdv.viewer.overlay.ScaleBarOverlayRenderer.setViewerState(ScaleBarOverlayRenderer.java:153)

Question: Should a Source provide a source transform also for missing time points or is that maybe a bug in BDV?

tpietzsch commented 2 years ago

That is a bug,..

tpietzsch commented 2 years ago

Hmm, actually...

I don't see how this could be a problem. The method in Source is

void getSourceTransform( int t, int level, AffineTransform3D transform );

So you already get the pre-allocated transform. So if you just do nothing if isPresent(t)==false all should be fine.

I'll probably change ScaleBarOverlayRenderer to handle isPresent()==false as a separate case. But I think it also wouldn't hurt if Source implementations were robust to non-existing t (and/or level) arguments

tischi commented 2 years ago

So you already get the pre-allocated transform. So if you just do nothing if isPresent(t)==false all should be fine.

Yeah, good point, we could do this... Whether "all should be fine" though depends on the consumer of this function :-) For example, depending on the implementation the scale bar may be all over the place at non-existing time points.

tischi commented 2 years ago

What I mean is: if something calls this function and interprets the transform as the actual sourceTransform at that time point then it would lead to a wrong result.

But I get your point: we could say it is the responsibility of the calling code to check whether this time point is present, before calling that function.

tischi commented 2 years ago

@tpietzsch I tried your suggestion of just returning the pre-allocated transform for missing time-points, but it looks like as if the current implementation in BDV does not handle this correctly. I get weird behavior: When hovering with the mouse across BDV the scale bar is changing.