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

Fix distorted TV episode posters. Add client-side progress bar and played indicator. #922

Merged
merged 8 commits into from
Jan 27, 2023

Conversation

ApexArray
Copy link
Contributor

@ApexArray ApexArray commented Dec 24, 2022

Changes
✔ Sets loadDisplayMode="scaleToZoom" on TV episode posters to avoid squashing the image.
✔ Removes AddPlayedIndicator and PercentPlayed parameters when retrieving episode images.
✔ Renders client-side progress bar (inspired by the HomeItem component)
✔ Renders client-side "played" indicator (inspired by @1hitsong's pr #882)

Before and after:
image

image

Issues
Fixes #603

Should also improve performance and free up texture memory, since we don't need to call/store a separate image with playback info overlay.

prevents distoring images in episode listing
@ApexArray ApexArray changed the title Fix distorted TV episode posters WIP: Fix distorted TV episode posters Dec 24, 2022
@cewert
Copy link
Member

cewert commented Dec 25, 2022

What if we changed the poster size to be 16:9 aspect ratio and used loadDisplayMode="scaleToFit"? HD Posters would be fixed with no resize and SD 4:3 posters would use the black bars but keep aspect ratio and no zooming. I would prefer to have the black bars than zoom and lose the whole image but that's my 2c.

@ApexArray
Copy link
Contributor Author

I considered that as well, but I figured it was best to default to the same behavior in jellyfin-web. It looks like they’re cropping the image and adding a custom icon/progress bar.

@1hitsong 1hitsong marked this pull request as draft December 28, 2022 13:45
render these client side to ensure they fit in custom image size
@ApexArray ApexArray changed the title WIP: Fix distorted TV episode posters Fix distorted TV episode posters. Add client-side progress bar and played indicator. Dec 28, 2022
@ApexArray
Copy link
Contributor Author

I think this is a good semi-final version of this PR. Thanks @neilsb for pointing me in the right direction on the "played" icon.

@1hitsong, care to take a look?

@ApexArray ApexArray marked this pull request as ready for review December 28, 2022 22:47
<Rectangle id="playedIndicator" color="#00a4dcFF" width="60" height="46" visible="false" translation="[290, 0]">
<Label id="checkmark" width="60" height="42" font="font:SmallestBoldSystemFont" horizAlign="center" vertAlign="bottom" text="✓"/>
</Rectangle>
<Rectangle id="progressBackground" visible="false" color="0x00000098" width="350" height="16" translation="[0,286]">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make the height 8 to match the homepage bar height?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a height of 8, the row selector blocks a good chunk of the progress bar. Do we want to stick with this height, or try to move/resize the progress bar on selection?

Copy link
Contributor Author

@ApexArray ApexArray Dec 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

source/api/Items.brs Show resolved Hide resolved
<LayoutGroup id="main_group" layoutDirection="horiz" itemSpacings="[30]">
<Poster id="poster" width="350" height="300" loadDisplayMode="scaleToZoom">
<Rectangle id="playedIndicator" color="#00a4dcFF" width="60" height="46" visible="false" translation="[290, 0]">
<Label id="checkmark" width="60" height="42" font="font:SmallestBoldSystemFont" horizAlign="center" vertAlign="bottom" text="✓"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's bump up the font size on the check to 35.

Here's what it looks like.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to override font size here? Couldn't find a way to do that, so I've set it over in the TVListDetails.brs init funciton.

@1hitsong
Copy link
Member

To be merged 1/27 unless an issue arises.

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.

Episode screenshots on episodes list have wrong aspect ratio
3 participants