rust-embedded-community / ssd1306

SSD1306 OLED driver
Apache License 2.0
283 stars 69 forks source link

Clean up graphics mode #123

Closed bugadani closed 4 years ago

bugadani commented 4 years ago

Extract resetting of the dirty area - use constants, since the actual values are irrelevant, only their relation is important for flush to return early

Remove redundant checks and min/max calls - assuming width/height is always divisible by 8, set_pixel will make sure max_x and max_y is in the valid range

Hi! Thank you for helping out with SSD1306 development! Please:

PR description

This needs some double checking because my understanding may be incorrect.

Closes #122 but that is not a bug; may result in a negligible amount of perf gain (some checks per frame).

jamwaffles commented 4 years ago

I haven't got to the bottom of exactly why yet, but the rtfm_dvd example crashes when the image touches the right hand side of the display. This doesn't occur in master so I'm assuming there's an overflow error introduced in this branch somewhere.

bugadani commented 4 years ago

Well, that means my logic was incorrect somewhere. Thanks, I'll check that. In the mean time, this gets demoted to draft :)

bugadani commented 4 years ago

The collision detection in rtfm_dvd triggers when the right edge of the image is outside of the display area, and that crashes because max_x (or max_y) can get larger than the width/height of the screen. This is because I removed the min calls in flush() and set_pixel does not validate the coordinates - this also means the rendered image could be incorrect in that case, even if I reintroduce the min calls.

Basically, currently it's permitted to draw outside of the screen, as long as it's addressing into the display buffer, so it gets wrapped into a different part of the image. This could be justified for performance reasons IMO, and handled by not trying to draw there in the first place, but that's just my .02€.

bugadani commented 4 years ago

I've reintroduced clipping the display area to its physical size, but the possibility of incorrect drawing is still there in set_pixel.

bugadani commented 4 years ago

I'm closing this in favour of #125 and #124