stoli / Metropolis

Continuation of Metropolis skin for XBMC
Other
23 stars 21 forks source link

CoverFlow view type doesn't show posters in Apple iTunes Trailers addon #295

Closed jingai closed 9 years ago

MacGyverr commented 9 years ago

screenshot000

I wonder why?

MacGyverr commented 9 years ago

I may have fixed this, but at least it's smaller (I deleted 3000 lines, it's now a fourth the original size). It should allow for easier artwork changes.

screenshot000

jingai commented 9 years ago

Cool, I'll look at it when I get a chance.

Noting the commit here for reference: https://github.com/MacGyverr/skin.metropolis-personal/commit/d850aa9be326a00a4ace3b0a5775712c9454ec83

MacGyverr commented 9 years ago

It might have slowed it down though, I suck at optimization so you might need to see if it moves too slowly. Hopefully you can see a way to speed it up. It works great on my i7 :)

jingai commented 9 years ago

I'll look at it closely, especially since a lot of folks are running on set-top boxes with minimal hardware.

jingai commented 9 years ago

Had a chance to look at it. Really good work!

The only issue I see is it doesn't fallback to DefaultThumb.png while it's caching the next images. This is easiest to see in the Trailers addon.

edit: I see why -- it's the conditionals in the thumb variables. Fixing now and will commit this.

edit2: or not.. hmm.. not sure why it's doing that.

jingai commented 9 years ago

Well I'm not going to worry about the caching thing too much. But there are a couple more issues.

I have test library here that only has 4 movies. On this view, it doesn't always show all 4 at once depending on where the user is in the list.

On 3rd item: screenshot001

On 2nd item: screenshot002

The second issue is that if the item has no Poster, ListItem.Icon defaults to a screencap from the video, which obviously doesn't have the correct aspect ratio:

screenshot000

MacGyverr commented 9 years ago

Is the speed ok for you though? I hadn't tested it on only 4 items, but I'm sure it's a simple fix, I'll look into it tomorrow. I was concerned about the speed, my i7 does a fine job, but my single core Atom, not so much. Maybe just me though.

jingai commented 9 years ago

I don't see any particular reason it should be slower than it was before, just from looking at the code. To be honest, the Coverflow view has always been a bit sluggish on minimal hardware anyway. edit: in other words, I think it's OK to just say that this particular view requires better hardware.

Utilizing the preloaditems tag may help, not sure. The old one didn't use it though. Are you finding that the new view is significantly slower on the machine with the Atom processor?

edit: ahh, never mind about the preloaditems thing -- didn't realize it's not a list at all, so that won't be applicable here.

jingai commented 9 years ago

So I'm guessing the original problem was that the iTunes Trailer addon isn't setting ListItem.Art(Poster), but it is setting ListItem.Icon?

If that's the case, the simple thing to do is to only utilize ListItem.Icon if inside an addon currently.

jingai commented 9 years ago

Working from https://github.com/stoli/Metropolis/tree/coverflow

Don't pull that into your master yet though, as I'm going to force-push updates. Just create a new local tracking branch.

The fallback image thing has been fixed. It only falls back to ListItem.Icon if currently inside an addon.

jingai commented 9 years ago

Noting here that the watched overlay flag should be on the lower-right corner of the image, not near the title text.

MacGyverr commented 9 years ago

Using the variables starting at line 256 solves the image issue, but I was hoping for the huge size reduction and a more easily movable coverflow "blob".

jingai commented 9 years ago

Closing this and opening Issue #302 for the problem with short lists.