Open bablokb opened 4 months ago
@bablokb the branch new_blinka_displayio_no_seperate_thread
is where the most recent work is at: https://github.com/FoamyGuy/Blinka_Displayio_PyGameDisplay/tree/new_blinka_displayio_no_seperate_thread.
I need to dive back into it again to be certain of the exact status of the work. To the best of my recollection when I last set it down it is functioning successfully but with some rather large stipulations:
displayio
must be imported after blinka_displayio_pygamedisplay
or else the execution seems to "freeze" for a few seconds during one of the imports. My best guess is very vaguely this is "something to do with threading" but I've never really seen anything like it before where import order seems to have such a large impact on behavior. With the absolute newest commit(s) I started trying to remove threading from this library (because it now exists inside of Blinka DisplayIO, and I was guessing that both trying to utilize it was not playing nicely together)refresh()
refresh work, then auto_refresh being enabled doesn't work. Or vice versa with some changes to this library I did manage to make auto_refresh
work successfully but then disabling it and calling the manual refresh()
wasn't working. I will try to get my hands back into this a bit to try to be more confident and accurate about the current status and leave a new comment here if / when I have any additional details or corrections.
I definitely would appreciate any help that you (or anyone else) wants to offer in getting this library updated to support the newer Blinka_DisplayIO.
small update:
Right now the manual refreshing by having user code call refresh()
is working and auto_refresh=True
does not work.
I've also noticed when you close with the window X button this exception is raised:
Traceback (most recent call last):
File "/home/timc/repos/circuitpython/Blinka_Displayio_PyGameDisplay/examples/blinka_displayio_pygamedisplay_scrolling_label.py", line 39, in <module>
if display.check_quit():
File "/home/timc/repos/circuitpython/Blinka_Displayio_PyGameDisplay/blinka_displayio_pygamedisplay.py", line 275, in check_quit
self._pygame_display_tevent.set()
AttributeError: 'PyGameDisplay' object has no attribute '_pygame_display_tevent'. Did you mean: '_pygame_display_thread'?
But the window still closes so it's not necessarily a critical issue.
I think if we can get the refreshing fixed so that both auto_refresh=True
and manual refresh()
are working I'd be willing to make a new release at that point.
Ideally I'd also love to have the import order thing solved too, but it's a bit more mysterious and for the time being I could be persuaded to just add a note to the docs that import order matters and explain the symptoms if done out of order (but would still like a better solution in the long run.)
I will have a look. But I am sure we do need a separate thread, simply because the Pygame window is one window of many on the system. It has to react to things like redraw-events (users open and close other windows or users move the Pygame-window and so on) or mouse-over events. If the Pygame-loop does handle or discard all these events on a timely basis, the event-queue of Pygame will fill up. All this stuff is not CircuitPython related, so you cannot deal with this from displayio side.
The main challenge with all of this is that a Pygame-program has to be a Pygame-program. Sounds trivial, but at the core this implies you have a main event-loop and all other things are handled by callbacks (limited to short runtime) and other threads. And it also implies that the Pygame program is not the sole program running and that it is not in control of all hardware resources.
The architecture of CircuitPython programs is different, because it is the single program running with full-control of the hardware.
I found some time looking at the code. I don't think this is ready yet. E.g. PygameDisplay
inherits from displayio.Display
which does not exist anymore. busdisplay.Display
also is no good choice for a parent class, since we are not on a bus.
I wonder if it would make more sense to first port framebufferio
to Blinka and then treat PygameDisplay
as a framebuffer-display. The pygame image-surface is in fact a framebuffer. So this would probably simplify the code because most of the handling will be in framebufferio
.
@bablokb My primary usages are all around displayio specifically, I'm not opposed to having a framebufferio option, but it's not likely to be one that I will develop or keep testing / maintaining.
I believe that busdevice.Display
is the best choice for a parent class. I do agree that it feels wrong since we aren't actually using a Bus, but this was always the case with this library in the past. The old displayio.Display
is basically the same as the new busdisplay.Display
It was moved and given a more specific name, and perhaps a few other internal tweaks, but essentially the latter is just the new / current version of the former.
For this library we basically will just have None / null for the Bus and keep only code that is needed for our real use-case of rendering into the pygame engine. Anything that deals with the Bus that isn't strictly necessary for us can (and should) be cut out.
@bablokb if you're still interested in this module. I've been giving it some thought in the back of my mind and I think I have an idea for an approach that could work to make this library support the newer Blinka_DisplayIO versions.
Currently the PyGameDisplay class is extending displayio.Display (or busdisplay.BusDisplay if it were the new API), which means that it's getting all of the behaviors associated with that parent class. It's then overriding some of the built-in functions in order to "hijack" the refresh action and send pixel information into the PyGame window. Part of the trouble that I was having during my most recent attempts was around the fact the newer versions of Blinka_DisplayIO introduced some internally used threading for it's refresh loop and the internal threading in Blinka_DisplayIO was not "playing nicely" with the threading which was in PyGameDisplay.
I am now thinking maybe that PyGameDisplay should not actually extend Display (or BusDisplay) at all and instead could just be a "plain" object that doesn't extend anything. Then we will implement only the functions of Display that are necessary for drawing to PyGame and leave out anything that isn't including the internal refresh loop thread. It might result in some duplicated code between PyGameDisplay and Blinka_DisplayIO if we end up needing to copy/paste some functionality since we no longer will get it from extending. But it will mean that we shouldn't have any problems with any sort of threading or anything else that is internal to Blinka_DisplayIO.
I think the PyGameDisplay module will still end up depending on the Blinka_DisplayIO module because it will want access to things like Bitmap, Group, Palette etc.. in order to process the refresh function. But it just won't extend the Display class.
If this is something you're still interested in does this sound like it might be a workable solution to you?
I am still interested in this module. And from what you write, I would say it does not differ much from what I proposed. If you look at framebufferio you will see it it contains already all the logic you want to duplicate (for "drawing to PyGame").
Of course you can skip framebufferio and use Pygame primitives to implement the rendering of the root_group and all of it's children. Might be a tad more efficient, but since we are running on a "real" computer, that should not really matter. Although more complex, first porting framebufferio is a more strategic option: it would add support for other displays as well.
The threading problem is a challenge, but I have done enough multi-threading applications that I am confident that we will find a solution. And as I explained above: you won't get far without threading on a multi-window desktop system, because the main thread has to be reactive. It is a mantra on all systems (Windows, Linux, Apple and so on) that you have to do everything time consuming in a separate thread so that the main thread can react to redraws and other standard window-events.
It seems there is no progress on porting this to the new Blinka_IO. Which is a pity, since the currently supported version of Blinka_IO does not have e.g.
vectorio
.I use this library very often and would be willing to invest some time. I can see there are additional branches in this repo, can I start working from there? Also, could you give me a list of things that don't work yet?