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

Layout/UI poster wall issues in 4k, lots of wasted space? #1774

Closed
Andy2244 opened this issue Jun 12, 2022 · 23 comments · Fixed by #1812
Closed

Layout/UI poster wall issues in 4k, lots of wasted space? #1774

Andy2244 opened this issue Jun 12, 2022 · 23 comments · Fixed by #1812
Labels
bug Something isn't working

Comments

@Andy2244
Copy link
Contributor

Andy2244 commented Jun 12, 2022

Describe the bug

On my Hisense 4k TV the UI layout while in a 4k resolution looks off and has lots of wasted space?
Thats from a Zidoo Z9X box running Android 9, not AndroidTV.

  • the home item rows are unnecessary big for 4k resolution and cant be changed in size via settings
  • all poster wall layouts have massive empty spaces at the sides/bottom
  • the vertical layouts have strangely cut off rows
  • the poster size for "small" in 4k is a little too small and medium too large for my taste, would have liked something in between

Would like to see a layout option that mimics something most Kodi skins do by default and use all available screen-space for poster rows/columns display.

Layouts are "small" and "default" and horizontal/vertical, Images are blurred for privacy reasons

jf_home
jf_horizontal_medium
jf_horizontal_small
jf_vertical_default
jf_vertical_small

Logs

No response

Application version

0.13.6, master

Where did you install the app from?

Sideload

Device information

Zidoo Z9X

Android version

Android 9

Jellyfin server version

10.8

@Andy2244 Andy2244 added the bug Something isn't working label Jun 12, 2022
@Andy2244 Andy2244 changed the title Layout/UI poster wall issues in 4k, lost of free space? Layout/UI poster wall issues in 4k, lots of wasted space? Jun 12, 2022
@anthonylavado
Copy link
Member

Oh wow. Yeah. I tested on a Chromecast with Google TV last night, and all the poster sizes definitely did not get that small, running at 4K native resolution. They also didn't have that weird spacing

@Andy2244
Copy link
Contributor Author

Andy2244 commented Jun 12, 2022

Yeah. I tested on a Chromecast with Google TV last night

I think you can get the same bug on ShieldTV if you enable developer options and than enable the "Show/Allow all resolutions" option. Than the device will run a "different" 4k UI resolution/layout and you get the same spacing, while the native OS settings panels are also much smaller.

I think some extra ui element/text zooming/sizing is going on by default on AndroidTV in 4k.

@Andy2244
Copy link
Contributor Author

Also not sure if related but i see a lot of those errors in the log:
org.jellyfin.androidtv E/ThemeUtils: View class org.jellyfin.androidtv.ui.AsyncImageView is an AppCompat widget that can only be used with a Theme.AppCompat theme (or descendant).

and

org.jellyfin.androidtv I/ViewTarget: Glide treats LayoutParams.WRAP_CONTENT as a request for an image the size of this device's screen dimensions. If you want to load the original image and are ok with the corresponding memory cost and OOMs (depending on the input size), use override(Target.SIZE_ORIGINAL). Otherwise, use LayoutParams.MATCH_PARENT, set layout_width and layout_height to fixed dimension, or use .override() with fixed dimensions.

@nielsvanvelzen
Copy link
Member

Those warnings can be safely ignored. The grid shouldn't look like this though. I have some ideas what could cause it so I'll look into it when I have some time.

@nielsvanvelzen nielsvanvelzen added this to 🚅 Roadmap: Short term in Niels' board (roadmap) Jun 13, 2022
@nielsvanvelzen
Copy link
Member

I've looked into this but from my testing the posters correctly use display pixels and scale with the pixel density as expected. So I don't see any issues with it.

@nielsvanvelzen nielsvanvelzen removed this from 🚅 Roadmap: Short term in Niels' board (roadmap) Jun 13, 2022
@Andy2244
Copy link
Contributor Author

Andy2244 commented Jun 13, 2022

I've looked into this but from my testing the posters correctly use display pixels and scale with the pixel density as expected. So I don't see any issues with it.

I mean something must be wrong/off, the evidence is in the pictures. Is there any debug output i can give you, so you can figure out whats going wrong?

Also can you point me to the code that manages the layout/sizing of those elements?

PS: Do you have a ShieldTV at hand? As noted there you could try the settings i posted.

@nielsvanvelzen
Copy link
Member

PS: Do you have a ShieldTV at hand? As noted there you could try the settings i posted.

Yes I did test on both the emulator (using adb to change window manager settings) and nvidia shield (the mentioned option). On the shield it just changes the UI rendering from 1080p to 4k, which worked fine.

Also can you point me to the code that manages the layout/sizing of those elements?

Start at StdGridFragment.java

@Andy2244
Copy link
Contributor Author

Andy2244 commented Jun 13, 2022

Yes I did test on both the emulator (using adb to change window manager settings) and nvidia shield

mhh any Android 9 based hardware to test on?
My guess is any of the none AndroidTV based sticks/boxes may have this issue.

PS: Finally manage to get debugging working on the zidoo, so i can directly look into the values. Can you give me the expected values i should look at from your 4k testcases?

@nielsvanvelzen
Copy link
Member

Nope, no other hardware to test on. The default screen density is 320 with a resolution of 1920x1080 in the emulator.

@Andy2244
Copy link
Contributor Author

Andy2244 commented Jun 14, 2022

The default screen density is 320 with a resolution of 1920x1080 in the emulator.

Ok in the Android 9 system settings is a option Display Size with [small, default, large, larger, largest] settings.
It is set to default and i get a 1.5 for the ctx.getResources().getDisplayMetrics().density call.

Whats odd is that all JellyfinTV Ui elements scale with this setting correctly, except the poster views?
This means settings/home/detail views all change size accordingly to this setting and i get expected/usable results

Yet for the poster views i only get somewhat reasonable results with [larger, largest] settings, which makes the remaining Ui unusable.

Can i get some more reference data for default values on a 4k TV?

Here are what i get with different Display Size settings:
Small: mCardHeight <148> density <1.275>
Default: mCardHeight <174> density <1.5>
Large: mCardHeight <203> density <1.75>
Larger: mCardHeight <232> density <2.0>

@Andy2244
Copy link
Contributor Author

Andy2244 commented Jun 14, 2022

@nielsvanvelzen ok i see whats going on, all of the grid values are hardcoded inside setGridSizes().

I can adapt those, but i cant get rid of the bottom padding/space?
So no matter what image size i pick, VerticalGridView always has the same bottom spacing/padding?
1

I tried this ((VerticalGridView)mGridView).setPadding(0,0,0,0); but it seems the view is somehow restricted from the parent maybe?
Can i get a hint how this bottom padding/size is defined and can be changed, so i can use all the space?

@Andy2244
Copy link
Contributor Author

Andy2244 commented Jun 14, 2022

ok found it, its the mGridDock = binding.rowsFragment; the rowsFragment in horizontal_grid_browse.xml is limited to this size.

It seems rowsFragment was never adjusted to support a VerticalGridView or why is there so much space wasted on the bottom?

@nielsvanvelzen
Copy link
Member

The vertical grid view is marked as experimental because it really is a hack.

@Andy2244
Copy link
Contributor Author

Andy2244 commented Jun 14, 2022

The vertical grid view is marked as experimental because it really is a hack.

After adapting a 585dp size for rowsFragment and getGridHeight(), i can now use the full screen on both views.
This means now i can fit 3 rows, instead of just 2 for the HorizontalGrid.

Is there a reason that even in the "none experimental" view the size is limited? How many rows you get for a default setup in 4k and do you have this marked empty area also by default?

I think the VerticalGridView should be the default, since that's how the browser app works. Most other media-centers (Kodi/Plex), tools that have poster views also have top->bottom as default view direction.
Its just more natural to scroll top->bottom for a Poster view. No idea why left->right was picked as default for TV?

@nielsvanvelzen
Copy link
Member

The experimental one was added by us and marked experimental because it only works properly 60% of the time. The other one is originally from mediabrowser/emby and works 80% of the time.

I'd say both implementations should be rewritten at some point but I haven't had the time to work on that.

@Andy2244
Copy link
Contributor Author

Andy2244 commented Jun 14, 2022

The experimental one was added by us and marked experimental because it only works properly 60% of the time.

What does this mean? What happens in those 40% and what are the 40% even?

From my point of view both work 100%, i get no crashes and can select/play each highlight element just fine.
I only noticed that sometimes either view will have wrong ordering or triggers weird sorting updates.

@danbezerra
Copy link

Great to see the discussion after my original feedback on the Reddit channel! I agree with Andy2244 that ideally the vertical layout should be the default - not only to keep a consistent look with the Web experience but I also feel it's more natural as well.

However, if fix the horizontal layout is an easier/safer approach then I have no problems using it! The most important step is to address the layout issue (empty space at the bottom 50% of the screen) on devices using Android 9.

Andy - do you have a private build with your fix? I can help by validating if it fixed the issue on my Zidoo while not damaging anything on Chromecast with Google TV.

@Andy2244
Copy link
Contributor Author

Andy2244 commented Jun 14, 2022

I can help by validating if it fixed the issue on my Zidoo while not damaging anything on Chromecast with Google TV.

Still working on rewriting the hardcoded layout setup, which makes no sense given that getDisplayMetrics().density can vary greatly on Android 9 devices and can even be changed in the Android settings.

I think i have some easy fixes, but need to validate them. I will also try to add a user setting "ui scale" where you can override the system density value.

Andy - do you have a private build with your fix?

I'm actually working on a full "Zidoo Edition" fork, that also fully supports the internal VS10 player. I will post the beta1 in the Zidoo Forum/Development and also setup a proper github page if i'm ready.

Here are some preview's of the fixed layouts, still working on the item spacing, view padding and left/right border space.

IMG_20220614_205623717 MP
IMG_20220614_205655939 MP
IMG_20220614_205719233 MP

@danbezerra can you also provide me photos of the homescreen, details-view and poster-views horizontal/vertical (small/default) from your Chromecast , so i can better compare vs Android 9/Zidoo.

I cant remember if the default HorizontalView was able to display 3 rows with the default setting, since it seems the empty bottom space was always there?

@danbezerra
Copy link

@Andy2244, see attached the zip file with the images. Horizontal Medium uses 3 rows with all default settings.

Jellyfin AndroidTV.zip

@Andy2244
Copy link
Contributor Author

ok thanks, will try to get to something similar.

@Andy2244
Copy link
Contributor Author

Andy2244 commented Jun 16, 2022

@nielsvanvelzen ok i see why the vertical grid is called "Experimental", there is not even a proper view definition for it.
The vertical view has no definitions at all, like in horizontal_grid.xml.

So it uses some large default padding and inconsistent spacing's.

I see what the fundamental issue is with the grids, that until the fragment is actually rendered we cant get the actual height/width and all this guesswork in setGridSizes() needs to take place.

I have found a way to get the actual fragment values, so we can always calculate the exact fitting row/columns for the presenter's, but need to-do more tests before i can setup a PR.
BTW i honestly have no idea how the vertical view even "works" on AndroidTV devices, since the padding is completely wrong.

PS: There is also a bug around the delayed loadMoreItemsIfNeeded(), which messes with other filters and screws-up the grid.
So sometimes if you filter by "watched" or other stuff, the first item will be wrong and a duplicate or you get a empty grid, while there should be entries.
This bug only happens if you move the selected item grid position far enough once, so the loadMoreItemsIfNeeded() triggers.

@nielsvanvelzen
Copy link
Member

If you're able to calculate the amount of rows/columns for the different sizes/image types that would be awesome. I'm a bit busy for the next few weeks so it might take bit until I'm able to review it, so take your time 😉

@Andy2244
Copy link
Contributor Author

@danbezerra just released my beta1 which include the layout fixes here: https://github.com/Andy2244/jellyfin-androidtv-zidoo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants