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

Slide-in screen navigation behaviour bug after Alpha 1 #21664

Closed
1 of 7 tasks
biner73 opened this issue Jul 9, 2022 · 8 comments · Fixed by #21645
Closed
1 of 7 tasks

Slide-in screen navigation behaviour bug after Alpha 1 #21664

biner73 opened this issue Jul 9, 2022 · 8 comments · Fixed by #21645
Labels
Resolution: Fixed issue was resolved by a code change

Comments

@biner73
Copy link
Contributor

biner73 commented Jul 9, 2022

Bug report

Describe the bug

I have noticed a bug in the slide-in window behaviour (default skin) somewhere after Alpha 1, and reproducible in Alpha 2. When navigating to the Settings screen and returning to the main Movie/TV screens, the slide-in animation is no longer smooth, and just cuts off, leaving a jarring effect. This only happens when going from Settings->Movie/TV, and not the other way around, or between Movie/TV screens. Please see videos attached showing the issue.

Expected Behavior

The screen slide-in behaviour should be smooth between windows.

Actual Behavior

Screen slide-in is not smooth (see attached video clips).

Possible Fix

To Reproduce

Steps to reproduce the behavior:

  1. Install Alpha 2.
  2. Navigate between Settings window and Movie/TV windows.
  3. Note jarring effect, where slide-in is not smooth.

Debuglog

The debuglog can be found here:
kodi.old.log

Screenshots

Here are some links or screenshots to help explain the problem. I don't know how to insert video inline, so attaching the links here.

Alpha 1 and prior correct slide-in behaviour:
https://drive.google.com/file/d/1UcICpdbn80J2qUZOKFTxh5wcCy2s3Uh-/view?usp=sharing

Alpha 2 incorrect behaviour:
https://drive.google.com/file/d/1W_Te6SZzwjPWIgAEU2ENokxAaHriBhDp/view?usp=sharing

Additional context or screenshots (if appropriate)

Here is some additional context or explanation that might help:

Your Environment

Used Operating system:

  • Android

  • iOS

  • tvOS

  • Linux

  • OSX

  • Windows 10 x64 21H2

  • Windows UWP

  • Operating system version/name:

  • Kodi version: Kodi 20 Alpha 2

note: Once the issue is made we require you to update it with new information or Kodi versions should that be required.
Team Kodi will consider your problem report however, we will not make any promises the problem will be solved.

@ksooo
Copy link
Member

ksooo commented Jul 9, 2022

I can confirm this behavior and I even have a patch. @thexai the GUIFontTTF lookup table (you already fixed once after HarfBuzz merge) seems still broken. I have the following patch, what do you say, does this make sense?

EDIT: Maybe the OP describes something else, unrelated to my patch, sorry. Nevertheless I noticed non-smooth animation effects when navigating from one Estuary home screen section to another (most notably when entering or leaving the "TV" section on my Android Shield Pro) and my patch definitely fixes this.

diff --git a/xbmc/guilib/GUIFontTTF.cpp b/xbmc/guilib/GUIFontTTF.cpp
index 7ff382dfee..435cd8cd43 100644
--- a/xbmc/guilib/GUIFontTTF.cpp
+++ b/xbmc/guilib/GUIFontTTF.cpp
@@ -739,9 +739,9 @@ CGUIFontTTF::Character* CGUIFontTTF::GetCharacter(character_t chr, FT_UInt glyph
     return nullptr;
 
   // quick access to ascii chars
-  if (letter < 255 && glyphIndex < 255)
+  if (letter < 255)
   {
-    character_t ch = (style << 8) | glyphIndex;
+    character_t ch = (style << 8) | letter;
 
     if (ch < LOOKUPTABLE_SIZE && m_charquick[ch])
       return m_charquick[ch];
@@ -811,9 +811,9 @@ CGUIFontTTF::Character* CGUIFontTTF::GetCharacter(character_t chr, FT_UInt glyph
   memset(m_charquick, 0, sizeof(m_charquick));
   for (int i = 0; i < m_numChars; i++)
   {
-    if (m_char[i].m_letter < 255 && m_char[i].m_glyphIndex < 255)
+    if (m_char[i].m_letter < 255)
     {
-      character_t ch = ((m_char[i].m_glyphAndStyle & 0xffff0000) >> 8) | m_char[i].m_glyphIndex;
+      character_t ch = ((m_char[i].m_glyphAndStyle & 0xffff0000) >> 8) | m_char[i].m_letter;
 
       if (ch < LOOKUPTABLE_SIZE)
         m_charquick[ch] = m_char + i;
@@ -911,7 +911,6 @@ bool CGUIFontTTF::CacheCharacter(wchar_t letter, uint32_t style, Character* ch,
 
   // set the character in our table
   ch->m_glyphAndStyle = (style << 16) | glyphIndex;
-  ch->m_glyphIndex = glyphIndex;
   ch->m_letter = letter;
   ch->m_offsetX = static_cast<short>(bitGlyph->left);
   ch->m_offsetY = static_cast<short>(m_cellBaseLine - bitGlyph->top);
diff --git a/xbmc/guilib/GUIFontTTF.h b/xbmc/guilib/GUIFontTTF.h
index 130c158cfb..f4ea732a24 100644
--- a/xbmc/guilib/GUIFontTTF.h
+++ b/xbmc/guilib/GUIFontTTF.h
@@ -130,7 +130,6 @@ protected:
     float m_right;
     float m_bottom;
     float m_advance;
-    FT_UInt m_glyphIndex;
     character_t m_glyphAndStyle;
     wchar_t m_letter;
   };

@biner73
Copy link
Contributor Author

biner73 commented Jul 9, 2022

I can confirm this behavior and I even have a patch. @thexai the GUIFontTTF lookup table (you already fixed once after HarfBuzz merge) seems still broken. I have the following patch, what do you say, does this make sense?

EDIT: Maybe the OP describes something else, unrelated to my patch, sorry. Nevertheless I noticed non-smooth animation effects when navigating from one Estuary home screen section to another (most notably when entering or leaving the "TV" section on my Android Shield Pro) and my patch definitely fixes this.

Thanks for taking a look. The problem you described seems similar, but not quite the same (it might be related, I don't know enough about the workings of the gui code). I tried your patch on top of Alpha 2, and it doesn't fix the issue unfortunately.

@ksooo
Copy link
Member

ksooo commented Jul 9, 2022

I tried your patch on top of Alpha 2, and it doesn't fix the issue unfortunately.

Yes, as I wrote I think my fix is for another issue, not for yours.

@thexai
Copy link
Member

thexai commented Jul 9, 2022

@thexai the GUIFontTTF lookup table (you already fixed once after HarfBuzz merge) seems still broken

In what sense? One example?

For me they are unrelated things: probably non-smooth transitions (or behavior changes) may have more to do with:
#21645
#21387
#21481
#21507

@thexai
Copy link
Member

thexai commented Jul 9, 2022

and my patch definitely fixes this

I don't think this patch is correct: it will probably cause swapped/wrong letters as it has in the past: #20130

@sarbes
Copy link
Member

sarbes commented Jul 9, 2022

If I had to guess, it is #21615.

Note that this is not a scientific guess.

@biner73
Copy link
Contributor Author

biner73 commented Jul 9, 2022

@thexai the GUIFontTTF lookup table (you already fixed once after HarfBuzz merge) seems still broken

In what sense? One example?

For me they are unrelated things: probably non-smooth transitions (or behavior changes) may have more to do with: #21645 #21387 #21481 #21507

Thanks @thexai. I've merged the changes in #21645 onto latest master in a test branch. This seems to fix the issue, the transitions slide-in smoothly again. Once #21645 is fully merged into master, I will retest and can close this issue.

@thexai thexai linked a pull request Jul 9, 2022 that will close this issue
13 tasks
@ksooo
Copy link
Member

ksooo commented Jul 9, 2022

In what sense? One example?

@thexai I will open a PR and explain problem and fix.

I don't think this patch is correct

I agree. My PR #21665 follows a different approach.

@thexai thexai added the Resolution: Fixed issue was resolved by a code change label Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Fixed issue was resolved by a code change
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants