search

rust-embedded-community / ssd1306

SSD1306 OLED driver
Apache License 2.0
300 stars 68 forks source link

Incorrect min_y? #122

Closed bugadani closed 4 years ago

bugadani commented 4 years ago

Based on this metric I believe the linked line may be incorrect.

https://github.com/jamwaffles/ssd1306/blob/96170fd21419e73fdb2299f395b07836d52d090f/src/mode/graphics.rs#L161

If that's supposed to be height, I'm happy to fix it, otherwise please provide an explanation why it's width.

jamwaffles commented 4 years ago

Yeah that definitely looks like it should be height - 1. There's a similar block of code here, so it might be worth moving the min/max reset into a private method.

bugadani commented 4 years ago

I haven't given it much thought, but shouldn't these be rotation aware values? Is min_y the same when rotated by 90 or 270 degrees?

therealprof commented 4 years ago

Yes, there're the other variables which take the rotation into account. However it's a bit weird to have the manual comparison in the code. There're max() and min() functions for that.

bugadani commented 4 years ago

There's a similar block of code here, so it might be worth moving the min/max reset into a private method.

You mislead me :) That block is marking the whole display to be updated, the original block resets the area to nothing.

jamwaffles commented 4 years ago

Oop yes you're right, min/max are swapped there. My bad!