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

Oops, make sure we use the C bb on all the eInk devices... #1047

Merged
merged 2 commits into from Feb 15, 2020

Conversation

NiLuJe
Copy link
Member

@NiLuJe NiLuJe commented Feb 12, 2020

Note that it's still blacklisted on PB because of their crappy unreliable fb info on some devices, though ;).

So this will be mainly of interest to Cervantes & reMarkable (ping @pazos / @avsej; @tcrs)


This change is Reviewable

@NiLuJe
Copy link
Member Author

NiLuJe commented Feb 12, 2020

I don't expect any trouble at all on reMarkable, because it's @ 8bpp, so it's essentially the same as on Kindle & Kobo.

But Cervantes is probably still @ 16bpp, and we didn't test that for very long on Kobo, because RGB565 is the worst.

@pazos
Copy link
Member

pazos commented Feb 13, 2020

So this will be mainly of interest to Cervantes ...

As far as I am concerned it works fine as is. I'm not too much interested in fiddling with my cervantes besides platform maintenance.

If you think it is good to have the same bb on all platforms or it can provide any benefit for cervantes users I think it is safe to merge. On the worst case we can revert it soon. 👍

@NiLuJe NiLuJe mentioned this pull request Feb 13, 2020
@NiLuJe
Copy link
Member Author

NiLuJe commented Feb 14, 2020

@pazos: Ideally, Cervantes would be switching to 8bpp like Kobo/reMarkable. It'll probably need some testing to have fbdepth behave because of NTX board quirks, though (unlike on reMarkable, where it turned out to be straightforward).

But, in the meantime, the C BB does have full RGB565 support.

@Frenzie
Copy link
Member

Frenzie commented Feb 15, 2020

@NiLuJe I never got the impression that the C BB was really any faster than the LuaJIT one? (Except on malfunctioning Android of course.)

@NiLuJe
Copy link
Member Author

NiLuJe commented Feb 15, 2020 via email

@NiLuJe
Copy link
Member Author

NiLuJe commented Feb 15, 2020

The main gain is arguably in consistency: when the JIT is happy, everything's mostly fine (close/very close to the C perf), but when it isn't, it can be 2 to 10 times slower for no discernible reason.

That's mostly obvious when rotation is involved, or with RGB565, or with alpha-blending.

(Also, dithering is slower in the pure-Lua version, but that's probably mostly because everything's secretely a double ;p).

@Frenzie
Copy link
Member

Frenzie commented Feb 15, 2020

Try panning in an imagewidget ;).

It only seems to "pan" by swipes or something? I don't really use that one.

By "impression" I meant a touch more than just eyeballometric measurements (though not too much more), and I noticed very little meaningful difference. In your terms, probably "very close"/practically identical to "close" in some cases, but that last one not necessarily to the benefit of C… (The C one has been optimized better since.)

That's mostly obvious when rotation is involved, or with RGB565, or with alpha-blending.

Right, alpha-blending is the only one I noticed and/or recall. Probably didn't look into 16-bit too much if at all either.

@NiLuJe
Copy link
Member Author

NiLuJe commented Feb 15, 2020

What I actively measured was blitting CRe's buffer to the framebuffer (i.e., page turns), as that's what I was mainly concerned with. The rest (alpha, image, dithering) is more of an observation than anything ;).

@NiLuJe NiLuJe merged commit e7e8a03 into koreader:master Feb 15, 2020
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Feb 17, 2020
• Workaround mysterous xtext/hb crner-cases on armv6
  (koreader/koreader-base#1046)
• Use the C BB on every !Android eInk device
  (koreader/koreader-base#1047)
• Switch to our own custom PB TC
  (koreader/koreader-base#1048)
• tabs/tabstop handling in xtext
  (koreader/koreader-base#1049)
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Feb 17, 2020
• Workaround mysterous xtext/hb crner-cases on armv6
  (koreader/koreader-base#1046)
• Use the C BB on every !Android eInk device
  (koreader/koreader-base#1047)
• Switch to our own custom PB TC
  (koreader/koreader-base#1048)
• tabs/tabstop handling in xtext
  (koreader/koreader-base#1049)
NiLuJe added a commit to koreader/koreader that referenced this pull request Feb 18, 2020
• Workaround mysterous xtext/hb crner-cases on armv6
  (koreader/koreader-base#1046)
• Use the C BB on every !Android eInk device
  (koreader/koreader-base#1047)
• Switch to our own custom PB TC
  (koreader/koreader-base#1048)
• tabs/tabstop handling in xtext
  (koreader/koreader-base#1049)
* Pickup koreader/koreader-base#1050
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Mar 15, 2020
Regression after koreader/koreader-base#1047 (I
thought it was already implemented).

Fix koreader#5963
pazos pushed a commit to koreader/koreader that referenced this pull request Mar 16, 2020
* Unbreak nightmode on Cervantes

Regression after koreader/koreader-base#1047 (I
thought it was already implemented).

Fix #5963

* make luacheck happy
mwoz123 pushed a commit to mwoz123/koreader that referenced this pull request Mar 29, 2020
• Workaround mysterous xtext/hb crner-cases on armv6
  (koreader/koreader-base#1046)
• Use the C BB on every !Android eInk device
  (koreader/koreader-base#1047)
• Switch to our own custom PB TC
  (koreader/koreader-base#1048)
• tabs/tabstop handling in xtext
  (koreader/koreader-base#1049)
* Pickup koreader/koreader-base#1050
mwoz123 pushed a commit to mwoz123/koreader that referenced this pull request Mar 29, 2020
* Unbreak nightmode on Cervantes

Regression after koreader/koreader-base#1047 (I
thought it was already implemented).

Fix koreader#5963

* make luacheck happy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants