Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More blitting tweaks #4847

Merged
merged 16 commits into from Mar 27, 2019

Conversation

Projects
None yet
3 participants
@NiLuJe
Copy link
Member

commented Mar 27, 2019

Companion PR to koreader/koreader-base#878

  • Cleanup BitOpts usage (require & cache)
  • Unify oddness checks (MOD -> AND)
  • Enforce the native Portrait orientation on Kobo (except @ 16bpp, i.e., KSM w/ 8bpp swap disabled), to allow for faster blitting when unrotted.
  • Switch CRe BB to 32BPP on color screens
  • Minor cleanups

NiLuJe added some commits Mar 18, 2019

Unify odd/even checks
AND instead of modulo
Always enforce a Portrait rotation via fbdepth
We were already doing it on the Forma, do it everywhere, so the 8bpp
blitting codepath has a chance to take the memcpy shortcuts...

Also, switch from MOD to AND, because MOD takes its sign from the
dividend in shell, which would be bad if our current computation didn't
ensure a positive unsigned value on that side of the divison.

(c.f., $(( (0 - 1) % 4 )) vs. $(( (0 - 1) & 3 )), which could
potentially be a viable computation for rotation, and which would
definitley be broken with a modulo).
Don't enforce Portrait @ 16bpp (unless we're on a Forma), as it won't
help much anyway, and there's a good chance it's going to be broken in
fun and weird corner-cases...
Huh. No idea why this only caused a crash on my H2O...
./luajit: frontend/ui/widget/configdialog.lua:368: attempt to index field 'values' (a nil value)
stack traceback:
        frontend/ui/widget/configdialog.lua:368: in function 'init'
        frontend/ui/widget/widget.lua:48: in function 'new'
        frontend/ui/widget/configdialog.lua:602: in function 'init'
        frontend/ui/widget/widget.lua:48: in function 'new'
        frontend/ui/widget/configdialog.lua:800: in function 'update'
        frontend/ui/widget/configdialog.lua:755: in function 'init'
        frontend/ui/widget/widget.lua:48: in function 'new'
        frontend/apps/reader/modules/readerconfig.lua:74: in function 'onShowConfigMenu'
        frontend/apps/reader/modules/readerconfig.lua:92: in function 'handler'
        frontend/ui/widget/container/inputcontainer.lua:248: in function 'handleEvent'
        frontend/ui/uimanager.lua:632: in function 'sendEvent'
        frontend/ui/uimanager.lua:153: in function '__default__'
        frontend/ui/uimanager.lua:966: in function 'handleInputEvent'
        frontend/ui/uimanager.lua:1027: in function 'handleInput'
        frontend/ui/uimanager.lua:1071: in function 'run'
        ./reader.lua:216: in main chunk
        [C]: at 0x00014251
Revert "Switch DjVu to 32BPP buffers on color screens"
This reverts commit 7851a53.

DjVu won't render to straight RGB32, only a custom pixelformat with
masks, so, stay with RGB24

@NiLuJe NiLuJe force-pushed the NiLuJe:blitcpy branch from f84845b to 7822fa3 Mar 27, 2019

@NiLuJe NiLuJe merged commit 7210fb4 into koreader:master Mar 27, 2019

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@poire-z

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

With today's nightly, all is fine and fast on my Kobo and the emulator.

But on my Android 6 phone, the screen is messed up and unreadable: again, some diagonal noise, like the scanline width is wrong.
That includes the cre book and the UI (even just the initial "Opening blah.epub" - it might have the good y and height, but the noise takes the whole width).

Dunno if it's specific to my Android 6, if some fullscreen/viewport twist is at work...

@NiLuJe

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

@poire-z : Does this help?

diff --git a/ffi/framebuffer_android.lua b/ffi/framebuffer_android.lua
index 8ab1af8..9226537 100644
--- a/ffi/framebuffer_android.lua
+++ b/ffi/framebuffer_android.lua
@@ -44,9 +44,9 @@ function framebuffer:_updateWindow()
     if buffer[0].format == C.WINDOW_FORMAT_RGBA_8888
     or buffer[0].format == C.WINDOW_FORMAT_RGBX_8888
     then
