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

[xbmc][fix] Fix theme xbt loading that broke during cleanup.in #9960 #10073

Merged
merged 1 commit into from Jul 8, 2016

Conversation

@Paxxi
Copy link
Member

commented Jul 6, 2016

Found a race condition that seems to have been hidden and a screwup during cleanup clobbering state.

The race condition is that g_graphicsContext.GetMediaDir() isn't set to anything when first called, it's only later on during skin loading it's actually populated. If this happens now we fall back to the currently set skin.

@MilhouseVH I've tested with the Titan beta skin, I can't see any difference on the corners when using modern_rounded but works fine setting it to classic so I'm thinking it might be my eyes deceiving me or some change in the beta I tested. Would be great if you or the author can test the changes.

@Paxxi Paxxi added the Type: Fix label Jul 6, 2016

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2016

Thanks, I'll include it in tonight's build (though a bit worried you can't see the difference rounded corners makes, it's fairly clear in the screenshots posted by the user).

@Paxxi

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2016

Yeah I saw it in the posted screenshots so that's why I'm doubting my own vision here. The classic theme option has rounded corners and switching between that and default it clearly changes the corners rounding so it could be something wrong with just the modern_rounded theme in the skin currently.

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2016

I'm pretty sure there's still an issue, though as you say it might be skin related (I've really no idea).

This is how Titan "modern_rounded" looked before PR9960 (ie. build #613):
s1

And now current kodi master (ie. with PR9960) and this PR:
s3

The strange thing is that build #0704x, which is reportedly working correctly (ie. with rounded corners), looks the same as current kodi master to me (ie. second screenshot) so I'm not convinced that reverting PR9960 is actually working as reported - I'll ask the user to test #0704x again.

Then again, if you suspect this might be some race condition, it could be that it works some of the time...

@Paxxi

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2016

maybe we can get @marcelveldt to comment, if I understand it correctly he's the maintainer of the skin.

@Paxxi Paxxi force-pushed the Paxxi:fix_theme branch from d05627a to c277f39 Jul 6, 2016

@Paxxi

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2016

oh nvm, had a look at it again and there was one more thing I failed to fix, have updated the PR and now it should be good to go for tonight @MilhouseVH.
screenshot 10

: m_TimeStamp{0}
, m_themeBundle{false}
{
}

CTextureBundleXBT::CTextureBundleXBT(bool themeBundle)
: m_TimeStamp{0}

This comment has been minimized.

Copy link
@FernetMenta

FernetMenta Jul 6, 2016

Member

is this a tab?

This comment has been minimized.

Copy link
@Paxxi

Paxxi Jul 6, 2016

Author Member

Looks like it, no idea how that got in there, will fix before merging

@@ -21,7 +21,14 @@
#include "TextureBundle.h"

CTextureBundle::CTextureBundle()
: m_useXBT{false}
: m_tbXBT{false}
, m_useXBT{false}

This comment has been minimized.

Copy link
@FernetMenta
@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2016

Looks good now @Paxxi - many thanks!

@marcelveldt

This comment has been minimized.

Copy link

commented Jul 6, 2016

Great work, thanks for the quick fix. This issue was indeed reported on the forums

Pär Björklund
Fix theme xbt loading that broke during cleanup.
Found a race condition that seems to have been hidden and a screwup during cleanup clobbering state.

@Paxxi Paxxi force-pushed the Paxxi:fix_theme branch from c277f39 to d26c5ee Jul 7, 2016

@Paxxi

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2016

jenkins build and merge this please

@jenkins4kodi jenkins4kodi merged commit cd4c875 into xbmc:master Jul 8, 2016

1 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
default I've found some spare time so building this now
Details
jenkins.kodi.tv Yeah yeah I'll get to it when i have some time
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@Paxxi Paxxi added this to the Krypton 17.0-alpha3 milestone Jul 8, 2016

@Paxxi Paxxi deleted the Paxxi:fix_theme branch Mar 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.