jellyfin / jellyfin-roku

The Official Roku Client for Jellyfin
https://jellyfin.org
GNU General Public License v2.0
449 stars 137 forks source link

Homescreen posters have blue border on older roku #321

Closed cewert closed 3 years ago

cewert commented 3 years ago

Describe the bug When using an older roku device, posters on the home screen are displaying blue borders.

To Reproduce

  1. Buy an older roku(3500X in my case)
  2. Run the app

Expected behavior App looks and performs the same as newer devices

Logs N/A

Screenshots IMG_20201201_131518

Additional context Bug discovered reviewing #320 Roku device thats affected: Model - 3500X(1st gen streaming stick) Roku OS - 9.4.0 Using latest version of master and latest stable server

cewert commented 3 years ago

Did a bunch of testing and it looks like this bug was introduced in #223 when the blue background was initially added since the commit before looks fine for me

neilsb commented 3 years ago

Weirdly it's not even "consistent". When scrolling along a row, items will gain and lose the blue lines round them.

neilsb commented 3 years ago

So having looked into it a bit more, I think this is due to not having used the divisible by 3 rule from #74. The older Roku's seem to be outputting 720p which will result in this. The hiding of the background in the PR will stop this issue, but perhaps we should look at updating the screens a little to take into account the divisible by 3.

Additionally, in the "Library Detail" view, only the top left corner of the poster was rounded in old Rokus (720p). This seems to be another quirk in that the while all elements are resized to the appropriate resolution for 1080/720, the mask image for a MaskGroup is not scaled. In order to fix this we need to manually specify the mask size (maskSize=[193,283]). I'll amend the PR to fix this (once I've check there are no other Masks that may be affected) but this will, unfortunatly, require having logic based on the output screen resolution.

cewert commented 3 years ago

I think this is due to not having used the divisible by 3 rule from #74

Good call. That definitely seems to be the case.

the mask image for a MaskGroup is not scaled

Not only does it not scale, it won't even work on some Roku devices "MaskGroup nodes only work on players that support OpenGL" (source) (list of devices without openGL)

I didn't realize we had code that wouldn't run on all devices. This kind of blew my mind. So not only should we be testing our app in 1080p and 720p, but we should also be testing some code with specific roku models? yikes

neilsb commented 3 years ago

I didn't realize we had code that wouldn't run on all devices. This kind of blew my mind. So not only should we be testing our app in 1080p and 720p, but we should also be testing some code with specific roku models? yikes

With the image masks, if they fail it's only going to affect aesthetics (i.e. rounded corners on the posters, and the fading of the Thumbnails on the TV Guide) so fortunately I don't think that's critical.

cewert commented 3 years ago

Sure, in theory the maskgroup doesn't load and everything functions but just looks worse. But only in theory, right? We'd have to test to know for sure.

I also think this puts us in a position where we should be tracking all the code in our app that functions in two different ways depending on which roku device you're using. That way we can document it as such and users would know what to expect as well as preventing bug reports when one users jellyfin app shows rounded corners but his buddies doesn't and he rightfully assumes it's a bug.

Just seems like a lot of extra maintenance for something that's not "critical" but that's my 2 cents

neilsb commented 3 years ago

Interestingly, I've got a Roku LT here for testing, which doesn't support OpenGL according to the hardware specifications but it still uses the mask to display the rounded corners.

I totally see your point, however at least the Masks degrade gracefully, whereas Roku OS Updates can (and do) introduce new brightscript language features that will definitely not work on non-upgradable devices (if they are used). Currently when publishing the client a "minimum OS version" is specified and I think it has just been dropped it to 9.1 or something as a test, since some users were not seeing the Jellyfin app in the store, and the minimum OS version could be a reason.

There is definitely a difficulty between providing a client that works on old devices; effort in documenting, supporting and developing the application; and providing a modern, attractive, feature rich client that shows off what Jellyfin can do. The tricky bit will be balancing all of those.