Re: [PATCH] "Natural" menu cursor 0.01
Commit Message
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 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)
Comments
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
>
> 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.
>
Yes, exactly. Thanks for helping me explain that. :)
Regards,
C.
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..
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
@@ -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)