Skip to content

Commit

Permalink
extmod/modframebuf: Fix FrameBuffer size check for stride corner-cases.
Browse files Browse the repository at this point in the history 
This is a fix for issue micropython#15944, and handles corner cases in the FrameBuffer
code when using stride values where the last line's stride may extend past
the end of the underlying buffer.  This commit includes extra tests for
these corner cases.

For example a GS8 format FrameBuffer with a width of 8, height of 2 and
stride of 10 should be able to fit into a buffer of size 18 (10 bytes for
the first horizontal line, and 8 bytes for the second -- the full 10 bytes
are not needed).

Similarly a 1 by 9 FrameBuffer in MONO_VLSB format with a stride of 10
should be able to fit into a buffer of length 11 (10 bytes for the first
8 lines, and then one byte for the 9th line.

Being able to do this is particularly important when cropping the corner of
an existing FrameBuffer, either to copy a sprite or to clip drawing.

Signed-off-by: Corran Webster <[email protected]>
  • Loading branch information
@corranwebster @dpgeorge
corranwebster authored and dpgeorge committed Oct 22, 2024
1 parent 7ed480f commit d1574de
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 2 deletions.
10 changes: 8 additions & 2 deletions extmod/modframebuf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,23 +282,29 @@ static mp_obj_t framebuf_make_new(const mp_obj_type_t *type, size_t n_args, size
mp_raise_ValueError(NULL);
}

size_t height_required = height;
size_t bpp = 1;
size_t height_required = height;
size_t width_required = width;
size_t strides_required = height - 1;

switch (format) {
case FRAMEBUF_MVLSB:
height_required = (height + 7) & ~7;
strides_required = height_required - 8;
break;
case FRAMEBUF_MHLSB:
case FRAMEBUF_MHMSB:
stride = (stride + 7) & ~7;
width_required = (width + 7) & ~7;
break;
case FRAMEBUF_GS2_HMSB:
stride = (stride + 3) & ~3;
width_required = (width + 3) & ~3;
bpp = 2;
break;
case FRAMEBUF_GS4_HMSB:
stride = (stride + 1) & ~1;
width_required = (width + 1) & ~1;
bpp = 4;
break;
case FRAMEBUF_GS8:
Expand All @@ -314,7 +320,7 @@ static mp_obj_t framebuf_make_new(const mp_obj_type_t *type, size_t n_args, size
mp_buffer_info_t bufinfo;
mp_get_buffer_raise(args_in[0], &bufinfo, MP_BUFFER_WRITE);

if (height_required * stride * bpp / 8 > bufinfo.len) {
if ((strides_required * stride + (height_required - strides_required) * width_required) * bpp / 8 > bufinfo.len) {
mp_raise_ValueError(NULL);
}

Expand Down
30 changes: 30 additions & 0 deletions tests/extmod/framebuf_bounds.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,25 @@ def test_constructor(*args):
test_constructor(bytearray(20), 10, 10, framebuf.MONO_HLSB, 10)
test_constructor(bytearray(20), 10, 10, framebuf.MONO_HLSB, 9)
test_constructor(bytearray(20), 10, 10, framebuf.MONO_HLSB, -1)
test_constructor(bytearray(20), 10, 10, framebuf.MONO_HLSB, 16)
test_constructor(bytearray(20), 10, 10, framebuf.MONO_HLSB, 17)
test_constructor(bytearray(19), 8, 10, framebuf.MONO_HLSB, 16)
test_constructor(bytearray(18), 8, 10, framebuf.MONO_HLSB, 9)
test_constructor(bytearray(19), 8, 10, framebuf.MONO_HLSB, 16)

print(framebuf.MONO_VLSB)
test_constructor(bytearray(8), 8, 1, framebuf.MONO_VLSB)
test_constructor(bytearray(7), 8, 1, framebuf.MONO_VLSB)
test_constructor(bytearray(8), 8, 8, framebuf.MONO_VLSB)
test_constructor(bytearray(1), 1, 8, framebuf.MONO_VLSB)
test_constructor(bytearray(1), 1, 9, framebuf.MONO_VLSB)
test_constructor(bytearray(2), 1, 9, framebuf.MONO_VLSB)
test_constructor(bytearray(1), 1, 8, framebuf.MONO_VLSB, 10)
test_constructor(bytearray(8), 8, 8, framebuf.MONO_VLSB, 10)
test_constructor(bytearray(8), 8, 8, framebuf.MONO_VLSB, 7)
test_constructor(bytearray(11), 1, 9, framebuf.MONO_VLSB, 10)
test_constructor(bytearray(10), 1, 9, framebuf.MONO_VLSB, 10)
test_constructor(bytearray(11), 1, 16, framebuf.MONO_VLSB, 10)

for f in (framebuf.MONO_HLSB, framebuf.MONO_HMSB):
print(f)
Expand All @@ -40,21 +54,37 @@ def test_constructor(*args):
test_constructor(bytearray(15), 5, 8, framebuf.GS2_HMSB)
test_constructor(bytearray(16), 5, 8, framebuf.GS2_HMSB)
test_constructor(bytearray(9), 4, 9, framebuf.GS2_HMSB)
test_constructor(bytearray(2), 7, 1, framebuf.GS2_HMSB, 9)
test_constructor(bytearray(4), 7, 2, framebuf.GS2_HMSB, 9)
test_constructor(bytearray(5), 7, 2, framebuf.GS2_HMSB, 9)

print(framebuf.GS4_HMSB)
test_constructor(bytearray(8), 2, 8, framebuf.GS4_HMSB)
test_constructor(bytearray(15), 3, 8, framebuf.GS4_HMSB)
test_constructor(bytearray(16), 3, 8, framebuf.GS4_HMSB)
test_constructor(bytearray(9), 2, 9, framebuf.GS4_HMSB)
test_constructor(bytearray(4), 7, 1, framebuf.GS4_HMSB, 9)
test_constructor(bytearray(8), 7, 2, framebuf.GS4_HMSB, 9)
test_constructor(bytearray(9), 7, 2, framebuf.GS4_HMSB, 9)

print(framebuf.GS8)
test_constructor(bytearray(63), 8, 8, framebuf.GS8)
test_constructor(bytearray(64), 8, 8, framebuf.GS8)
test_constructor(bytearray(64), 9, 8, framebuf.GS8)
test_constructor(bytearray(64), 8, 9, framebuf.GS8)
test_constructor(bytearray(64), 4, 8, framebuf.GS8, 8)
test_constructor(bytearray(64), 4, 8, framebuf.GS8, 9)
test_constructor(bytearray(8), 8, 1, framebuf.GS8, 10)
test_constructor(bytearray(17), 8, 2, framebuf.GS8, 10)
test_constructor(bytearray(18), 8, 2, framebuf.GS8, 10)

print(framebuf.RGB565)
test_constructor(bytearray(127), 8, 8, framebuf.RGB565)
test_constructor(bytearray(128), 8, 8, framebuf.RGB565)
test_constructor(bytearray(128), 9, 8, framebuf.RGB565)
test_constructor(bytearray(128), 8, 9, framebuf.RGB565)
test_constructor(bytearray(128), 4, 8, framebuf.RGB565, 8)
test_constructor(bytearray(128), 4, 8, framebuf.RGB565, 9)
test_constructor(bytearray(16), 8, 1, framebuf.RGB565, 10)
test_constructor(bytearray(34), 8, 2, framebuf.RGB565, 10)
test_constructor(bytearray(36), 8, 2, framebuf.RGB565, 10)
30 changes: 30 additions & 0 deletions tests/extmod/framebuf_bounds.py.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,24 @@ Valid
Valid
ValueError
ValueError
Valid
ValueError
Valid
ValueError
Valid
0
Valid
ValueError
Valid
Valid
ValueError
Valid
Valid
Valid
ValueError
Valid
ValueError
Valid
3
Valid
ValueError
Expand All @@ -27,18 +41,34 @@ Valid
ValueError
Valid
Valid
Valid
ValueError
Valid
2
Valid
ValueError
Valid
Valid
Valid
ValueError
Valid
6
ValueError
Valid
ValueError
ValueError
Valid
ValueError
Valid
ValueError
Valid
1
ValueError
Valid
ValueError
ValueError
Valid
ValueError
Valid
ValueError
Valid

0 comments on commit d1574de

Please sign in to comment.