lexus2k / ssd1306

Driver for SSD1306, SSD1331, SSD1351, IL9163, ILI9341, ST7735, PCD8544, Nokia 5110 displays running on Arduino/ESP32/Linux (Rasperry) platforms
MIT License
655 stars 125 forks source link

FreeFont -> ssd1306_print8() -> ssd1306_write8-> line break based on s_fixedFont.h.width #90

Open RoboDurden opened 4 years ago

RoboDurden commented 4 years ago

Describe the bug a freefont and ssd1306_print8 calls ssd1306_write8 which uses fixedfont-width to do line break :-(

size_t ssd1306_write8(uint8_t ch)
{
...
    else if ( (ssd1306_cursorX > ssd1306_lcd.width - s_fixedFont.h.width) || (ch == '\n') )

Please complete the following information:

how to fix if moved this to the beginning of the function:

    uint16_t unicode = ssd1306_unicode16FromUtf8(ch);
    if (unicode == SSD1306_MORE_CHARS_REQUIRED) return 0;
    SCharInfo char_info;
    ssd1306_getCharBitmap(unicode, &char_info);

and changed the new-line test to else if ( (ssd1306_cursorX + char_info.width > ssd1306_lcd.width) || (ch == '\n') )

as the fillRect includes the following spacing i had to change it to

        uint16_t x2 = ssd1306_cursorX + char_info.width + char_info.spacing;
        if (x2 >  ssd1306_lcd.width)    x2 =  ssd1306_lcd.width;
        ssd1306_fillRect8( ssd1306_cursorX, ssd1306_cursorY,
                    x2-1,
                    ssd1306_cursorY + s_fixedFont.h.height - 1 );
lexus2k commented 4 years ago

Hi. Ok. I see what you're talking about. I need to fix this for all types of displays, supported. Thank you

lexus2k commented 4 years ago

Please, check the changes I committed per this issue report (2094445)

RoboDurden commented 4 years ago

I do not like your unreadable gotoNewLine code ONLY to skip the extensive ssd1306_getCharBitmap() for the rare cases of a '\n' or a '\r'. Part of ssd1306_getCharBitmap would not need to be computed again and again for every char printed. So if speed is your concern, there are better ways to go. But as you refuse a nice object oriented api anyway, beauty has no priority here :-/

Do you want me to test the 1.7_dev branch on my ST7735 display before pushing it to master ?

RoboDurden commented 4 years ago

By the way, your lib seems to have great problems when a pixel outside the display size is to be drawn. With ssd1306_drawRect8 and ssd1306_drawLine8 i only need to draw to 1 pixel > display_width and the rect is drawn may pixels more in y+ direction. A drawLine needs several milliseonds because instead of drawing to y=127 it draws to something like y=56322. There might be some uint16_t overflows. This is not a bug report. Have no code to reproduce yet. But you might already be aware of these problems.

lexus2k commented 4 years ago

By the way, your lib seems to have great problems when a pixel outside the display size is to be drawn.

Direct draw API was never intended to do something with pixels outside to save the flash space and speed up the process. It is ok, when big controllers are used, like ARM Cortex-M based, Extensa arch, etc., but as for AVR especially for small attiny ones, any additional code is killing. That's why is up to application to do some checks.

But as you refuse a nice object oriented api anyway, beauty has no priority here :-/

v2.X has something to do with the beauty problem, and originally I started it to get advantages of C++ language (to use templates to reduce code size even more), but after some work on the new library version, I discovered that C++ code still eats more flash, because of using objects; and virtual methods are killer feature for the library (this is how compilers work with virtual methods). So, I think that you're agree - object oriented api is not always the best solution. That's why v2.X is still in development.

I do not like your unreadable gotoNewLine code ONLY to skip the extensive ssd1306_getCharBitmap() for the rare cases of a '\n' or a '\r'. Part of ssd1306_getCharBitmap would not need to be computed again and again for every char printed. So if speed is your concern, there are better ways to go.

As for speed concern, in the code provided above, calling ssd1306_unicode16FromUtf8() ssd1306_getCharBitmap() (at the beginning of write8) for '\r' and '\n' chars leads to more CPU resources consumption. Also, in case of calling ssd1306_drawMonoBitmap8() for '\n' and '\r' chars you need always to remember that ssd1306_getCharBitmap() needs to return zero height/width for them, and ssd1306_drawMonoBitmap8() should correctly process zero-size objects. This seems to me more complicated.

RoboDurden commented 4 years ago

i am not really interested in such low level programming so i am no expert. But maybe you could use templates to have object like interface but still stick to pure c. That is a template i stumbled over and added CRC:

template <typename O,typename T> unsigned int SerialWrite (O& oSerial, const T& o)
{
    oSerial.write((byte *) &o, sizeof (o));
    byte iCRC8 = CRC8((byte *) &o,sizeof (o));
    oSerial.write(iCRC8);
    return sizeof (o);
}

You can use SerialWrite just like any other function but oSerial can be an I2C Wire object or a HardwareSerial or SoftwareSerial. This is a bit like a virtual function..

If you don't want me to test your 1.7_dev branch on my ArduinoMini+ST7735, you can close this isse :-)