-        bb = BB.new(buffer[0].width, buffer[0].height, BB.TYPE_BBRGB32, buffer[0].bits, buffer[0].stride*4)
+        bb = BB.new(buffer[0].width, buffer[0].height, BB.TYPE_BBRGB32, buffer[0].bits, buffer[0].stride*4, buffer[0].stride)
     elseif buffer[0].format == C.WINDOW_FORMAT_RGB_565 then
-        bb = BB.new(buffer[0].width, buffer[0].height, BB.TYPE_BBRGB16, buffer[0].bits, buffer[0].stride*2)
+        bb = BB.new(buffer[0].width, buffer[0].height, BB.TYPE_BBRGB16, buffer[0].bits, buffer[0].stride*2, buffer[0].stride)
     else
         android.LOGE("unsupported window format!")
     end
@poire-z

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

Yep it does! All seems fine with this patch :)

@poire-z

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

Any combination of night mode and orientation seems fine too.

There's just a strange thing (seen on the emulator and android, no kobo at hand) that I don't think I introduced willingly, so it could be some effect of the BB changes (like background gray filling?) :
In landscape and in 2-pages mode, I now have some gray line in the middle of the middle margin, that I didn't have some days ago.
(And it's actually quite not bad :) I may have introduced that willingly if I had thought about it and seen how it looks :)

Wanted to check that on the emulator with blitbuffer.lua, without libblitbuffer.so. But if I move it out, the emulator is stuck on displaying only the widgets: the rendering of the book below stays blank, but the footer page number changes!

@NiLuJe

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

Cool, I'll push the Android fix in the next round of tweaks ;).

I'll see if I can reproduce the Landscape thing on Kobo, and fix it if needed, because the emulator definitely should work with the Lua blitter ;).

@NiLuJe

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

Hmm, no issues with either the Lua or C blitter @ 32bpp on Kobo, but then CRe renders @ 8bpp there ;).

@NiLuJe

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

Okay, can repro on the emulator.

And it actually crashes with the Lua blitter on my end ;D.

@NiLuJe

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

Okay, caught the crash, stupid typo. -_-".

Still not quite sure where that line is coming from though :D.

Since it's rotated, the rendering is punted off the the original blitting code, so I'm going to assume it's coming from CRe's side, somehow? And only happens when it renders at 32bpp?

@NiLuJe

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

It sill happens with the original BGR stuff, so, that's not it either.

@poire-z

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

so I'm going to assume it's coming from CRe's side, somehow? And only happens when it renders at 32bpp?

Exactly :) and that's a feature (only done when 32bpp), not a bug :
https://github.com/koreader/crengine/blob/8f15e01c8aea15920381f14ea85b0b4abeb36cfd/crengine/src/lvdocview.cpp#L2227-L2232

(Whether to keep it or remove it, or make it available on 8bpp too... is another question I don't want an answer to now :)

NiLuJe added a commit that referenced this pull request Mar 29, 2019

A few minor fixes after #4847 (#4850)
* Add a toggle to disable the C blitter in the Dev menu (depends on koreader/koreader-base#882) (never shown if the JIT is disabled, grayed out if the C blitter is not installed)
* Fix a few sizeUtf8Text call sites that were doing a nil check in order to account for the new return type.
* Tweak statusbar handling to avoid spurious sizeUtf8Text warnings when it's hidden, and unify its behavior between being hidden via toggle, and hidden on book open (at least when all-at-once is not enabled).
* c.f., koreader/koreader-base#882 (Android, PB, RGB32 & Legacy Kindle regression fixes).

Frenzie referenced this pull request in koreader/crengine Mar 30, 2019

Fiddle with the RGB pixelformat to play ball with our own BBs...
Here be dragons...

The (very) slight overhead of repacking pixels is outweighed by the
blitting speed gain. At least on SDL/x86_64.
Might be another kettle of fish on Android/ARM...

We enforce this behavior w/ a CR_RENDER_32BPP_RGB_PXFMT define on our
side, leaving vanilla builds unaffected by these changes.

@Frenzie Frenzie added this to the 2019.04 milestone Mar 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.