Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source transform for missing timepoints? #140

Open
tischi opened this issue Jun 8, 2022 · 5 comments
Open

Source transform for missing timepoints? #140

tischi opened this issue Jun 8, 2022 · 5 comments

Comments

@tischi
Copy link
Contributor

tischi commented Jun 8, 2022

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
Copy link
Member

That is a bug,..

@tpietzsch
Copy link
Member

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
Copy link
Contributor Author

tischi commented Jun 8, 2022

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
Copy link
Contributor Author

tischi commented Jun 8, 2022

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
Copy link
Contributor Author

tischi commented Jun 23, 2022

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants