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

Leaving a 90° rotated PDF shortens the file browser on Android #6704

Closed
RueleuR opened this issue Sep 23, 2020 · 15 comments · Fixed by #6788
Closed

Leaving a 90° rotated PDF shortens the file browser on Android #6704

RueleuR opened this issue Sep 23, 2020 · 15 comments · Fixed by #6788

Comments

@RueleuR
Copy link

RueleuR commented Sep 23, 2020

KOReader version: 2020.09 and 2020.08.1

Device: Tolino Epos with Android 4.4.2 and phone with Android 10

Issue

Leaving a 90° rotated PDF shortens the file browser on Android.

Steps to reproduce

  1. open PDF
  2. rotate 90°
  3. switch to the file browser --> picture 1 and picture 3 should be identical.

Picture 1. (0°)
Screenshot_20200923-205627_KOReader

Picture 2. (90°)
Screenshot_20200923-205502_KOReader

Picture 3. (0°, there is blank space below the file list an the menu a the top is false alligned)
Screenshot_20200923-205440_KOReader

crash.log (if applicable)

Here is a logcat: Height and width are alternating. The file is opened in 90° and than the file browser will be opened
logcat.txt

@pazos
Copy link
Member

pazos commented Sep 23, 2020

Probably not android related. The screen orientation is changed fine and all the screen is filled with white color. A bad rota on android would mean the half lower part of pic3 is black.

@NiLuJe
Copy link
Member

NiLuJe commented Sep 23, 2020

Can't repro on Kobo, FWIW.

@RueleuR
Copy link
Author

RueleuR commented Sep 23, 2020

I think the filebrowser is still 90° rotated while the screen is 0°. It is visible on the top of the third screen. The "KOreader" title is not in the middle of the screen. A logcat is added. I guess one of this two lines is wrong.

[09-23 21:26:37.772 18418:18452 I/KOReader]
Resizing screen to {
["h"] = 1080,
["w"] = 1920
}

[09-23 21:26:45.401 18418:18418 V/KOReader]
[NativeSurfaceView] surface changed {
width: 1080
height: 1920
}

@pazos
Copy link
Member

pazos commented Sep 23, 2020

@RueleuR:

The surface changes first, then some events are broadcasted to widgets to resize the screen to the new window size. So your two log samples are from two different orientation changes. The first for a portrait -> landscape resize. The second for landscape -> portrait surface change.

A surface change is a kind of configuration change and handled in https://github.com/koreader/koreader/blob/master/frontend/device/android/device.lua#L140-L152

Please attach the full log.

@RueleuR
Copy link
Author

RueleuR commented Sep 23, 2020

@pazos A new log is attached in the main topic: https://github.com/koreader/koreader/files/5271737/logcat.txt

@pazos
Copy link
Member

pazos commented Sep 23, 2020

Actually the new log makes sense. Could you attach the same log but including verbose messages too? (ie: instead of KOReader:I use KOReader:V)

@RueleuR
Copy link
Author

RueleuR commented Sep 24, 2020

New logcat.txt added to main topic using command 'adb logcat KOReader:V ActivityManager:* AndroidRuntime:* *:F': https://github.com/koreader/koreader/files/5274052/logcat.txt

@pazos
Copy link
Member

pazos commented Sep 27, 2020

Everything seems fine:

first config change

09-24 08:56:04.130  1852  4480 I ActivityManager: Override config changes=20000480 {1.0 262mcc7mnc [de_DE] ldltr sw400dp w711dp h375dp 432dpi nrml long land night finger -keyb/v/h -nav/h winConfig={ mBounds=Rect(0, 0 - 1080, 1920) mAppBounds=Rect(0, 0 - 1920, 1080) mWindowingMode=fullscreen mActivityType=undefined} s.121} for displayId=0
09-24 08:56:04.221 14708 14708 V KOReader: [NativeSurfaceView] surface changed {
09-24 08:56:04.221 14708 14708 V KOReader:   width:  1920
09-24 08:56:04.221 14708 14708 V KOReader:   height: 1080
09-24 08:56:04.221 14708 14708 V KOReader: }
09-24 08:56:04.453 14708 14743 I KOReader:  Resizing screen to {
09-24 08:56:04.453 14708 14743 I KOReader:     ["h"] = 1080,
09-24 08:56:04.453 14708 14743 I KOReader:     ["w"] = 1920
09-24 08:56:04.453 14708 14743 I KOReader: }

second config change

09-24 08:56:14.996  1852 28767 I ActivityManager: Override config changes=20000480 {1.0 262mcc7mnc [de_DE] ldltr sw400dp w400dp h687dp 432dpi nrml long port night finger -keyb/v/h -nav/h winConfig={ mBounds=Rect(0, 0 - 1920, 1080) mAppBounds=Rect(0, 0 - 1080, 1920) mWindowingMode=fullscreen mActivityType=undefined} s.122} for displayId=0
09-24 08:56:15.079 14708 14708 V KOReader: [NativeSurfaceView] surface changed {
09-24 08:56:15.079 14708 14708 V KOReader:   width:  1080
09-24 08:56:15.079 14708 14708 V KOReader:   height: 1920
09-24 08:56:15.079 14708 14708 V KOReader: }
09-24 08:56:15.272 14708 14743 I KOReader:  Resizing screen to {
09-24 08:56:15.272 14708 14743 I KOReader:     ["h"] = 1920,
09-24 08:56:15.272 14708 14743 I KOReader:     ["w"] = 1080
09-24 08:56:15.272 14708 14743 I KOReader: }

Probably we miss some event to resize the FM in https://github.com/koreader/koreader/blob/master/frontend/device/android/device.lua#L141-L151, but I don't know if something like that is available.

@NiLuJe
Copy link
Member

NiLuJe commented Sep 28, 2020

IIRC, it's as easy as re-init'ing the FM or something.

In a hurry right now, but try to dig up the PR where @yparitcher implemented Landscape in the FM, I seem to remember we discussed this at one point in there.

@yparitcher
Copy link
Member

see:

function FileManager:onSetRotationMode(rotation)
if rotation ~= nil and rotation ~= Screen:getRotationMode() then
Screen:setRotationMode(rotation)
if self.instance then
self:reinit(self.instance.path, self.instance.focused_file)
UIManager:setDirty(self.instance.banner, function()
return "ui", self.instance.banner.dimen
end)
end
end
return true
end

specifically

self:reinit(self.instance.path, self.instance.focused_file)

you might want to borrow from this, or just broadcast "SetRotationMode" directly.

@yparitcher
Copy link
Member

also input.lua for how we deal with gyro events on kindle/kobo.

local old_rotation_mode = self.device.screen:getRotationMode()
if self.device:isGSensorLocked() then
local old_screen_mode = self.device.screen:getScreenMode()
if rotation_mode and rotation_mode ~= old_rotation_mode and screen_mode == old_screen_mode then
-- Cheaper than a full SetRotationMode event, as we don't need to re-layout anything.
self.device.screen:setRotationMode(rotation_mode)
local UIManager = require("ui/uimanager")
UIManager:onRotation()
end
else
if rotation_mode and rotation_mode ~= old_rotation_mode then
return Event:new("SetRotationMode", rotation_mode)
end
end

@pazos
Copy link
Member

pazos commented Oct 1, 2020

@yparitcher: thanks for the hints. Feel free to give it a try if you wish. I'm very much done with android nonsense.

Hint: the issue described by OP just affects devices with HW rotation. I cannot reproduce in legacy emulators (API17).

@pazos
Copy link
Member

pazos commented Oct 12, 2020

I fixed HW/SW orientation mismatches in koreader/android-luajit-launcher@00d9e87

What's left is to handle FM rescale to the new screen size in android. I didn't follow the codepath but the way HW orientation works is encapsulated in framebuffer:getRotationMode() and framebuffer:setRotationMode(mode)

Any UI resize event that happens before framebuffer:setRotationMode(mode) will be performed according to last android surface size (which is wrong because we didn't perform the surface recreate yet).

Also, in android, a orientation change triggers a "config change" event. We use that to know when the surface is actually recreated, get its new size and send the UI events needed to rescale the reader

TL;DR:

  1. setRotationMode discards previous resize events
  2. SetDimensions, ScreenResize and RedrawCurrentPage are ok to rescale the reader app, but no the FM.
  3. missing event?

@yparitcher
Copy link
Member

try Event:new("SetRotationMode", rotation_mode)

as in the code above #6704 (comment)

@pazos
Copy link
Member

pazos commented Oct 12, 2020

try Event:new("SetRotationMode", rotation_mode)

Yup, that would make a loop!!.

I will try to do the FM instance reinit when the new surface is ready.

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

Successfully merging a pull request may close this issue.

4 participants