Patrick Gleichmann wrote:
Hi, thanks for this bug report.
I really like this patch and would like to know if there is a fix for vdr-1.3.26 available?
@Klaus: Would you consider adding this functionality to core vdr (wrap around menus)?
Best Regards, C.Y.M.
C.Y.M wrote:
Patrick Gleichmann wrote:
Hi, thanks for this bug report.
I really like this patch and would like to know if there is a fix for vdr-1.3.26 available?
@Klaus: Would you consider adding this functionality to core vdr (wrap around menus)?
Well, since I was at it, anyway, I finally have adopted this.
However, I didn't use the original code from Patrick "as is", because it
- would have looped endlessly if there was no selectable item - made wrapping permanent - which at least I don't like - moved cOsdMenu::CursorUp() after cOsdMenu::CursorDown() for no reason - didn't adhere to the VDR coding style
Please try the attached code sequence, which is a drop in replacement for the functions cOsdMenu::CursorUp/Down() and cOsdMenu::PageUp/Down().
For testing there is currently a
#define DO_WRAP 1
at the beginning of this code. Set it to 0 if you want to turn off wrapping. There will be a setup option for this in the final version.
Please let me know whether this works as expected, so I can avoid breaking this again ;-)
Klaus
Klaus Schmidinger wrote:
C.Y.M wrote:
Patrick Gleichmann wrote:
Hi, thanks for this bug report.
I really like this patch and would like to know if there is a fix for vdr-1.3.26 available?
@Klaus: Would you consider adding this functionality to core vdr (wrap around menus)?
Well, since I was at it, anyway, I finally have adopted this.
However, I didn't use the original code from Patrick "as is", because it
- would have looped endlessly if there was no selectable item
- made wrapping permanent - which at least I don't like
- moved cOsdMenu::CursorUp() after cOsdMenu::CursorDown() for no reason
- didn't adhere to the VDR coding style
Please try the attached code sequence, which is a drop in replacement for the functions cOsdMenu::CursorUp/Down() and cOsdMenu::PageUp/Down().
For testing there is currently a
#define DO_WRAP 1
at the beginning of this code. Set it to 0 if you want to turn off wrapping. There will be a setup option for this in the final version.
Please let me know whether this works as expected, so I can avoid breaking this again ;-)
Thank you, Klaus. This works great. I have just one suggestion. There is a patch that comes with Text2skin that allows the OSD to display the "Stop Recording ..." message at the bottom of the menu. Without this patch, it is not visible when using skins.
Best Regards, C.
--- vdr-1.3.26/osdbase.c.orig 2005-06-18 03:55:26.000000000 -0700 +++ vdr-1.3.26/osdbase.c 2005-06-18 04:19:40.000000000 -0700 @@ -261,56 +261,67 @@ return item && item->Selectable(); }
+#define DO_WRAP 1//XXX void cOsdMenu::CursorUp(void) { - if (current > 0) { - int tmpCurrent = current; - while (--tmpCurrent >= 0 && !SelectableItem(tmpCurrent)) - ; - if (tmpCurrent < 0) - return; - if (tmpCurrent >= first) - DisplayCurrent(false); - current = tmpCurrent; - if (current < first) { - first = first > displayMenuItems - 1 ? first - (displayMenuItems - 1) : 0; - if (Setup.MenuScrollPage) - current = !SelectableItem(first) ? first + 1 : first; - Display(); + int tmpCurrent = current; + int lastOnScreen = first + displayMenuItems - 1; + int last = Count() - 1; + while (--tmpCurrent != current) { + if (tmpCurrent < 0) { + if (DO_WRAP) + tmpCurrent = last; + else + return; + } + if (SelectableItem(tmpCurrent)) + break; } - else - DisplayCurrent(true); + if (first <= tmpCurrent && tmpCurrent <= lastOnScreen) + DisplayCurrent(false); + current = tmpCurrent; + if (current < first) { + first = Setup.MenuScrollPage ? max(0, current - displayMenuItems + 1) : current; + Display(); } + else if (current > lastOnScreen) { + first = max(0, current - displayMenuItems + 1); + Display(); + } + else + DisplayCurrent(true); }
void cOsdMenu::CursorDown(void) { - int last = Count() - 1; + int tmpCurrent = current; int lastOnScreen = first + displayMenuItems - 1; - - if (current < last) { - int tmpCurrent = current; - while (++tmpCurrent <= last && !SelectableItem(tmpCurrent)) - ; - if (tmpCurrent > last) - return; - if (tmpCurrent <= lastOnScreen) - DisplayCurrent(false); - current = tmpCurrent; - if (current > lastOnScreen) { - first += displayMenuItems - 1; - lastOnScreen = first + displayMenuItems - 1; - if (lastOnScreen > last) { - first = last - (displayMenuItems - 1); - lastOnScreen = last; + int last = Count() - 1; + while (++tmpCurrent != current) { + if (tmpCurrent > last) { + if (DO_WRAP) + tmpCurrent = 0; + else + return; } - if (Setup.MenuScrollPage) - current = !SelectableItem(lastOnScreen) ? lastOnScreen - 1 : lastOnScreen; - Display(); + if (SelectableItem(tmpCurrent)) + break; } - else - DisplayCurrent(true); + if (first <= tmpCurrent && tmpCurrent <= lastOnScreen) + DisplayCurrent(false); + current = tmpCurrent; + if (current > lastOnScreen) { + first = Setup.MenuScrollPage ? current : max(0, current - displayMenuItems + 1); + if (first + displayMenuItems > last) + first = max(0, last - displayMenuItems + 1); + Display(); } + else if (current < first) { + first = current; + Display(); + } + else + DisplayCurrent(true); }
void cOsdMenu::PageUp(void) @@ -343,15 +354,21 @@ Display(); DisplayCurrent(true); } + else if (DO_WRAP) + CursorUp(); }
-void cOsdMenu::PageDown(void) +void cOsdMenu::PageDown(void) { int oldCurrent = current; int oldFirst = first; current += displayMenuItems; first += displayMenuItems; int last = Count() - 1; + if (current > last) + current = last; + if (first + displayMenuItems > last) + first = max(0, last - displayMenuItems + 1); int tmpCurrent = current; while (!SelectableItem(tmpCurrent) && ++tmpCurrent <= last) ; @@ -371,6 +388,8 @@ Display(); DisplayCurrent(true); } + else if (DO_WRAP) + CursorDown(); }
void cOsdMenu::Mark(void)
--- vdr-1.3.26/osdbase.c.orig 2005-06-18 04:19:40.000000000 -0700 +++ vdr-1.3.26/osdbase.c 2005-06-18 04:28:23.000000000 -0700 @@ -187,6 +187,7 @@ subMenu->Display(); return; } + displayMenuItems = displayMenu->MaxItems(); displayMenu->SetMessage(mtStatus, NULL); displayMenu->Clear(); cStatus::MsgOsdClear(); @@ -297,6 +298,7 @@ int tmpCurrent = current; int lastOnScreen = first + displayMenuItems - 1; int last = Count() - 1; + displayMenuItems = displayMenu->MaxItems(); while (++tmpCurrent != current) { if (tmpCurrent > last) { if (DO_WRAP)
C.Y.M wrote:
...
Please try the attached code sequence, which is a drop in replacement for the functions cOsdMenu::CursorUp/Down() and cOsdMenu::PageUp/Down().
For testing there is currently a
#define DO_WRAP 1
at the beginning of this code. Set it to 0 if you want to turn off wrapping. There will be a setup option for this in the final version.
Please let me know whether this works as expected, so I can avoid breaking this again ;-)
Thank you, Klaus. This works great. I have just one suggestion. There is a patch that comes with Text2skin that allows the OSD to display the "Stop Recording ..." message at the bottom of the menu. Without this patch, it is not visible when using skins.
Best Regards, C.
--- vdr-1.3.26/osdbase.c.orig 2005-06-18 03:55:26.000000000 -0700 +++ vdr-1.3.26/osdbase.c 2005-06-18 04:19:40.000000000 -0700 ...
--- vdr-1.3.26/osdbase.c.orig 2005-06-18 04:19:40.000000000 -0700 +++ vdr-1.3.26/osdbase.c 2005-06-18 04:28:23.000000000 -0700 ...
Which of these is it you would like to have added? The first one looks just like what I've posted (just as a diff). I can't imagine that you want me to change the whole thing again?
So please give _exact_ details what you want - and why. The default skins can display the "Stop recording" message(s), so where's the problem?
Klaus
Klaus Schmidinger wrote:
Which of these is it you would like to have added? The first one looks just like what I've posted (just as a diff).
Yes, the first one (wrap menu cursor) is just a diff of the patch you posted and should apply to vanilla 1.3.26. I just wanted an easy way to apply the second patch (osdbase-maxitems) which is what I was talking about.
I can't imagine that you want me to change the whole thing again?
So please give _exact_ details what you want - and why. The default skins can display the "Stop recording" message(s), so where's the problem?
All of your default skins are fine. I was just referring to all the skins that require the Text2skin plugin. Those two extra lines are needed for Text2skin skins to display the "Stop Recording ..." message(s). Since that "osdbase-maxitems" patch affects some of the new functions you wrote, I thought they might be included as well.
Best Regards, C.
C.Y.M wrote:
... All of your default skins are fine. I was just referring to all the skins that require the Text2skin plugin. Those two extra lines are needed for Text2skin skins to display the "Stop Recording ..." message(s). Since that "osdbase-maxitems" patch affects some of the new functions you wrote, I thought they might be included as well.
The "Stop recording" messages are just menu items like all the others - why would they need special treatment?
Klaus
Klaus Schmidinger wrote:
C.Y.M wrote:
... All of your default skins are fine. I was just referring to all the skins that require the Text2skin plugin. Those two extra lines are needed for Text2skin skins to display the "Stop Recording ..." message(s). Since that "osdbase-maxitems" patch affects some of the new functions you wrote, I thought they might be included as well.
The "Stop recording" messages are just menu items like all the others - why would they need special treatment?
I think CYM meant the additional status line at the bottom saying "recording whatever"
Bye
Klaus Schmidinger wrote:
C.Y.M wrote:
... All of your default skins are fine. I was just referring to all the skins that require the Text2skin plugin. Those two extra lines are needed for Text2skin skins to display the "Stop Recording ..." message(s). Since that "osdbase-maxitems" patch affects some of the new functions you wrote, I thought they might be included as well.
The "Stop recording" messages are just menu items like all the others - why would they need special treatment?
Sascha Volkenandt could probably answer that question better than I. I merely fixed his patch to apply to your new functions in osdbase.c.
C.
I think CYM meant the additional status line at the bottom saying "recording whatever"
Actually, no. Its not the status line that doesnt show up. Its when you are recording something, select the Menu button, and scroll down to the last item in the Menu to be able to stop the recording.
C.
C.Y.M wrote:
I think CYM meant the additional status line at the bottom saying "recording whatever"
Actually, no. Its not the status line that doesnt show up. Its when you are recording something, select the Menu button, and scroll down to the last item in the Menu to be able to stop the recording.
Well, yes, because the additional status line is covering the last menu item. The patch reduces the number of lines available when the extra status line is visible.
Bye
Klaus Schmidinger wrote:
Please try the attached code sequence, which is a drop in replacement for the functions cOsdMenu::CursorUp/Down() and cOsdMenu::PageUp/Down().
Just to make sure this is intended behavior:
Before, MenuScrollPage=true: CursorDown on last line of menu moves window one complete page down, focus stays on last line. (eg. each CursorDown scrolls one page from now on)
After, MenuScrollPage=true: CursorDown on last line of menu moves window one complete page down, focus is on first line.
----
Before, MenuScrollPage=false: CursorDown on last line of menu moves window one page - 1 line down, focus is on second line.
After, MenuScrollPage=false: CursorDown on last line of menu moves window one line down, focus is on last line.
With this change, the mode MenuScrollPage=false before is most simillar to MenuScrollPage=true in new versions, except that the focus is on first/last line instead of second/second-last line. The old MenuScrollPage=true behavior is gone. The new MenuScrollPage=false behavior is horribly slow (for me) because of the complete osd redraw for each CursorDown.
Cheers,
Udo
C.Y.M wrote:
Thank you, Klaus. This works great. I have just one suggestion. There is a patch that comes with Text2skin that allows the OSD to display the "Stop Recording ..." message at the bottom of the menu. Without this patch, it is not visible when using skins.
Hopefully it stays out, like any other useless bloat, like skins and stuff like that in general..
Udo Richter wrote:
Klaus Schmidinger wrote:
Please try the attached code sequence, which is a drop in replacement for the functions cOsdMenu::CursorUp/Down() and cOsdMenu::PageUp/Down().
Just to make sure this is intended behavior:
Before, MenuScrollPage=true: CursorDown on last line of menu moves window one complete page down, focus stays on last line. (eg. each CursorDown scrolls one page from now on)
After, MenuScrollPage=true: CursorDown on last line of menu moves window one complete page down, focus is on first line.
Before, MenuScrollPage=false: CursorDown on last line of menu moves window one page - 1 line down, focus is on second line.
After, MenuScrollPage=false: CursorDown on last line of menu moves window one line down, focus is on last line.
With this change, the mode MenuScrollPage=false before is most simillar to MenuScrollPage=true in new versions, except that the focus is on first/last line instead of second/second-last line. The old MenuScrollPage=true behavior is gone. The new MenuScrollPage=false behavior is horribly slow (for me) because of the complete osd redraw for each CursorDown.
The intention of Patrick's change (as far as I understand it) was to make sure that, if the cursor is at item N, a "cursor down" positions it at item N + 1 (N -> N - 1 for "cursor up"). I have to concur with Patrick on this, and that's how it is implemented now.
The old MenuScrollPage=true mode was actually not a "cursor down" function, but rather a "page down" function, so there was an inconsistency there.
If you want to "page down", just use the "Right" key.
Klaus
Luca Olivetti wrote:
C.Y.M wrote:
I think CYM meant the additional status line at the bottom saying "recording whatever"
Actually, no. Its not the status line that doesnt show up. Its when you are recording something, select the Menu button, and scroll down to the last item in the Menu to be able to stop the recording.
Well, yes, because the additional status line is covering the last menu item. The patch reduces the number of lines available when the extra status line is visible.
Well, apparently the text2skin plugin is using a dirty trick here, which the skin mechanism is (currently) not designed to do.
I'm not going to adopt this now - maybe there will be a clean solution after version 1.4...
Klaus
Klaus Schmidinger wrote:
The intention of Patrick's change (as far as I understand it) was to make sure that, if the cursor is at item N, a "cursor down" positions it at item N + 1 (N -> N - 1 for "cursor up"). I have to concur with Patrick on this, and that's how it is implemented now.
The old MenuScrollPage=true mode was actually not a "cursor down" function, but rather a "page down" function, so there was an inconsistency there.
If you want to "page down", just use the "Right" key.
I dont have any problems with that. I've been using MenuuScrollPage=false until now, and I'll be using MenuScrollPage=true from now on, so thats fine with me. 1.3.27 will show if the masses will agree.
I had another idea about long menus: Orientation in them is a bit difficult, you usually dont know very well where you are. And its difficult to see whether you just paged down a complete page, a partial page or just one line. I think this would improve by adding a pseudo scroll bar to long menus, similar to the timebar in channel OSD. The scroll bar would visually show which part of the menu is visible, and would visually show the movement from one page to the next.
For implementation, the skin needs to know about first and count, but thats a small change. The rest would be skin implementation. I would offer to implement it, if needed.
Cheers,
Udo
On 19.06.2005, at 11:09, Klaus Schmidinger wrote:
Udo Richter wrote:
Klaus Schmidinger wrote:
Please try the attached code sequence, which is a drop in replacement for the functions cOsdMenu::CursorUp/Down() and cOsdMenu::PageUp/ Down().
Just to make sure this is intended behavior: Before, MenuScrollPage=true: CursorDown on last line of menu moves window one complete page down, focus stays on last line. (eg. each CursorDown scrolls one page from now on) After, MenuScrollPage=true: CursorDown on last line of menu moves window one complete page down, focus is on first line. :
The intention of Patrick's change (as far as I understand it) was to make sure that, if the cursor is at item N, a "cursor down" positions it at item N + 1 (N -> N - 1 for "cursor up"). I have to concur with Patrick on this, and that's how it is implemented now.
Yes, the cursor moves freely while inside of a single menu "page", and keeps it position when the menu is scrolled 1 item up/down.
Patrick
-- Patrick Gleichmann (mailto:patrick@feedface.com) PGP key > http://www.feedface.com/patrick/pubkey
-
C.Y.M -
Klaus Schmidinger -
Lauri Tischler -
Luca Olivetti -
Patrick Gleichmann -
Udo Richter