[feat] SDL2: update parts of window like a real device

[Frenzie]

This commit first renders the full updated blitbuffer to a temporary texture.
Subsequently it renders the updated rectangle from the temporary texture onto
the final texture, which has up to that point retained all of the old pixel
info. That final texture, of which only the one rectangular area was updated,
is then rendered to the window.

Without this, there are cases where a full screen (or at least bigger) refresh
is required that will not be obvious without first testing on a real device.
See koreader/koreader#3804

Also fixes EMULATE_READER_FLASH, which was accidentally broken in 981bd40

[poire-z]

Working fine.

One small observation:
When in reader, and tap on top of screen: it opens top menu and bottom menu.
Before/after: same log

03/27/18-17:10:11 DEBUG refresh on physical rectangle -13 444 626 167
03/27/18-17:10:12 DEBUG refresh on physical rectangle -1 -1 602 368

Before this, over slow ssh, I saw a single paint from top to bottom: so top menu was drawn first.
With this PR, I saw first the bottom menu being painted, and then the top menu. Which is conform to the order they are done in the above log. And is nice as it shows visually the order things are done.

I'm just wondering if the fact that I didn't see 2 painting before is may be one being cancelled by the 2nd by X11 or SDL - or them being combined.
Or if the fact that I see 2 paintings after, is because you push to SDL/X11 earlier?
I also thought that somehow, 2 rects (top menu, bottom menu) were combined, and that, as they don't intersect, they were combined to the full screen. But may be there are some scheduleIn involved that provokes these 2 refreshes/paintings.

So, no real problem, just saying in case that behaviour talks to you that just put eyes on this code :)

[Frenzie]

Note that you should've always been able to see it with something like EMULATE_READER_FLASH=200 ./kodev run.

I think I've made things slightly slower with my workaround. I do see the effect you mention, at least when maximized. It's hard to tell, but with manual stopwatch timing I get ~30 ms for before and ~40 ms for after on fullscreen HD to open the menu. Which certainly makes intuitive sense with the overhead of creating an extra texture and all that.

Since SDL isn't just for debugging that means this might need a slight rethink. On the other hand, since no one uses Ubuntu Phone anymore anyway it might not matter for the moment?

Before

====== ./ffi/SDL2_0.lua ======
@@ 61 @@
      |         bit.bor(full_screen and 1 or 0, SDL.SDL_WINDOW_RESIZABLE)
      |     )
      | 
   3% |     S.renderer = SDL.SDL_CreateRenderer(S.screen, -1, 0)
      |     S.texture = S.createTexture()
      | end
      | 

====== ./ffi/blitbuffer.lua ======
@@ 757 @@
      |     if use_cblitbuffer and setter == self.setPixel then
      |         cblitbuffer.BB_blit_to(ffi.cast("struct BlitBuffer *", source),
      |             ffi.cast("struct BlitBuffer *", self),
  21% |             dest_x, dest_y, offs_x, offs_y, width, height)
      |     else
      |         source[self.blitfunc](source, self, dest_x, dest_y, offs_x, offs_y, width, height, setter, set_param)
      |     end

====== ./ffi/framebuffer_SDL2_0.lua ======
@@ 69 @@
      |     else
      |         SDL.SDL.SDL_UpdateTexture(SDL.texture, nil, bb.data, bb.pitch)
      |     end
  54% |     SDL.SDL.SDL_RenderClear(SDL.renderer)
      |     SDL.SDL.SDL_RenderCopy(SDL.renderer, SDL.texture, nil, nil)
      |     SDL.SDL.SDL_RenderPresent(SDL.renderer)
      | end

====== frontend/document/credocument.lua ======
@@ 289 @@
      |         self.buffer = Blitbuffer.new(rect.w, rect.h, self.render_color and Blitbuffer.TYPE_BBRGB16 or nil)
      |     end
      |     self._document:drawCurrentPage(self.buffer, self.render_color)
   3% |     target:blitFrom(self.buffer, x, y, 0, 0, rect.w, rect.h)
      | end
      | 
      | function CreDocument:drawCurrentViewByPos(target, x, y, rect, pos)

After

====== ./ffi/SDL2_0.lua ======
@@ 61 @@
      |         bit.bor(full_screen and 1 or 0, SDL.SDL_WINDOW_RESIZABLE)
      |     )
      | 
   3% |     S.renderer = SDL.SDL_CreateRenderer(S.screen, -1, 0)
      |     S.texture = S.createTexture()
      | end
      | 

