TimePointListener - not the same API for BVV and BDV

[NicoKiaru]

Hi @tpietzsch,

I'm trying to simplify a bit bdv-playground by using the AbstractViewerPanel interface instead of my own ViewerAdapter. That dates from the time when BdvHandle and BvvHandle did not implement this interface.

It mostly works, except for timepoint listeners. Indeed, the access to timepointListeners is not part of AbstractViewerPanel interface. And there is currently a difference between the way to add a timepoint listener between bvv and bdv:

			BdvHandle bdvh = null;
			bdvh.getViewerPanel().timePointListeners().add(listener);
			
			BvvHandle bvvh = null;
			bvvh.getViewerPanel().addTimePointListener(listener);
			
			AbstractViewerPanel avp = null;
			avp.timepointListeners does not exist

Do you think you could homogeneize this ? I can try to do a PR if you want.

[NicoKiaru]

Ah, I found another thing: the getConverterSetups method exist in both BdvHandle and BvvHandle, but is not part of AbstractViewerPanel interface. Is it doable to add it ?

[NicoKiaru]

Ah, actually the convertersetup method is more problematic because it is accessible from a BdvHandle object in Bdv and in the viewer panel for Bvv.

BdvHandle bdvh;
bdvh.getConverterSetups();

BvvHandle bvvh;
bvvh.getViewerPanel().getConverterSetups()

So I can't unfortunately simplify my code because I don't see a way to get the convertersetups from the ViewerPanel of BDV (while I can for bvv).

[NicoKiaru]

@tpietzsch do you think it's possible to harmonize bvv and bdv regarding the converter setups access ?

[tpietzsch]

I just looked, and there is BvvHandle.getConverterSetups()?

bigvolumeviewer-core/src/main/java/bvv/vistools/BvvHandle.java

Lines 109 to 112 in 86c89d9

	public ConverterSetups getConverterSetups()
	{
	return setups;
	}

Or is there another problem.

About the timepoint listeners:
It would be great to harmonize this, I agree. If you want to make a PR to move timePointListeners() into AbtractViewerPanel, that would be great 👍

[tpietzsch]

Argh, I didn't read everything carefully enough. Sorry.
Yes, it would also be good to just put ConverterSetups into the AbstractViewerPanel.
Probably for the time being, I would just add getConverterSetups() as a abstract method. To initialize the ConverterSetups we need a bdv.viewer.ViewerState, and I just saw that BVV still uses the @Deprecated old bdv.viewer.state.ViewerState. I'll see whether I can work on it a little bit in the next few days.

[NicoKiaru]

Hi Tobias,

Thanks for the fast answer! I can try to work on the PR if you think it's still worth, but if you think it's an extra 2 min work for you when you tackle the rest, let me know!

Cheers

[tpietzsch]

Yes, would be great if you work on a PR 👍

[NicoKiaru]

Ok, so I've made (full disclaimer assisted by Claude) 2 PRs:

• bigdataviewer-core: adds timePointListeners() and getConverterSetups() to AbstractViewerPanel, moves ConverterSetups ownership into ViewerPanel
• bigvolumeviewer-core: implements timePointListeners() in VolumeViewerPanel, deprecates the old add/remove methods

With these 2 PRs I managed to remove my ViewerAdapter class (https://github.com/bigdataviewer/bigdataviewer-playground/tree/rmvieweradapter), and everything I've tested works well. I can now deal with a bvv or a bdv viewer the same way! 🎉

[tpietzsch]

Great! Thank you!

I pushed more updates to both your PR branches (cleaning up and moving VolumeViewerPanel internals closer to ViewerPanel).
Would you mind testing whether this still works for you?

[tpietzsch]

@NicoKiaru did you try it? does it work? If so, I would merge and release ...

[NicoKiaru]

yep, I just took the time to test and, as far as I can tell from various demos with bdv(s), bvv(s), synced or not, it works nicely!

[tpietzsch]

Thanks!
I merged the PRs and released bigdataviewer-core-10.6.8 and bigvolumeviewer-0.4.2.