Hi, In my version, I was worried that there might be very occasional problems if _lastxand _lasty were not invalidated by the rectangle drawing routine so I initialised both variables to -1 when declared and then reset them to -1 after drawing the rectangle.

Thanks! I'd definitely add @jeffmer's suggestion.

Also, any chance of just using gfx->data.width rather than having a separate var for it?

Thanks for checking.

Please do not merge now, like to run some more tests. Let you know when good to go.

I really like lcd_spi_unbuf class, but drawing anything with vector font or drawImages() is not fast.

Would it make sense to implement

• http://forum.espruino.com/comments/15491106 .setRotation() dependent?

  does the fill algorithm work horizontal even with setRotation(1)?

• a dynamic sendbuffer with window options for quicker updates?

Tweaking the window/optimisation based on setRotation would be really neat. I can't remember for sure, but it may be that fillRect/fillPoly automatically rotate before rendering so you won't see a big difference though?

The dynamic sendbuffer could be neat. I feel it's diminishing returns though - in a lot of cases I'm not sure the sendbuffer will help. It's literally just bitmap rendering where it'll help you, not fonts or anything else.

IMO it might be more sensible to look at ways of improving the API so that drawing bitmaps can be handled quickly (eg. a blitStart/blitEnd functionality like I'd started doing with st7789_8bit)

IMO it might be more sensible to look at ways of improving the API so that drawing bitmaps can be handled quickly (eg. a blitStart/blitEnd functionality like I'd started doing with st7789_8bit)

Great, so let's improve this by using blitbit operations.

Ok, got blit1Bit working and it is a improvement for 1 BPP images, but when it comes to g.setRotation() it's slow .....

Any hint how to improve/extend bit blitting for e.g. g.setRotation(1)?

I have been doing some measurements comparing Bangle with an ESP32 with an ST789V 240x240 display. The driver is my optimised version of @MaBecker's excellent lcd_spi_unbuf which can be viewed here. This version uses a chunk buffer to optimise both spi writes and pin operations.

I compare the time taken to fill a 240 x 160 pixel rectangle with the time to draw a 240 x 160 1 bit image. I include the test code below in case you want to try it.

RESULTS

Bangle: fillRect 11ms, drawImage 66ms.

ESP32: fillRect 44ms, drawImage 82ms.

Given these numbers, I wonder if the extra complexity of adding bitblit operations to the existing simple callback interface is merited since my understanding is that the fillRect figure of 44ms represents the best you can do using the current Espruino implementation of SPI.

function time_fill(){
    g.setColor(0x07E0);
    var time= Date.now();
    g.fillRect(0,40,239,199);
    g.flip();
    time = Math.floor(Date.now()-time);
    console.log("Time to Draw Rectangle: "+time+"ms");
}

var pal1color = new Uint16Array([0x0000,0xF100]);
var buf = Graphics.createArrayBuffer(240,160,1,{msb:true});
buf.setColor(1);
buf.fillRect(0,0,239,159);

function time_image(){
    var time= Date.now();
    g.drawImage({width:240,height:160,bpp:1,buffer:buf.buffer, palette:pal1color},0,40);
    g.flip();
    time = Math.floor(Date.now()-time);
    console.log("Time to Draw Image: "+time+"ms");
}

//Bangle.on('swipe',(dir)=>{
FT5206.on('swipe',(dir)=>{
    if(dir<0)
        time_fill();
    else 
        time_image();
});

@jeffmer can you please provide numbers when using g.setRotation(1).

RESULTS

Bangle: fillRect 11ms, drawImage 722ms.

ESP32: fillRect 44ms, drawImage 5841ms.

It’s predictably awful - not great on Bangle either. I have not needed it, however, I will try to implement an overriding setRotation function in the driver using ST7789 hardware operations which should fix the problem.

fillRect figure of 44ms represents the best you can do using the current Espruino implementation of SPI.

I believe that is correct. There is an issue with setRotation in that it'll cause the direction that pixels are written in to be modified, which means that the lastPixel optimisations have no effect.

Even the Bangle.js optimised blit only works when unrotated

I really think that trying to hack around this is massive overkill though. As @jeffmer point out you can just write to the MADCTL register on ST7789 to effectively do setRotation, while allowing Espruino to still write at full speed. It'll be far faster than any hack we could do here

edit: having said that ST7789V may not allow row/column swaps. Some drivers do though.

I'd be interested to see if a rotated drawImage on setRotation(0) is faster than a normal drawImage on setRotation(1)

I really think that trying to hack around this is massive overkill though.

that's what I thought too when implementing this bit stuff.

So that's it, ready to merge.

The version that you plan to merge here is six times slower for drawImage than the optimised version I reference above.

The version that you plan to merge here is six times slower for drawImage than the optimised version I reference above.

feel free to go for your own pull request.

Thanks will do. I was concerned as to your and Gordon’s view as to whether the need for the flip() operation to flush the buffer is consistent with other graphic drivers.

I think the flip could be problematic as you're still rendering, just not completely. Chances are people would think it was a glitch rather than their issue.

Maybe since the file is being scanned for jswrapper anyway (@MaBecker normally when a file contains stuff that will get created in JS it should be called jswrap_xyz - I missed that first time) you could just define an idle handler:

/*JSON{
  "type" : "idle",
  "generate" : "jswrap_lcd_spi_unbuf_idle"
}*/
bool jswrap_lcd_spi_unbuf_idle() {
//  if there's stuff left to send do it here
return false;
}

Thanks that’s neat, I was not aware of the idle facility. I will implement and test.

@jeffmer Please keep in mind: The main goal of this lib is to use no buffers.

Thanks @gfwilliams for double checking the lib.

Will add jswrap_lcd_spi_unbuf_idle and go for some more test with blit1Bit and MADCTL.

Just to add here - not sure what the situation is with DMA on ESP32 (I guess we don't have it?) but that would be a great way to speed this up. Small buffer, and then queue pixels to transmit if it's in progress. Realistically we could get almost fillRect performance with drawImage and other stuff by sending at the same time as calculating

not sure what the situation is with DMA on ESP32

Good point, for now we use spi_send_many for ESP32 with a default buffer size of 128 byte.

Ok, using MADCTL is a big help.

// details
#define MADCTL_MY 0x80  ///< Bottom to top
#define MADCTL_MX 0x40  ///< Right to left
#define MADCTL_MV 0x20  ///< Reverse Mode
#define MADCTL_ML 0x10  ///< LCD refresh Bottom to top
#define MADCTL_RGB 0x00 ///< Red-Green-Blue pixel order
#define MADCTL_BGR 0x08 ///< Blue-Green-Red pixel order
#define MADCTL_MH 0x04  ///< LCD refresh right to left

This is what can be used for a M5STACK with ILI9341 lcd instead of g.setRotation(1)

// TFT_MADCTL: Memory Access Control (orientation)
cmd(0x36,0);

plus flip height, width and add rotate

var g = require("ILI9341-UB-SV2").connect(SPI1,
          { cs: M5.LCD_CS,
            dc: M5.LCD_DC,
            width: M5.LCD_HEIGHT, //M5.LCD_WIDTH,
            height: M5.LCD_WIDTH, //M5.LCD_HEIGHT,
            colstart:0,
            rowstart:0,
            inverse : 1,
            rotation: 1      // <-- add rotation 
          });

What about adding a line buffer to blit1Bit to improve drawImage?

  int lb_len = w*scale;
  uint16_t line_buffer[lb_len];

  // loop over y
     // loop over x
       //  get a pixel and fill  line_buffer  scale times 
    // send that line buffer scaled times

I think the point is that with the buffer it can be made fast enough that you don't need special cases like blit1bit any more

ok, just removed the bilt code and implemented jeffmer latest version

this image took around 700ms and now it is finish in 120ms - this is a great improvement - many thanks to jeffmer.

@gfwilliams Please let us know if there is something you like to get updated.

Looks good - please could you just rename #define BUFFER SPISENDMANY_BUFFER_SIZE - maybe to LCD_SPI_UNBUF_LEN or something like that? Having a global BUFFER #define is likely to conflict with other stuff

Thanks for double checking.

Great - thanks!