[chore] framebuffer:setViewport ability to set multiple times#558
[chore] framebuffer:setViewport ability to set multiple times#558
Conversation
ffi/framebuffer.lua
Outdated
| self.full_bb = self.bb | ||
| else | ||
| -- reset before applying new viewport | ||
| self.bb = self.bb:viewport( |
There was a problem hiding this comment.
@poire-z I'm not sure if this is quite the right way to go, although a viewport doesn't duplicate any memory and such.
There was a problem hiding this comment.
Is there a real need to reset?
koreader-base/ffi/blitbuffer.lua
Lines 1135 to 1141 in 0b4c48a
Can't you just call it (so, no
else) on self.full_bb : self.bb = self.full_bb:viewport() ?
There was a problem hiding this comment.
I tried, but that just keeps pushing it down.
There was a problem hiding this comment.
Pardon, that's a bit incomplete. But my test case is a top "status bar height" on the Android emulator. I'll give provide some screenshots.
There was a problem hiding this comment.
Okay, so I was preparing my screenshots and I guess the problem I was trying to work around was caused by something incorrect in front? Or maybe it was just too late last night… but I'd swear what you said was the first thing I tried. Seems to be working now though.
There is this remaining beauty flaw. I know there's a draw white thing called somewhere…
| viewport.x, viewport.y, | ||
| viewport.w, viewport.h) | ||
| self.viewport = viewport | ||
| self.full_bb:fill(Blitbuffer.COLOR_WHITE) |
There was a problem hiding this comment.
@poire-z Okay, it all looks good this way. Do you think these extra two lines are a self-evident addition to setViewport or some kind of overreach?
There was a problem hiding this comment.
From nowhere, I'd ask: why do you need them? You're setting a viewport on an existing full_bb, why would you need to erase it and repaint it? But if you did it, it obviously helps in some way, no?
There was a problem hiding this comment.
You didn't see my screenshot? The existing full_bb already has content on it. It'll just remain there in perpetuity.
The only question is where these lines should be called from. Not if.
There was a problem hiding this comment.
Oh, ok, I probably don't get what viewport do (thought you had a full_bb, and viewport was the part of it that was displayed, so stuff outside would not matter).
So, you're more knowing than me to answer this question :)
There was a problem hiding this comment.
Traditionally what you say is essentially correct.
- We have a device, such as the H2O.
- We set the viewport on init combined with a white fill. This is enough. The white will stay white.
But now we add:
- We dynamically adjust the viewport on Android to adjust our display for the statusbar/toolbar. (I'd prefer to separate them out rather than just a generic "fullscreen".)
Whether this is 100% for sure the best way to do it I'm not entirely sure, but it's the easiest to implement for now and all roadblocks along the way are relevant with or without viewport (e.g., resizable SDL window).
Only the ReaderUI is currently capable of dynamic adjustment due to portrait/landscape. It'll need to be added to the FileManager. But I'll ignore that problem for now. Especially since there are some issues to be worked out on the Android side of things that I want to be as little involved in as possible.
References koreader/koreader#3392