AnHardt / Marlin

Reprap FW with look ahead. SDcard and LCD support. It works on Gen6, Ultimaker, RAMPS and Sanguinololu
GNU General Public License v3.0
1 stars 1 forks source link

Glcdtests241116 #67

Closed AnHardt closed 7 years ago

AnHardt commented 7 years ago

Todays test of the GLCD enhacements

RCBugFix Status screen 2 Stripes 77ms 4 Stripes 86ms 8 Stripes 110ms

Selective rendering 2 Stripes 75ms 4 Stripes 76ms 8 Stripes 85ms

Selective + unroll 2 Stripes 78ms

Selective + unroll + decrease delay 2 Stripes 76ms

Selective + decrease delay 2 Stripes 73ms

Detailed timings in the commits comments.

Conclusion: Selective rendering is a small win for the two stripe ST7920 but makes a huge difference for displays with more stripes. Go for it. Unrolling the loops seems to make the updates a tiny bit slower. Don't merge. Decreasing the 10µs-delay to 5Ms speeds up ~2ms per complete update. Today i could not go below 5µs. If we should do that - with the original 10 µseconds and a big fat warning - decrease on your own risk.

AnHardt commented 7 years ago

You can run this benchmarks completely without any display connected (but compiled in) - with the same results.

AnHardt commented 7 years ago
    case U8G_DEV_MSG_PAGE_NEXT: {
      uint8_t* ptr;
      u8g_pb_t* pb = (u8g_pb_t*)(dev->dev_mem);
      y = pb->p.page_y0;
      ptr = (uint8_t*)pb->buf;

      ST7920_CS();
      //for (i = 0; i < PAGE_HEIGHT; i++) {
      for (i = 0; i < PAGE_HEIGHT-1; i++) {
        ST7920_SET_CMD();
        if (y < 32) {
          ST7920_WRITE_BYTE(0x80 | y);       //y
          ST7920_WRITE_BYTE(0x80);           //x=0
        }
        else {

gives an inpressoin of the stripes edges. picture 8

AnHardt commented 7 years ago

Send 'M117' to clear the status line and save 3ms.

AnHardt commented 7 years ago

Save about 16ms !!! by replacing the inverse coordinate box by a frame.

  if (PAGE_CONTAINS(30, 39)) {
    //u8g.drawBox(0, 30, LCD_PIXEL_WIDTH, INFO_FONT_HEIGHT + 2); // 10: 30-39  9: 30-37 !!!!!!!!!
    u8g.drawFrame(0, 30, LCD_PIXEL_WIDTH, INFO_FONT_HEIGHT + 2); // 10: 30-39  9: 30-37

    if (PAGE_CONTAINS(XYZ_BASELINE - INFO_FONT_HEIGHT + 1, XYZ_BASELINE)) {

      // Generate the strings only once
      if (remake_strings) {
        remake_strings = false;
        strcpy(xstring, ftostr4sign(current_position[X_AXIS]));
        strcpy(ystring, ftostr4sign(current_position[Y_AXIS]));
        strcpy(zstring, ftostr52sp(current_position[Z_AXIS] + 0.00001));
      }

      //u8g.setColorIndex(0); // white on black                   !!!!

      u8g.setPrintPos(2, XYZ_BASELINE);
      _draw_axis_label(X_AXIS, PSTR(MSG_X), blink);
      u8g.setPrintPos(10, XYZ_BASELINE);
      lcd_print(xstring);

      u8g.setPrintPos(43, XYZ_BASELINE);
      _draw_axis_label(Y_AXIS, PSTR(MSG_Y), blink);
      u8g.setPrintPos(51, XYZ_BASELINE);
      lcd_print(ystring);

      u8g.setPrintPos(83, XYZ_BASELINE);
      _draw_axis_label(Z_AXIS, PSTR(MSG_Z), blink);
      u8g.setPrintPos(91, XYZ_BASELINE);
      lcd_print(zstring);

      //u8g.setColorIndex(1); // black on white                        !!!!!
    }

picture 9

AnHardt commented 7 years ago
  if (PAGE_CONTAINS(29, 39)) {
    //u8g.drawBox(0, 30, LCD_PIXEL_WIDTH, INFO_FONT_HEIGHT + 2); // 10: 30-39  9: 30-37
    u8g.drawFrame(0, 29, LCD_PIXEL_WIDTH, INFO_FONT_HEIGHT + 3); // 11: 29-39  10: 29-37

Is looking much better. picture 10

AnHardt commented 7 years ago
    //
    // SD Card Symbol
    //

    if (PAGE_CONTAINS(42 - (TALL_FONT_CORRECTION), 51 - (TALL_FONT_CORRECTION))) {
/*      // Upper box
      u8g.drawBox(42, 42 - (TALL_FONT_CORRECTION), 8, 7);     // 42-48 (or 41-47)
      // Right edge
      u8g.drawBox(50, 44 - (TALL_FONT_CORRECTION), 2, 5);     // 44-48 (or 43-47)
      // Bottom hollow box
      u8g.drawFrame(42, 49 - (TALL_FONT_CORRECTION), 10, 4);  // 49-52 (or 48-51)
      // Corner pixel
      u8g.drawPixel(50, 43 - (TALL_FONT_CORRECTION));         // 43 (or 42)
*/
      u8g.drawHLine(42, 42 - (TALL_FONT_CORRECTION), 8);
      u8g.drawVLine(42, 43 - (TALL_FONT_CORRECTION), 9);
      u8g.drawHLine(43, 49 - (TALL_FONT_CORRECTION), 8);
      u8g.drawHLine(42, 52 - (TALL_FONT_CORRECTION), 10);
      u8g.drawVLine(51, 44 - (TALL_FONT_CORRECTION), 8);
      u8g.drawPixel(50, 43 - (TALL_FONT_CORRECTION));         // 43 (or 42)
    }

does not save anything.

thinkyhead commented 7 years ago

I noticed that the Status Message was being drawn three (possibly two) lines higher than the bottom of the screen so I moved it down to the next-to-last line. Maybe we can move it down one more line to keep it within the last stripe.

Some other elements can be spaced out a bit more, if we want. Then, for some of the lines that use 8-pixel tall fonts, we can try to ensure that they're within a single stripe.

And of course we can recommend the USE_SMALL_INFOFONT option to save some cycles too.

AnHardt commented 7 years ago

This version of the sd-card-symbol saves maybe a fraction of a ms.

In dogm_bitmaps.h

#endif // HAS_TEMP_BED

#if ENABLED(SDSUPPORT)

  #define STATUS_SD0_WIDTH 8
  #define STATUS_SD0_BYTEWIDTH 1
  #define STATUS_SD0_HEIGHT 7

  const unsigned char sd0_graphic[STATUS_SD0_BYTEWIDTH*STATUS_SD0_HEIGHT] PROGMEM =
  {
     0xfc
    ,0x82
    ,0x81
    ,0x81
    ,0x81
    ,0x81
    ,0xff
  };

  #define STATUS_SD1_WIDTH 8
  #define STATUS_SD1_BYTEWIDTH 1
  #define STATUS_SD1_HEIGHT 3

  const unsigned char sd1_graphic[STATUS_SD1_BYTEWIDTH*STATUS_SD1_HEIGHT] PROGMEM =
  {
    0x81
   ,0x81
   ,0xff
  };
#endif //SDSUPPORT

In ultralcd_impl_DOGM.h

  #if ENABLED(SDSUPPORT)

    //
    // SD Card Symbol
    //

    if (PAGE_CONTAINS(43 - (TALL_FONT_CORRECTION), 52 - (TALL_FONT_CORRECTION))) {
/*      // Upper box
      u8g.drawBox(42, 42 - (TALL_FONT_CORRECTION), 8, 7);     // 42-48 (or 41-47)
      // Right edge
      u8g.drawBox(50, 44 - (TALL_FONT_CORRECTION), 2, 5);     // 44-48 (or 43-47)
      // Bottom hollow box
      u8g.drawFrame(42, 49 - (TALL_FONT_CORRECTION), 10, 4);  // 49-52 (or 48-51)
      // Corner pixel
      u8g.drawPixel(50, 43 - (TALL_FONT_CORRECTION));         // 43 (or 42)
*/
      u8g.drawBitmapP(43, 43 - (TALL_FONT_CORRECTION), STATUS_SD0_BYTEWIDTH, STATUS_SD0_HEIGHT, sd0_graphic);
      if (IS_SD_INSERTED)
        u8g.drawBitmapP(43, 50 - (TALL_FONT_CORRECTION), STATUS_SD1_BYTEWIDTH, STATUS_SD1_HEIGHT, sd1_graphic);
    }

    //
    // Progress bar frame

If you want to time the difference - don't forget to send a M117 before (clear the status line). Else the different text length in the statusline makes more difference then the sd-card-symbol.

It has a gimmick to save some more cycles when the card is not inserted. picture 13

picture 12

AnHardt commented 7 years ago

Would make more sense to time this without selective rendering - makes more difference then.

Hollow extruders are a pain - we do not have the sources of the graphics. Maybe later.

@thinkyhead

I noticed that the Status Message was being drawn three (possibly two) lines higher than the bottom of the screen so I moved it down to the next-to-last line. Maybe we can move it down one more line to keep it within the last stripe.

Not a good idea. We are setting the baseline of the font here. That is the height where the 'normal symbols have their base. g, j, p, q, y, ',', ';' are 1 pixel lower, And in the new Greek and Turkish fonts there are chars being 2 pixels lower. I will try to correct that soon. Until then the baseline needs to be two pixels above the end.

If you ask me, USE_SMALL_INFOFONT should be dropped at all. The font is unmaintainable because we do bot have the sources. It is only available in a few languages using pure ASCII (ISO10646_1). And in my opinion it is ugly and not good readable (ok i'm biased). When i made the 'language system', the 'small font' was already removed. It came back for the request of a single user. About the same is true for the big edit font. But here I was the only one who defended it. With my new glasses and a tiny bit larger normal then the former (small) font i have no problems anymore.

Getting rid of these two fonts would not improve Marlin, but also, Marlin would not be worse without them. What we would get, is a much improved testability und a somewhat less complex code with less 'exceptions'.

The numbers 0-9 are all only 7 pixel tall - in the normal font. We can trust in that. "°C%:?XYZ" are all <= 7 pixel.

On the other hand it does not make much sense to optimize for 8/8 stripes. As far i can see, we can have 4/4 or 2/2 devices vor all the currently supported displays. And 4/4 was already very close to 2/2 now. (https://github.com/AnHardt/Marlin/blob/dc7e68454aa9f7230e51e51fe3fb7753eceb40af/Marlin/dogm_lcd_implementation.h#L115-L146)

thinkyhead commented 7 years ago

I agree the small font is kind of pointless. We still use the same amount of screen space. It doesn't draw all that much less.