[PATCH] menu_entry: Disable cursor during update_screen()

[PATCH] menu_entry: Disable cursor during update_screen()

[Thomas Huth]

When running grub in a VGA console of a KVM pseries guest on PowerPC,
you can see the cursor sweeping over the whole line when entering a
character in editor mode. This is visible because grub always refreshes
the whole line when entering a character in editor mode, and drawing
characters is quite a slow operation with the firmware used for the
powerpc pseries guests (SLOF).
To avoid this ugliness, the cursor should be disabled when refreshing
the screen contents during update_screen().

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 grub-core/normal/menu_entry.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/grub-core/normal/menu_entry.c b/grub-core/normal/menu_entry.c
index 62c7e16..68359eb 100644
--- a/grub-core/normal/menu_entry.c
+++ b/grub-core/normal/menu_entry.c
@@ -250,6 +250,8 @@ update_screen (struct screen *screen, struct per_term_screen *term_screen,
       mode = ALL_LINES;
     }
 
+  grub_term_setcursor (term_screen->term, 0);
+
   if (mode != NO_LINE)
     {
       /* Draw lines. This code is tricky, because this must calculate logical
@@ -357,6 +359,8 @@ update_screen (struct screen *screen, struct per_term_screen *term_screen,
 
     }
 
+  grub_term_setcursor (term_screen->term, 1);
+
   grub_term_refresh (term_screen->term);
 }
 
-- 
1.8.3.1

Re: [PATCH] menu_entry: Disable cursor during update_screen()

[Andrei Borzenkov]

29.01.2016 17:26, Thomas Huth пишет:
> When running grub in a VGA console of a KVM pseries guest on PowerPC,
> you can see the cursor sweeping over the whole line when entering a
> character in editor mode. This is visible because grub always refreshes
> the whole line when entering a character in editor mode, and drawing
> characters is quite a slow operation with the firmware used for the
> powerpc pseries guests (SLOF).
> To avoid this ugliness, the cursor should be disabled when refreshing
> the screen contents during update_screen().
> 

While it looks OK - any reason it is not present in Fedora grub2 GIT?

> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  grub-core/normal/menu_entry.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/grub-core/normal/menu_entry.c b/grub-core/normal/menu_entry.c
> index 62c7e16..68359eb 100644
> --- a/grub-core/normal/menu_entry.c
> +++ b/grub-core/normal/menu_entry.c
> @@ -250,6 +250,8 @@ update_screen (struct screen *screen, struct per_term_screen *term_screen,
>        mode = ALL_LINES;
>      }
>  
> +  grub_term_setcursor (term_screen->term, 0);
> +
>    if (mode != NO_LINE)
>      {
>        /* Draw lines. This code is tricky, because this must calculate logical
> @@ -357,6 +359,8 @@ update_screen (struct screen *screen, struct per_term_screen *term_screen,
>  
>      }
>  
> +  grub_term_setcursor (term_screen->term, 1);
> +
>    grub_term_refresh (term_screen->term);
>  }
>  
> 

Re: [PATCH] menu_entry: Disable cursor during update_screen()

[Thomas Huth]

 Hi,

On 30.01.2016 08:33, Andrei Borzenkov wrote:
> 29.01.2016 17:26, Thomas Huth пишет:
>> When running grub in a VGA console of a KVM pseries guest on PowerPC,
>> you can see the cursor sweeping over the whole line when entering a
>> character in editor mode. This is visible because grub always refreshes
>> the whole line when entering a character in editor mode, and drawing
>> characters is quite a slow operation with the firmware used for the
>> powerpc pseries guests (SLOF).
>> To avoid this ugliness, the cursor should be disabled when refreshing
>> the screen contents during update_screen().
>>
> 
> While it looks OK - any reason it is not present in Fedora grub2 GIT?

Sorry, I didn't quite got your question ... do you mean the problem can
not be seen with the version from Fedora? If so, how did you start QEMU
to run the guest?

 Thomas

Re: [PATCH] menu_entry: Disable cursor during update_screen()

[Thomas Huth]

2016-01-29 17:26, Thomas Huth wrote:
>> When running grub in a VGA console of a KVM pseries guest on PowerPC,
>> you can see the cursor sweeping over the whole line when entering a
>> character in editor mode. This is visible because grub always refreshes
>> the whole line when entering a character in editor mode, and drawing
>> characters is quite a slow operation with the firmware used for the
>> powerpc pseries guests (SLOF).
>> To avoid this ugliness, the cursor should be disabled when refreshing
>> the screen contents during update_screen().
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  grub-core/normal/menu_entry.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/grub-core/normal/menu_entry.c b/grub-core/normal/menu_entry.c
>> index 62c7e16..68359eb 100644
>> --- a/grub-core/normal/menu_entry.c
>> +++ b/grub-core/normal/menu_entry.c
>> @@ -250,6 +250,8 @@ update_screen (struct screen *screen, struct per_term_screen *term_screen,
>>        mode = ALL_LINES;
>>      }
>>  
>> +  grub_term_setcursor (term_screen->term, 0);
>> +
>>    if (mode != NO_LINE)
>>      {
>>        /* Draw lines. This code is tricky, because this must calculate logical
>> @@ -357,6 +359,8 @@ update_screen (struct screen *screen, struct per_term_screen *term_screen,
>>  
>>      }
>>  
>> +  grub_term_setcursor (term_screen->term, 1);
>> +
>>    grub_term_refresh (term_screen->term);
>>  }

Ping?

Re: [PATCH] menu_entry: Disable cursor during update_screen()

[Andrei Borzenkov]

31.01.2016 18:02, Thomas Huth пишет:
>  Hi,
> 
> On 30.01.2016 08:33, Andrei Borzenkov wrote:
>> 29.01.2016 17:26, Thomas Huth пишет:
>>> When running grub in a VGA console of a KVM pseries guest on PowerPC,
>>> you can see the cursor sweeping over the whole line when entering a
>>> character in editor mode. This is visible because grub always refreshes
>>> the whole line when entering a character in editor mode, and drawing
>>> characters is quite a slow operation with the firmware used for the
>>> powerpc pseries guests (SLOF).
>>> To avoid this ugliness, the cursor should be disabled when refreshing
>>> the screen contents during update_screen().
>>>
>>
>> While it looks OK - any reason it is not present in Fedora grub2 GIT?
> 
> Sorry, I didn't quite got your question ... do you mean the problem can
> not be seen with the version from Fedora? If so, how did you start QEMU
> to run the guest?
> 

I usually expect that patches from distributions initially appear in
distribution packages, so I was surprised that I could not see it in
Fedora package.

Re: [PATCH] menu_entry: Disable cursor during update_screen()

[Thomas Huth]

 Hi Andrei,

On 13.02.2016 08:41, Andrei Borzenkov wrote:
> 31.01.2016 18:02, Thomas Huth пишет:
>>
>> On 30.01.2016 08:33, Andrei Borzenkov wrote:
>>> 29.01.2016 17:26, Thomas Huth пишет:
>>>> When running grub in a VGA console of a KVM pseries guest on PowerPC,
>>>> you can see the cursor sweeping over the whole line when entering a
>>>> character in editor mode. This is visible because grub always refreshes
>>>> the whole line when entering a character in editor mode, and drawing
>>>> characters is quite a slow operation with the firmware used for the
>>>> powerpc pseries guests (SLOF).
>>>> To avoid this ugliness, the cursor should be disabled when refreshing
>>>> the screen contents during update_screen().
>>>
>>> While it looks OK - any reason it is not present in Fedora grub2 GIT?
>>
>> Sorry, I didn't quite got your question ... do you mean the problem can
>> not be seen with the version from Fedora? If so, how did you start QEMU
>> to run the guest?
> 
> I usually expect that patches from distributions initially appear in
> distribution packages, so I was surprised that I could not see it in
> Fedora package.

Ah, ok, simple answer: Although I'm working for Red Hat, I am not
involved in the Fedora project (yet). And the problem with the cursor
during update_screen() has been found while testing RHEL, not Fedora, so
discussing this issue directly upstream here seems to make more sense
for me (and if the patch is fine for you and it gets included, Fedora
will automatically benefit from this, too, once they update to a new
version of grub).

 Regards,
  Thomas

Re: [PATCH] menu_entry: Disable cursor during update_screen()

[Andrei Borzenkov]

29.01.2016 17:26, Thomas Huth пишет:
> When running grub in a VGA console of a KVM pseries guest on PowerPC,
> you can see the cursor sweeping over the whole line when entering a
> character in editor mode. This is visible because grub always refreshes
> the whole line when entering a character in editor mode, and drawing
> characters is quite a slow operation with the firmware used for the
> powerpc pseries guests (SLOF).
> To avoid this ugliness, the cursor should be disabled when refreshing
> the screen contents during update_screen().
> 

Applied, thanks!

> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  grub-core/normal/menu_entry.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/grub-core/normal/menu_entry.c b/grub-core/normal/menu_entry.c
> index 62c7e16..68359eb 100644
> --- a/grub-core/normal/menu_entry.c
> +++ b/grub-core/normal/menu_entry.c
> @@ -250,6 +250,8 @@ update_screen (struct screen *screen, struct per_term_screen *term_screen,
>        mode = ALL_LINES;
>      }
>  
> +  grub_term_setcursor (term_screen->term, 0);
> +
>    if (mode != NO_LINE)
>      {
>        /* Draw lines. This code is tricky, because this must calculate logical
> @@ -357,6 +359,8 @@ update_screen (struct screen *screen, struct per_term_screen *term_screen,
>  
>      }
>  
> +  grub_term_setcursor (term_screen->term, 1);
> +
>    grub_term_refresh (term_screen->term);
>  }
>  
>