[PATCH] fix entry editor screen screen layout corruption in trunk

[PATCH] fix entry editor screen screen layout corruption in trunk

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 5254 bytes --]

See screenshot for what I get using current trunk.

I do not pretend to fully understand all this code so review is
appreciated. So far it does fix both gfxterm and console cases for me.

Patch follows.

---

From: Andrey Borzenkov <arvidjaar@gmail.com>
Subject: [PATCH] fix entry editor screen corruption

Fixes the following issues after rev 7137

1. right margines were erased on wrapped lines (with contibuation char)

2. left margines were erased on continuation lines

3. max_lines was effectively ignored by gfxterm, so last line was printed
outside of editor window, overwriting bottom border and probably beyond

4. menu_entry:update_screen() unconditionally padded last line with
spaces, but print_ucs4_menu() would move to next screen position if
line was wrapped. This caused bottom border to be partially overwritten
with spaces. The patch moves line padding to term.c. It also
makes behaviour consistent with print_ucs4_terminal which did padding
itself.

Signed-off-by: Andrey Borzenkov <arvidjaar@gmail.com>

---
 grub-core/normal/menu_entry.c |   10 ----------
 grub-core/normal/term.c       |   29 +++++++++++++++++++++++------
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/grub-core/normal/menu_entry.c b/grub-core/normal/menu_entry.c
index 7cd67f3..cc977b5 100644
--- a/grub-core/normal/menu_entry.c
+++ b/grub-core/normal/menu_entry.c
@@ -256,7 +256,6 @@ update_screen (struct screen *screen, struct per_term_screen *term_screen,
 
 	  if (i == region_start || linep == screen->lines + screen->line)
 	    {
-	      int sp;
 	      grub_term_gotoxy (term_screen->term, GRUB_TERM_LEFT_BORDER_X
 				+ GRUB_TERM_MARGIN + 1, (y < 0 ? 0 : y)
 				+ GRUB_TERM_FIRST_ENTRY_Y);
@@ -271,14 +270,9 @@ update_screen (struct screen *screen, struct per_term_screen *term_screen,
 				    term_screen->num_entries
 				    - ((y > 0) ? y : 0), '\\',
 				    *pos);
-	      sp = grub_term_entry_width (term_screen->term)
-		 - (*pos)[linep->len].x;
-	      if (sp > 0)
-		grub_print_spaces (term_screen->term, sp);
  	    }
 	  else if (i > region_start && mode == ALL_LINES)
 	    {
-	      int sp;
 	      grub_term_gotoxy (term_screen->term, GRUB_TERM_LEFT_BORDER_X
 				+ GRUB_TERM_MARGIN + 1, (y < 0 ? 0 : y)
 				+ GRUB_TERM_FIRST_ENTRY_Y);
@@ -293,10 +287,6 @@ update_screen (struct screen *screen, struct per_term_screen *term_screen,
 				    term_screen->num_entries
 				    - ((y > 0) ? y : 0), '\\',
 				    *pos);
-	      sp = grub_term_entry_width (term_screen->term)
-		- (*pos)[linep->len].x;
-	      if (sp > 0)
-		grub_print_spaces (term_screen->term, sp);
 	    }
 	  y += get_logical_num_lines (linep, term_screen);
 	  if (y >= term_screen->num_entries)
diff --git a/grub-core/normal/term.c b/grub-core/normal/term.c
index dc03268..0290968 100644
--- a/grub-core/normal/term.c
+++ b/grub-core/normal/term.c
@@ -736,6 +736,8 @@ print_ucs4_terminal (const grub_uint32_t * str,
 	    continue;
 	  putcode_real (*ptr2, term, fixed_tab);
 	}
+      if (contchar)
+	fill_margin (term, margin_right);
     }
   return dry_run ? lines : 0;
 }