====== ./ffi/blitbuffer.lua ======
@@ 757 @@
      |     if use_cblitbuffer and setter == self.setPixel then
      |         cblitbuffer.BB_blit_to(ffi.cast("struct BlitBuffer *", source),
      |             ffi.cast("struct BlitBuffer *", self),
  21% |             dest_x, dest_y, offs_x, offs_y, width, height)
      |     else
      |         source[self.blitfunc](source, self, dest_x, dest_y, offs_x, offs_y, width, height, setter, set_param)
      |     end
@@ 938 @@
      |     h, y = BB.checkBounds(h, y, 0, self:getHeight(), 0xFFFF)
      |     if use_cblitbuffer and setter == self.setPixel then
      |         cblitbuffer.BB_fill_rect(ffi.cast("struct BlitBuffer *", self),
   3% |             x, y, w, h, value:getColorRGB32())
      |     else
      |         for tmp_y = y, y+h-1 do
      |             for tmp_x = x, x+w-1 do

====== ./ffi/framebuffer_SDL2_0.lua ======
@@ 82 @@
      |     end
      | 
      |     -- render to our beloved texture instead of the window
  53% |     SDL.SDL.SDL_SetRenderTarget(SDL.renderer, SDL.texture)
      |     -- ideally nil, rect to render the full temp_texture to rect in
      |     -- our beloved target texture but in this case we select the same
      |     -- rect from both textures

I'm actually slightly surprised to see SDL_SetRenderTarget taking the bulk of the CPU time.

Get this in now and improve later or let it sit until I have time to experiment?

[poire-z]

OK, I see the 2 steps with EMULATE_READER_FLASH=1000 and just your flash fix, so I guess it has always been 2 steps.
Reading again uimanager, it seems that only when regions do intersect that they are combined into a single one. So, in our case, they don't intersect, and there are 2 calls to the framebuffer refresh functions. So what we see is explained.

Get this in now and improve later or let it sit until I have time to experiment?

I don't mind the little extra slowness. The fact that we see multiple small stuff happening makes it feel less slow actually :) So, go ahead. (btw, i'll soon be off for a week, so please don't wait for my comments - you can go on building that koreader desktop app while I have my back turned :).

[Frenzie]

I don't think slower is part of a desktop app. :-p

(And this particular setup is definitely wholly for the emulator.)

Before you go, do you have any opinion on hotkeys to trigger some refresh debug output during runtime? (Similar to that flash thing.) I got some ideas while playing with this.

[poire-z]

No particular opinion, but no pb with having hotkeys for debugging.

[Frenzie]

Yeah, so Callgrind says (or at least strongly implies) LuaJIT is just getting the important bit wrong for some reason and that SDL_UpdateTexture is responsible as made much more sense really. But good news! I found a much better solution. It changes much less, it looks like this and it performs very well:

function framebuffer:_render(bb, x, y, w, h)
    w, x = BB.checkBounds(w or bb:getWidth(), x or 0, 0, bb:getWidth(), 0xFFFF)
    h, y = BB.checkBounds(h or bb:getHeight(), y or 0, 0, bb:getHeight(), 0xFFFF)
    x, y, w, h = bb:getPhysicalRect(x, y, w, h)

    local ffi = require("ffi")

    local cdata = ffi.C.malloc(w * h * 4)
    local mem = ffi.cast("char*", cdata)
    local offset_counter = 0
    for from_top = y, y+h-1 do
        local offset = 4 * w * offset_counter
        offset_counter = offset_counter + 1
        for from_left = x, x+w-1 do
            local c = bb:getPixel(from_left, from_top):getColorRGB32()
            mem[offset] = c.r
            mem[offset + 1] = c.g
            mem[offset + 2] = c.b
            mem[offset + 3] = 0xFF
            offset = offset + 4
        end
    end

    local rect = SDL.rect(x, y, w, h)

    if bb:getInverse() == 1 then
        self.invert_bb:invertblitFrom(bb)
        SDL.SDL.SDL_UpdateTexture(temp_texture, nil, self.invert_bb.data, self.invert_bb.pitch)
    else
        SDL.SDL.SDL_UpdateTexture(SDL.texture, rect, mem, 4*w)
    end

    ffi.C.free(cdata)

    SDL.SDL.SDL_RenderClear(SDL.renderer)
    SDL.SDL.SDL_RenderCopy(SDL.renderer, SDL.texture, nil, nil)
    SDL.SDL.SDL_RenderPresent(SDL.renderer)
end

On a side note, I just came across https://github.com/torch/sdl2-ffi/blob/50659fbeca83d667240b197298a0462c7ec0ad21/cdefs.lua#L2293 which looks like it makes for easier c/p (because ready-made).

