Page 3 of 3

Re: Native C module: accessing objects. Now it's crashing...

Posted: Tue Sep 15, 2020 5:32 am
by jimmo
pythoncoder wrote:
Sun Sep 13, 2020 2:20 pm
I don't know if this type conflict is to be expected - the code could be much simplified if it did not occur or if I could subclass the device from my FrameBuffer, using that class throughout.
Yes, I didn't take this into account in my earlier reply... There's just no compile-time way to get hold of the mp_type_framebuf instance.

As you've already pointed out, the issue is that you end up completely duplicating the FrameBuffer type (and all its code). The only thing they have in common is their name.

Unfortunately I wasn't able to repro your crash (unix 64bit, unix 32bit, PYBD_SF6, or PYBD_SF2).

I wonder instead if you could provide "additional helper methods for framebuffers" and have those helper methods do what's necessary to access the internal state of the framebuffers. The key thing is that they need mp_type_framebuf, but potentially that could be part of the API.

Here's a rough sketch of the idea...
- framebuf_utils is a module that provides a method render(type, dest, src, x, y, x, y, fgcol, bgcol)
- type is expected to be FrameBuffer (i.e. you rely on the Python caller to give you the necessary hook to the builtin FrameBuffer. (I think this would also work if FrameBuffer came from a .mpy too).
- dest and src can both be FrameBuffer-derived instances.
- You get access to the set/get pixel functionality by getting it from the type's locals_dict. (Equivalently you could access other methods on FrameBuffer)
- It's not-quite-but-almost-very-nearly as fast as calling setpixel/getpixel directly, and still gets all the format conversion.

Example implementation and test at:
https://github.com/jimmo/micropython/tr ... ebuf_utils

(I'm not sure I'm actually endorsing this idea, but it does seem to work. I'd love to see an officially supported mechanism for this... but I have no idea how that would work!)

Fixed!

Posted: Tue Sep 15, 2020 12:06 pm
by pythoncoder
Thanks for that. A nice solution which solves the type mismatches and so lets me minimise allocations. It works well with my Writer class and nano-gui.

The crashing proved to be a firmware issue - probably this commit on Aug 2nd.

Re: Native C module: accessing objects. Now it's crashing...

Posted: Tue Sep 15, 2020 6:55 pm
by pythoncoder
jimmo wrote:
Tue Sep 15, 2020 5:32 am
...
(I'm not sure I'm actually endorsing this idea, but it does seem to work. I'd love to see an officially supported mechanism for this... but I have no idea how that would work!)
This works extremely well and speeds up rendering by a factor of 5-10 (proportional to the number of pixels in the glyph). Thank you.

On a general point, what is the intended use case for natmod/framebuf?

If the idea is to enable users to add functionality to the framebuf - for example to add methods to draw an ellipse or triangle - the user would encounter the type issues I described. Ideally the display driver would be able to subclass the new framebuf so that this class could be used throughout, but this doesn't seem to work.

Re: Native C module: accessing objects. [SOLVED]

Posted: Tue Sep 15, 2020 11:11 pm
by jimmo
pythoncoder wrote:
Tue Sep 15, 2020 6:55 pm
On a general point, what is the intended use case for natmod/framebuf?
Many projects using MicroPython would never use a FrameBuffer, so from their perspective it's really just taking up space in the firmware. You could say the same about many other features, but modframebuf is particularly easy to separate as it has no dependencies.

So the idea was to demonstrate that you could disable modframebuf in the main build and add it back at runtime.

This will make more sense in the future when hopefully we support loading mpys directly from flash (i.e. runtime freezing) and don't have to pay the RAM penalty.
pythoncoder wrote:
Tue Sep 15, 2020 6:55 pm
If the idea is to enable users to add functionality to the framebuf - for example to add methods to draw an ellipse or triangle - the user would encounter the type issues I described. Ideally the display driver would be able to subclass the new framebuf so that this class could be used throughout, but this doesn't seem to work.
No I don't believe this was the purpose (and as we've discovered, it just makes this hard!).

Although an alternative approach I guess would be to disable framebuf in the main build and use natmod modframebuf to provide any enhancements you like. (Although loading from RAM is an obvious downside).

Subclassing of native types by native types isn't supported (at all right now I think). And especially in the natmod case where the base type isn't available to the compiler.

Right now the natmod feature is really designed for the specific case of "i have a method/class that has no dependencies or interactions with existing firmware features and i want it to be fast".

I don't really see an easy way forward to natmod providing derived implementations of framebuf in a natmod, however a more "official" way of providing helper methods (like the approach I shared above for "render") seems feasible - i.e. if there was access via dynruntime.h for the framebuf type and a less nasty way of invocating methods from its locals_dict, plus maybe a way of adding arbitrary symbols to the type. (This is very much related to the PR linked earlier in the thread for making access to thr ESP IDF possible).

Re: Native C module: accessing objects. [SOLVED]

Posted: Wed Sep 16, 2020 2:17 am
by jimmo
pythoncoder wrote:
Tue Sep 15, 2020 6:55 pm
jimmo wrote: ↑Tue Sep 15, 2020 3:32 pm
...
(I'm not sure I'm actually endorsing this idea, but it does seem to work. I'd love to see an officially supported mechanism for this... but I have no idea how that would work!)

This works extremely well and speeds up rendering by a factor of 5-10 (proportional to the number of pixels in the glyph). Thank you.
I had a second go at this... it's not as fast due to some extra overhead in invoking FrameBuffer.pixel() but it is much less fragile and overall much more in line with the intent of native modules. And using it no longer requires passing in the FrameBuffer type.

https://github.com/jimmo/micropython/tr ... ebuf_utils

I had to add some extra helpers to dynruntime.h which I will send a PR for (but you could just put these at the top of framebuf_utils.c until this is merged -- no change to the MicroPython firmware results from the change to dynruntime.h as the methods were already in the nativeglue struct).

We might be able to improve on the performance of this approach with some future changes to the .mpy ABI.

Re: Native C module: accessing objects. [SOLVED]

Posted: Wed Sep 16, 2020 10:45 am
by pythoncoder
This works fine. Speedup relative to Python is now 4-5, now largely independent of the glyph size. Still a very worthwhile improvement.

Re: Native C module: accessing objects. [SOLVED]

Posted: Fri Sep 18, 2020 4:43 pm
by pythoncoder
jimmo wrote:
Wed Sep 16, 2020 2:17 am
...
I had a second go at this... it's not as fast due to some extra overhead in invoking FrameBuffer.pixel() but it is much less fragile and overall much more in line with the intent of native modules. And using it no longer requires passing in the FrameBuffer type...
While this works well, before advocating it do you think a native module is optimal?

Rendering a monochrome graphic onto a color space is a commonplace requirement with applications beyond glyphs. I think there is a case for adding a new method to modframebuf.c:

FrameBuffer.render(src, x, y, fgcolor, bgcolor=0)

Note this offer of a PR modifying blit was not taken up. A new method name avoids any change to the blit call signature which may be subject to change if someone solves the general problem of blitting between arbitrary color spaces.

On the other hand the MicroPython way is to be to wait as many years as it takes for a perfect solution rather than implement a subset. Maybe a native module is the way forward - but I thought I'd ask ;)

Re: Native C module: accessing objects. [SOLVED]

Posted: Sat Sep 19, 2020 3:42 am
by jimmo
pythoncoder wrote:
Fri Sep 18, 2020 4:43 pm
While this works well, before advocating it do you think a native module is optimal?
I'm pretty happy with this approach _in general_ and would probably advocate for it. However, for this specific case, nah you're absolutely right this should be built-in.

As I was playing with implementing this, I just kept thinking that it should be a standard feature of framebuf.

TBH I like the palette-style approach suggested in the referenced bug, and then implemented in https://github.com/micropython/micropython/pull/3238 makes sense to me, although I agree with Damien's suggestion regarding how to represent the palette.

While we're at it, I'd also like to see a "scale" argument to blit too :)