@@ -767,13 +769,22 @@ put_glyphs_terminal (const struct grub_unicode_glyph *visual,
 		     grub_ssize_t visual_len,
 		     int margin_left, int margin_right,
 		     struct grub_term_output *term,
-		     struct term_state *state, int fixed_tab)
+		     struct term_state *state, int fixed_tab, grub_uint32_t contchar)
 {
   const struct grub_unicode_glyph *visual_ptr;
+  int wasn = 0;
+
   for (visual_ptr = visual; visual_ptr < visual + visual_len; visual_ptr++)
     {
       if (visual_ptr->base == '\n')
-	grub_print_spaces (term, margin_right);
+	{
+	  wasn = 1;
+	  if (contchar)
+	    fill_margin (term, margin_right);
+	}
+      else
+	wasn = 0;
+ 
       putglyph (visual_ptr, term, fixed_tab);
       if (visual_ptr->base == '\n')
 	{
@@ -786,10 +797,16 @@ put_glyphs_terminal (const struct grub_unicode_glyph *visual,
 	      return 1;
 	    }
 
-	  grub_print_spaces (term, margin_left);
+	  if (!contchar)
+	    grub_print_spaces (term, margin_left);
+	  else
+	    grub_term_gotoxy (term, margin_left,
+			      grub_term_getxy (term) & 0xff);
 	}
       grub_free (visual_ptr->combining);
     }
+    if (!wasn && contchar)
+      fill_margin (term, margin_right);
   return 0;
 }
 