[Frenzie]

Now the profile looks like this (i.e., that free should obviously be post-render):

====== ./ffi/blitbuffer.lua ======
@@ 757 @@
      |     if use_cblitbuffer and setter == self.setPixel then
      |         cblitbuffer.BB_blit_to(ffi.cast("struct BlitBuffer *", source),
      |             ffi.cast("struct BlitBuffer *", self),
  29% |             dest_x, dest_y, offs_x, offs_y, width, height)
      |     else
      |         source[self.blitfunc](source, self, dest_x, dest_y, offs_x, offs_y, width, height, setter, set_param)
      |     end
@@ 938 @@
      |     h, y = BB.checkBounds(h, y, 0, self:getHeight(), 0xFFFF)
      |     if use_cblitbuffer and setter == self.setPixel then
      |         cblitbuffer.BB_fill_rect(ffi.cast("struct BlitBuffer *", self),
   3% |             x, y, w, h, value:getColorRGB32())
      |     else
      |         for tmp_y = y, y+h-1 do
      |             for tmp_x = x, x+w-1 do

====== ./ffi/framebuffer_SDL2_0.lua ======
@@ 80 @@
      |         for from_left = x, x+w-1 do
      |             local c = bb:getPixel(from_left, from_top):getColorRGB32()
      |             mem[offset] = c.r
   5% |             mem[offset + 1] = c.g
      |             mem[offset + 2] = c.b
      |             mem[offset + 3] = 0xFF
      |             offset = offset + 4
@@ 96 @@
      |         SDL.SDL.SDL_UpdateTexture(SDL.texture, rect, mem, 4*w)
      |     end
      | 
  31% |     ffi.C.free(cdata)
      | 
      |     SDL.SDL.SDL_RenderClear(SDL.renderer)
      |     SDL.SDL.SDL_RenderCopy(SDL.renderer, SDL.texture, nil, nil)

====== frontend/document/credocument.lua ======
@@ 289 @@
      |         self.buffer = Blitbuffer.new(rect.w, rect.h, self.render_color and Blitbuffer.TYPE_BBRGB16 or nil)
      |     end
      |     self._document:drawCurrentPage(self.buffer, self.render_color)
   3% |     target:blitFrom(self.buffer, x, y, 0, 0, rect.w, rect.h)
      | end
      | 
      | function CreDocument:drawCurrentViewByPos(target, x, y, rect, pos)

Edit: output after moving the ffi.C.free to the bottom:

====== ./ffi/blitbuffer.lua ======
@@ 757 @@
      |     if use_cblitbuffer and setter == self.setPixel then
      |         cblitbuffer.BB_blit_to(ffi.cast("struct BlitBuffer *", source),
      |             ffi.cast("struct BlitBuffer *", self),
  30% |             dest_x, dest_y, offs_x, offs_y, width, height)
      |     else
      |         source[self.blitfunc](source, self, dest_x, dest_y, offs_x, offs_y, width, height, setter, set_param)
      |     end
@@ 938 @@
      |     h, y = BB.checkBounds(h, y, 0, self:getHeight(), 0xFFFF)
      |     if use_cblitbuffer and setter == self.setPixel then
      |         cblitbuffer.BB_fill_rect(ffi.cast("struct BlitBuffer *", self),
   3% |             x, y, w, h, value:getColorRGB32())
      |     else
      |         for tmp_y = y, y+h-1 do
      |             for tmp_x = x, x+w-1 do

====== ./ffi/framebuffer_SDL2_0.lua ======
@@ 80 @@
      |         for from_left = x, x+w-1 do
      |             local c = bb:getPixel(from_left, from_top):getColorRGB32()
      |             mem[offset] = c.r
   6% |             mem[offset + 1] = c.g
      |             mem[offset + 2] = c.b
      |             mem[offset + 3] = 0xFF
      |             offset = offset + 4
@@ 96 @@
      |         SDL.SDL.SDL_UpdateTexture(SDL.texture, rect, mem, 4*w)
      |     end
      | 
  31% |     SDL.SDL.SDL_RenderClear(SDL.renderer)
      |     SDL.SDL.SDL_RenderCopy(SDL.renderer, SDL.texture, nil, nil)
      |     SDL.SDL.SDL_RenderPresent(SDL.renderer)

It's looking like a performance increase, neat. :-)

[Frenzie]

Makes a bigger difference for performance than I thought, too.

[Frenzie]

Maybe some kind of BB:getRect(angle)?