extmod/modframebuf Enable blit between different formats. #7666
Conversation
There was a problem hiding this comment.
Thanks @peterhinch, this is a really good enhancement, and only +40 bytes on PYBV11.
(I've replied to #7665 regarding your question about fixing the PR diffs)
| mp_obj_framebuf_t *palette = MP_OBJ_NULL; | ||
| if (n_args > 5 && args[5] != mp_const_none) { | ||
| mp_obj_t palette_in = mp_obj_cast_to_native_base(args[5], MP_OBJ_FROM_PTR(&mp_type_framebuf)); | ||
| palette = MP_OBJ_TO_PTR(palette_in); |
There was a problem hiding this comment.
mp_obj_cast_to_native_base can return mp_const_none, but that's not the same as MP_OBJ_NULL.
So needs to be
if (palette_in != mp_const_none) {
palette = MP_OBJ_TO_PTR(palette_in);
}in order to make the code below work.
There was a problem hiding this comment.
Sorry if I'm being dumb here but I'm afraid I don't understand this.
This check worked if I didn't pass palette or I explicitly passed a palette value None:
if (n_args > 5 && args[5] != mp_const_none)Are you saying that I need an additional check after the above, or that the above check should only be on n_args and be followed by this subsequent check? I'm not clear about the circumstances in which palette_in is mp_const_none.
if (palette_in != mp_const_none) {
palette = MP_OBJ_TO_PTR(palette_in);
}There was a problem hiding this comment.
The case that this matters for is if palette is non-None but isn't derived from FrameBuffer.
But I said completely the wrong thing -- mp_obj_cast_to_native_base does actually return MP_OBJ_NULL in this case. Which means the code as you have it is exactly correct. Sorry!
There was a problem hiding this comment.
Ah! That's fortuitous as I hadn't considered that case :)
14e5525 to
793f902
Compare
|
This is superseded by #7682 which I believe now contains only the correct files. |
…n-main Translations update from Hosted Weblate
This achieves a substantial performance improvement when rendering glyphs to color displays, the benefit increasing proportional to the number of pixels in the glyph. Its inclusion would enable me to improve the performance of most of my GUI repos. Please see
framebuf.rstfor details.@jimmo This has a similar aim to the dynamic C module you wrote, but is more general, faster and portable (I never managed to successfully build your module for anything other than STM).
My GitFu has failed me here and I seem to have a lot of cruft included with this PR. Any guidance as to how to fix this would be much appreciated.