vispy / jupyter_rfb

Remote Frame Buffer for Jupyter
https://jupyter-rfb.readthedocs.io
MIT License
58 stars 10 forks source link

API change for sending frames #18

Closed almarklein closed 3 years ago

almarklein commented 3 years ago

Right now the widget has the send_frame() method. If you're lucky the widget can send the frame away directly. Otherwise the frame is queued ... or dropped.

I propose to change this to an on_draw() method that automatically gets called when the widget is ready to send a new frame.

However, there is also the server who may not have anything new to draw. So I think we need an update() or request_draw method too.

djhoese commented 3 years ago

I definitely having been keeping up with this development, but the user is calling send_frame() right? Who is calling on_draw() (you say it is automatically called)? Do both send_frame and on_draw exist?

How does this compare with other tools like this and with vispy desktop backends? Isn't Qt's event loop blocking for draws? How do your proposed changes differ from behavior like that?

almarklein commented 3 years ago

Do both send_frame and on_draw exist?

No, the send_frame() method would be removed.

Things would actually be more like gui toolkits like Qt, where you ask for a redraw (using .update() in qt), and the redraw will be scheduled at a moment that Qt thinks is best. In a similar way, a redraw can be requested for our widget, and it can execute the draw (call on_draw) when it has received confirmation (from the client) of the nth earlier frame being shown. In effect, if the frame rate drops because of the client or connection, the frames are also generated at a lower pace.

In other words, the throttling is extended to the actual (custom) generation of the image, instead of only the sending part (and dropping frames if necessary).

djhoese commented 3 years ago

I like it.

Would it maybe be good to include both? Kind of an asynchronous and a synchronous solution to sending frames? May be helpful to others in the future. If it feels wrong to do it then don't. Just an idea.

almarklein commented 3 years ago

I don't want to include both solutions, because it makes the widget and API more complicated. I did add an example notebook that shows how you can easily implement send_frame in a subclass.