@@ -826,7 +843,7 @@ print_backlog (struct grub_term_output *term,
       ret = put_glyphs_terminal (state->backlog_glyphs,
 				 state->backlog_len,
 				 margin_left, margin_right, term, state,
-				 state->backlog_fixed_tab);
+				 state->backlog_fixed_tab, 0);
       if (!ret)
 	{
 	  grub_free (state->free);
@@ -908,7 +925,7 @@ print_ucs4_real (const grub_uint32_t * str,
 	{
 	  for (vptr = visual_show;
 	       max_lines && vptr < visual + visual_len; vptr++)
-	    if (visual_show->base == '\n')
+	    if (vptr->base == '\n')
 	      max_lines--;
 
 	  visual_len_show = vptr - visual_show;	  
@@ -930,7 +947,7 @@ print_ucs4_real (const grub_uint32_t * str,
 	{
 	  ret = put_glyphs_terminal (visual_show, visual_len_show, margin_left,
 				     contchar ? margin_right : 1,
-				     term, state, fixed_tab);
+				     term, state, fixed_tab, contchar);
 
 	  if (!ret)
 	    grub_free (visual);
-- 
tg: (c235052..) u/fix-after-bidi (depends on: master)

[-- Attachment #2: QEMU_006.png --]
[-- Type: image/png, Size: 24296 bytes --]

Re: [PATCH] fix entry editor screen screen layout corruption in trunk

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 5945 bytes --]

On 17.02.2013 16:15, Andrey Borzenkov wrote:

> See screenshot for what I get using current trunk.
> 
> I do not pretend to fully understand all this code so review is
> appreciated. So far it does fix both gfxterm and console cases for me.

I've fixed the same problem by much smaller logic change

> 
> Patch follows.
> 
> ---
> 
> From: Andrey Borzenkov <arvidjaar@gmail.com>
> Subject: [PATCH] fix entry editor screen corruption
> 
> Fixes the following issues after rev 7137
> 
> 1. right margines were erased on wrapped lines (with contibuation char)
> 
> 2. left margines were erased on continuation lines
> 
> 3. max_lines was effectively ignored by gfxterm, so last line was printed
> outside of editor window, overwriting bottom border and probably beyond
> 
> 4. menu_entry:update_screen() unconditionally padded last line with
> spaces, but print_ucs4_menu() would move to next screen position if
> line was wrapped. This caused bottom border to be partially overwritten
> with spaces. The patch moves line padding to term.c. It also
> makes behaviour consistent with print_ucs4_terminal which did padding
> itself.
> 
> Signed-off-by: Andrey Borzenkov <arvidjaar@gmail.com>
> 
> ---
>  grub-core/normal/menu_entry.c |   10 ----------
>  grub-core/normal/term.c       |   29 +++++++++++++++++++++++------
>  2 files changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/grub-core/normal/menu_entry.c b/grub-core/normal/menu_entry.c
> index 7cd67f3..cc977b5 100644
> --- a/grub-core/normal/menu_entry.c
> +++ b/grub-core/normal/menu_entry.c
> @@ -256,7 +256,6 @@ update_screen (struct screen *screen, struct per_term_screen *term_screen,
>  
>  	  if (i == region_start || linep == screen->lines + screen->line)
>  	    {
> -	      int sp;
>  	      grub_term_gotoxy (term_screen->term, GRUB_TERM_LEFT_BORDER_X
>  				+ GRUB_TERM_MARGIN + 1, (y < 0 ? 0 : y)
>  				+ GRUB_TERM_FIRST_ENTRY_Y);
> @@ -271,14 +270,9 @@ update_screen (struct screen *screen, struct per_term_screen *term_screen,
>  				    term_screen->num_entries
>  				    - ((y > 0) ? y : 0), '\\',
>  				    *pos);
> -	      sp = grub_term_entry_width (term_screen->term)
> -		 - (*pos)[linep->len].x;
> -	      if (sp > 0)
> -		grub_print_spaces (term_screen->term, sp);
>   	    }
>  	  else if (i > region_start && mode == ALL_LINES)
>  	    {
> -	      int sp;
>  	      grub_term_gotoxy (term_screen->term, GRUB_TERM_LEFT_BORDER_X
>  				+ GRUB_TERM_MARGIN + 1, (y < 0 ? 0 : y)
>  				+ GRUB_TERM_FIRST_ENTRY_Y);
> @@ -293,10 +287,6 @@ update_screen (struct screen *screen, struct per_term_screen *term_screen,
>  				    term_screen->num_entries
>  				    - ((y > 0) ? y : 0), '\\',
>  				    *pos);
> -	      sp = grub_term_entry_width (term_screen->term)
> -		- (*pos)[linep->len].x;
> -	      if (sp > 0)
> -		grub_print_spaces (term_screen->term, sp);
>  	    }
>  	  y += get_logical_num_lines (linep, term_screen);
>  	  if (y >= term_screen->num_entries)
> diff --git a/grub-core/normal/term.c b/grub-core/normal/term.c
> index dc03268..0290968 100644
> --- a/grub-core/normal/term.c
> +++ b/grub-core/normal/term.c
> @@ -736,6 +736,8 @@ print_ucs4_terminal (const grub_uint32_t * str,
>  	    continue;
>  	  putcode_real (*ptr2, term, fixed_tab);
>  	}
> +      if (contchar)
> +	fill_margin (term, margin_right);
>      }
>    return dry_run ? lines : 0;
>  }
> @@ -767,13 +769,22 @@ put_glyphs_terminal (const struct grub_unicode_glyph *visual,
>  		     grub_ssize_t visual_len,
>  		     int margin_left, int margin_right,
>  		     struct grub_term_output *term,
> -		     struct term_state *state, int fixed_tab)
> +		     struct term_state *state, int fixed_tab, grub_uint32_t contchar)
>  {
>    const struct grub_unicode_glyph *visual_ptr;
> +  int wasn = 0;
> +
>    for (visual_ptr = visual; visual_ptr < visual + visual_len; visual_ptr++)
>      {
>        if (visual_ptr->base == '\n')
> -	grub_print_spaces (term, margin_right);
> +	{
> +	  wasn = 1;
> +	  if (contchar)
> +	    fill_margin (term, margin_right);
> +	}
> +      else
> +	wasn = 0;
> + 
>        putglyph (visual_ptr, term, fixed_tab);
>        if (visual_ptr->base == '\n')
>  	{
> @@ -786,10 +797,16 @@ put_glyphs_terminal (const struct grub_unicode_glyph *visual,
>  	      return 1;
>  	    }
>  
> -	  grub_print_spaces (term, margin_left);
> +	  if (!contchar)
> +	    grub_print_spaces (term, margin_left);
> +	  else
> +	    grub_term_gotoxy (term, margin_left,
> +			      grub_term_getxy (term) & 0xff);
>  	}
>        grub_free (visual_ptr->combining);
>      }
> +    if (!wasn && contchar)
> +      fill_margin (term, margin_right);
>    return 0;
>  }
>  
> @@ -826,7 +843,7 @@ print_backlog (struct grub_term_output *term,
>        ret = put_glyphs_terminal (state->backlog_glyphs,
>  				 state->backlog_len,
>  				 margin_left, margin_right, term, state,
> -				 state->backlog_fixed_tab);
> +				 state->backlog_fixed_tab, 0);
>        if (!ret)
>  	{
>  	  grub_free (state->free);
> @@ -908,7 +925,7 @@ print_ucs4_real (const grub_uint32_t * str,
>  	{
>  	  for (vptr = visual_show;
>  	       max_lines && vptr < visual + visual_len; vptr++)
> -	    if (visual_show->base == '\n')
> +	    if (vptr->base == '\n')
>  	      max_lines--;
>  
>  	  visual_len_show = vptr - visual_show;	  
> @@ -930,7 +947,7 @@ print_ucs4_real (const grub_uint32_t * str,
>  	{
>  	  ret = put_glyphs_terminal (visual_show, visual_len_show, margin_left,
>  				     contchar ? margin_right : 1,
> -				     term, state, fixed_tab);
> +				     term, state, fixed_tab, contchar);
>  
>  	  if (!ret)
>  	    grub_free (visual);
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [PATCH] fix entry editor screen screen layout corruption in trunk

[Andrey Borzenkov]

[-- Attachment #1.1: Type: text/plain, Size: 549 bytes --]

В Wed, 03 Apr 2013 15:22:31 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> On 17.02.2013 16:15, Andrey Borzenkov wrote:
> 
> > See screenshot for what I get using current trunk.
> > 
> > I do not pretend to fully understand all this code so review is
> > appreciated. So far it does fix both gfxterm and console cases for me.
> 
> I've fixed the same problem by much smaller logic change
> 

Unfortunately now gfxterm case it completely broken. IIRC with my patch
it worked right in both cases ...


[-- Attachment #1.2: QEMU_007.png --]
[-- Type: image/png, Size: 212743 bytes --]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] fix entry editor screen screen layout corruption in trunk

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 656 bytes --]

On 07.04.2013 14:26, Andrey Borzenkov wrote:

> В Wed, 03 Apr 2013 15:22:31 +0200
> Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:
> 
>> On 17.02.2013 16:15, Andrey Borzenkov wrote:
>>
>>> See screenshot for what I get using current trunk.
>>>
>>> I do not pretend to fully understand all this code so review is
>>> appreciated. So far it does fix both gfxterm and console cases for me.
>>
>> I've fixed the same problem by much smaller logic change
>>
> 
> Unfortunately now gfxterm case it completely broken. IIRC with my patch
> it worked right in both cases ...
> 

Thank you for reporting this. Fixed.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [PATCH] fix entry editor screen screen layout corruption in trunk

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]

В Sun, 7 Apr 2013 22:50:29 +0400
Andrey Borzenkov <arvidjaar@gmail.com> пишет:

> В Sun, 07 Apr 2013 17:49:09 +0200
> Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:
> 
> > On 07.04.2013 14:26, Andrey Borzenkov wrote:
> > 
> > > В Wed, 03 Apr 2013 15:22:31 +0200
> > > Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:
> > > 
> > >> On 17.02.2013 16:15, Andrey Borzenkov wrote:
> > >>
> > >>> See screenshot for what I get using current trunk.
> > >>>
> > >>> I do not pretend to fully understand all this code so review is
> > >>> appreciated. So far it does fix both gfxterm and console cases for me.
> > >>
> > >> I've fixed the same problem by much smaller logic change
> > >>
> > > 
> > > Unfortunately now gfxterm case it completely broken. IIRC with my patch
> > > it worked right in both cases ...
> > > 
> > 
> > Thank you for reporting this. Fixed.
> > 
> > 
> 
> Not quite. Better, but ...
> 

Mail was rejected due to size so here link to screenshot:

http://susepaste.org/49485899 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]