Allow multiple callback to View#onBeforeRender/onAfterRender

[zarov]

Suggestion

We can improve the preRender/onAfterRender mechanism by allowing more than one callback per property.

Context

There are only two examples for now where this is used (see below). The two of them are overloading in each case the preRender/onAfterRender properties of a View, which is inconvenient in case we have things already set in those properties (like in GlobeView with the preRender property).

• https://github.com/iTowns/itowns/blob/76ae961cbaffb0736ce5babaf27e0f2a34742985/examples/layersColorVisible.html#L93-L103 (note that the preRender here needs to be changed in onBeforeRender)
• https://github.com/iTowns/itowns/blob/d919316900e3f209555525483f08702127701ec6/examples/globe.js#L39-L48

Solution

• Modify the View to have two arrays, one for onBeforeRender and the other for onAfterRender.
• Call every these arrays in MainLoop, with view.onBeforeRender.forEach((fn) => fn.call(this)) or something like this

Note that this solved https://github.com/iTowns/itowns/blob/master/src/Core/Prefab/GlobeView.js#L247-L249

WDYT ?

[autra]

Even though I agree these callbacks are totally inconvenient, let's not jump too soon to technical discussion. You didn't actually explained your use case, and with so little context, it's difficult to know what would fit you best.

What problem are you trying to solve (in a general, functional fashion)? Why do you think preRender / afterRender are a good technical solution for your problem? Can you provide real-life example (ideally with the functions you want to register in these hook)?

Maybe we want several hook points for a FrameRequester instead? Or maybe we just want to have some before-render and after-render events dispatched instead?

But yeah, about these callbacks, you're totally right, we need to do something about these. If I have to make a wild guess, I think the former (several hook points for frame requesters) would be a good solution. It's been several times already that we tell ourselves we need this.

In the current state of the code, frameRequesters are conceptually preUpdate callbacks. So it'd be easy to extend this concept to have preRender, postRender etc... callbacks. We could register them this way:

view.addFrameRequester('preRender', cb1);
view.addFrameRequester('postRender', cb2);

And they'd get called in the corresponding phase, with sensible arguments.

WDYT? Would you feel comfortable having a try at it?

[zarov]

You didn't actually explained your use case, and with so little context, it's difficult to know what would fit you best.

This is my usecase. If you wait for everything to load and then you zoom in, you can see a change in position. This is due to the fact that preRender is overloaded here.

What problem are you trying to solve (in a general, functional fashion)? Why do you think preRender / afterRender are a good technical solution for your problem? Can you provide real-life example (ideally with the functions you want to register in these hook)?

See the quoted use case above. It seems there may be use cases where a user (or even us, internally) would like to perform some actions on a view before or after the rendering phase. We can imagine (as the example above) some change on the interface, or some pre processing of some data, or post processing effect (I'm personally not a big fan of the implementation of the postprocessing example).

Maybe we want several hook points for a FrameRequester instead? Or maybe we just want to have some before-render and after-render events dispatched instead?

Indeed, I didn't see the FrameRequester object. I'm good with events too, but at the same time isn't it overkill ? I honestly prefer FrameRequester object.

In the current state of the code, frameRequesters are conceptually preUpdate callbacks. So it'd be easy to extend this concept to have preRender, postRender etc... callbacks. We could register them this way: view.addFrameRequester('preRender', cb1); view.addFrameRequester('postRender', cb2); And they'd get called in the corresponding phase, with sensible arguments.

Yeah I find this solution prettier, I'm having a look at it !

Note that I originally stayed with onBeforeRender/onAfterRender to be close to the three.js syntax.

[zarov]

I did a follow-up in #591

[zarov]

Fixed with #591