About x_draw_xwidget_glyph_string

About x_draw_xwidget_glyph_string

[YAMAMOTO Mitsuharu]

I have a question and a comment about x_draw_xwidget_glyph_string in
src/xwidget.c.

1. Question about xwidget initialization.

   559	x_draw_xwidget_glyph_string (struct glyph_string *s)
   560	{
   561	  /* This method is called by the redisplay engine and places the
   562	     xwidget on screen.  Moving and clipping is done here.  Also view
   563	     initialization.  */
   564	  struct xwidget *xww = s->xwidget;
   565	  struct xwidget_view *xv = xwidget_view_lookup (xww, s->w);
(snip)
   574	  /* Do initialization here in the display loop because there is no
   575	     other time to know things like window placement etc.  */
   576	  xv = xwidget_init_view (xww, s, x, y);

Line 576 indicates a new xwidget view is created in every
x_draw_xwidget_glyph_string call.  But this makes xwidget-view-list
longer and longer, and looks like a waste of memory, if not a leak.
Also, if it is an intended behavior, then it looks strange to look up
a view at Line 565.

2. Comment on clipping.

   578	  /* Calculate clipping, which is used for all manner of onscreen
   579	     xwidget views.  Each widget border can get clipped by other emacs
   580	     objects so there are four clipping variables.  */
   581	  clip_right =
   582	    min (xww->width,
   583	         WINDOW_RIGHT_EDGE_X (s->w) - x -
   584	         WINDOW_RIGHT_SCROLL_BAR_AREA_WIDTH (s->w) -
   585	         WINDOW_RIGHT_FRINGE_WIDTH (s->w));
   586	  clip_left =
   587	    max (0,
   588	         WINDOW_LEFT_EDGE_X (s->w) - x +
   589	         WINDOW_LEFT_SCROLL_BAR_AREA_WIDTH (s->w) +
   590	         WINDOW_LEFT_FRINGE_WIDTH (s->w));
   591	
   592	  clip_bottom =
   593	    min (xww->height,
   594	         WINDOW_BOTTOM_EDGE_Y (s->w) - WINDOW_MODE_LINE_HEIGHT (s->w) - y);
   595	  clip_top = max (0, WINDOW_TOP_EDGE_Y (s->w) - y);

I think the calculation of clipping should use the function window_box
rather than manual calculation with various window macros.  Otherwise,
xwidget views will cover horizontal scroll bars, for example.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: About x_draw_xwidget_glyph_string

[joakim]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

> I have a question and a comment about x_draw_xwidget_glyph_string in
> src/xwidget.c.
>
> 1. Question about xwidget initialization.
>
>    559	x_draw_xwidget_glyph_string (struct glyph_string *s)
>    560	{
>    561	  /* This method is called by the redisplay engine and places the
>    562	     xwidget on screen.  Moving and clipping is done here.  Also view
>    563	     initialization.  */
>    564	  struct xwidget *xww = s->xwidget;
>    565	  struct xwidget_view *xv = xwidget_view_lookup (xww, s->w);
> (snip)
>    574	  /* Do initialization here in the display loop because there is no
>    575	     other time to know things like window placement etc.  */
>    576	  xv = xwidget_init_view (xww, s, x, y);
>
> Line 576 indicates a new xwidget view is created in every
> x_draw_xwidget_glyph_string call.  But this makes xwidget-view-list
> longer and longer, and looks like a waste of memory, if not a leak.
> Also, if it is an intended behavior, then it looks strange to look up
> a view at Line 565.
>
> 2. Comment on clipping.
>
>    578	  /* Calculate clipping, which is used for all manner of onscreen
>    579	     xwidget views.  Each widget border can get clipped by other emacs
>    580	     objects so there are four clipping variables.  */
>    581	  clip_right =
>    582	    min (xww->width,
>    583	         WINDOW_RIGHT_EDGE_X (s->w) - x -
>    584	         WINDOW_RIGHT_SCROLL_BAR_AREA_WIDTH (s->w) -
>    585	         WINDOW_RIGHT_FRINGE_WIDTH (s->w));
>    586	  clip_left =
>    587	    max (0,
>    588	         WINDOW_LEFT_EDGE_X (s->w) - x +
>    589	         WINDOW_LEFT_SCROLL_BAR_AREA_WIDTH (s->w) +
>    590	         WINDOW_LEFT_FRINGE_WIDTH (s->w));
>    591	
>    592	  clip_bottom =
>    593	    min (xww->height,
>    594	         WINDOW_BOTTOM_EDGE_Y (s->w) - WINDOW_MODE_LINE_HEIGHT (s->w) - y);
>    595	  clip_top = max (0, WINDOW_TOP_EDGE_Y (s->w) - y);
>
> I think the calculation of clipping should use the function window_box
> rather than manual calculation with various window macros.  Otherwise,
> xwidget views will cover horizontal scroll bars, for example.
>
> 				     YAMAMOTO Mitsuharu
> 				mituharu@math.s.chiba-u.ac.jp
>

I won't have time to have a look at this until a couple of days, but
your suggestions sound promising. If you have a patch I can test I would
be grateful.

-- 
Joakim Verona

Re: About x_draw_xwidget_glyph_string

[Eli Zaretskii]

> Date: Mon, 25 Jan 2016 11:07:16 +0900
> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> 
> 2. Comment on clipping.
> 
>    578	  /* Calculate clipping, which is used for all manner of onscreen
>    579	     xwidget views.  Each widget border can get clipped by other emacs
>    580	     objects so there are four clipping variables.  */
>    581	  clip_right =
>    582	    min (xww->width,
>    583	         WINDOW_RIGHT_EDGE_X (s->w) - x -
>    584	         WINDOW_RIGHT_SCROLL_BAR_AREA_WIDTH (s->w) -
>    585	         WINDOW_RIGHT_FRINGE_WIDTH (s->w));
>    586	  clip_left =
>    587	    max (0,
>    588	         WINDOW_LEFT_EDGE_X (s->w) - x +
>    589	         WINDOW_LEFT_SCROLL_BAR_AREA_WIDTH (s->w) +
>    590	         WINDOW_LEFT_FRINGE_WIDTH (s->w));
>    591	
>    592	  clip_bottom =
>    593	    min (xww->height,
>    594	         WINDOW_BOTTOM_EDGE_Y (s->w) - WINDOW_MODE_LINE_HEIGHT (s->w) - y);
>    595	  clip_top = max (0, WINDOW_TOP_EDGE_Y (s->w) - y);
> 
> I think the calculation of clipping should use the function window_box
> rather than manual calculation with various window macros.  Otherwise,
> xwidget views will cover horizontal scroll bars, for example.

I agree.

Thanks.

Re: About x_draw_xwidget_glyph_string

[joakim]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Mon, 25 Jan 2016 11:07:16 +0900
>> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
>> 
>> 2. Comment on clipping.
>> 
>>    578	  /* Calculate clipping, which is used for all manner of onscreen
>>    579	     xwidget views.  Each widget border can get clipped by other emacs
>>    580	     objects so there are four clipping variables.  */
>>    581	  clip_right =
>>    582	    min (xww->width,
>>    583	         WINDOW_RIGHT_EDGE_X (s->w) - x -
>>    584	         WINDOW_RIGHT_SCROLL_BAR_AREA_WIDTH (s->w) -
>>    585	         WINDOW_RIGHT_FRINGE_WIDTH (s->w));
>>    586	  clip_left =
>>    587	    max (0,
>>    588	         WINDOW_LEFT_EDGE_X (s->w) - x +
>>    589	         WINDOW_LEFT_SCROLL_BAR_AREA_WIDTH (s->w) +
>>    590	         WINDOW_LEFT_FRINGE_WIDTH (s->w));
>>    591	
>>    592	  clip_bottom =
>>    593	    min (xww->height,
>>    594	         WINDOW_BOTTOM_EDGE_Y (s->w) - WINDOW_MODE_LINE_HEIGHT (s->w) - y);
>>    595	  clip_top = max (0, WINDOW_TOP_EDGE_Y (s->w) - y);
>> 
>> I think the calculation of clipping should use the function window_box
>> rather than manual calculation with various window macros.  Otherwise,
>> xwidget views will cover horizontal scroll bars, for example.
>
> I agree.
>
> Thanks.
>

I tried to do this, but I'm doing something wrong. How is window_box
meant to be used?

This is my attempt to replace the code above:

  //JAVE work in progressing, suggested by YAMAMOTO Mitsuharu
  int text_area_x, text_area_y, text_area_width, text_area_height;
  
  window_box (s->w,
              ANY_AREA, //also tried TEXT_AREA
              &text_area_x,
              &text_area_y,
              &text_area_width,
              &text_area_height);
  clip_right =
    min (xww->width,
         text_area_width);
  clip_left =
    max (0,
         text_area_x);

  clip_bottom =
    min (xww->height,
         text_area_y);
  clip_top = max (0, text_area_height);



-- 
Joakim Verona

Re: About x_draw_xwidget_glyph_string

[Eli Zaretskii]

> From: joakim@verona.se
> Cc: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>,  emacs-devel@gnu.org
> Date: Fri, 08 Apr 2016 17:35:23 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Date: Mon, 25 Jan 2016 11:07:16 +0900
> >> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> >> 
> >> 2. Comment on clipping.
> >> 
> >>    578	  /* Calculate clipping, which is used for all manner of onscreen
> >>    579	     xwidget views.  Each widget border can get clipped by other emacs
> >>    580	     objects so there are four clipping variables.  */
> >>    581	  clip_right =
> >>    582	    min (xww->width,
> >>    583	         WINDOW_RIGHT_EDGE_X (s->w) - x -
> >>    584	         WINDOW_RIGHT_SCROLL_BAR_AREA_WIDTH (s->w) -
> >>    585	         WINDOW_RIGHT_FRINGE_WIDTH (s->w));
> >>    586	  clip_left =
> >>    587	    max (0,
> >>    588	         WINDOW_LEFT_EDGE_X (s->w) - x +
> >>    589	         WINDOW_LEFT_SCROLL_BAR_AREA_WIDTH (s->w) +
> >>    590	         WINDOW_LEFT_FRINGE_WIDTH (s->w));
> >>    591	
> >>    592	  clip_bottom =
> >>    593	    min (xww->height,
> >>    594	         WINDOW_BOTTOM_EDGE_Y (s->w) - WINDOW_MODE_LINE_HEIGHT (s->w) - y);
> >>    595	  clip_top = max (0, WINDOW_TOP_EDGE_Y (s->w) - y);
> >> 
> >> I think the calculation of clipping should use the function window_box
> >> rather than manual calculation with various window macros.  Otherwise,
> >> xwidget views will cover horizontal scroll bars, for example.
> >
> > I agree.
> >
> > Thanks.
> >
> 
> I tried to do this, but I'm doing something wrong. How is window_box
> meant to be used?
> 
> This is my attempt to replace the code above:
> 
>   //JAVE work in progressing, suggested by YAMAMOTO Mitsuharu
>   int text_area_x, text_area_y, text_area_width, text_area_height;
>   
>   window_box (s->w,
>               ANY_AREA, //also tried TEXT_AREA

You should use TEXT_AREA here.

>               &text_area_x,
>               &text_area_y,
>               &text_area_width,
>               &text_area_height);
>   clip_right =
>     min (xww->width,
>          text_area_width);
>   clip_left =
>     max (0,
>          text_area_x);
> 
>   clip_bottom =
>     min (xww->height,
>          text_area_y);
>   clip_top = max (0, text_area_height);

I think clip_top should use text_area_y and clip_bottom should use
text_area_height.

Other than those two issues, what other problems do you see?

Re: About x_draw_xwidget_glyph_string

[joakim]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: joakim@verona.se
>> Cc: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>,  emacs-devel@gnu.org
>> Date: Fri, 08 Apr 2016 17:35:23 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> Date: Mon, 25 Jan 2016 11:07:16 +0900
>> >> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
>> >> 
>> >> 2. Comment on clipping.
>> >> 
>> >>    578	  /* Calculate clipping, which is used for all manner of onscreen
>> >>    579	     xwidget views.  Each widget border can get clipped by other emacs
>> >>    580	     objects so there are four clipping variables.  */
>> >>    581	  clip_right =
>> >>    582	    min (xww->width,
>> >>    583	         WINDOW_RIGHT_EDGE_X (s->w) - x -
>> >>    584	         WINDOW_RIGHT_SCROLL_BAR_AREA_WIDTH (s->w) -
>> >>    585	         WINDOW_RIGHT_FRINGE_WIDTH (s->w));
>> >>    586	  clip_left =
>> >>    587	    max (0,
>> >>    588	         WINDOW_LEFT_EDGE_X (s->w) - x +
>> >>    589	         WINDOW_LEFT_SCROLL_BAR_AREA_WIDTH (s->w) +
>> >>    590	         WINDOW_LEFT_FRINGE_WIDTH (s->w));
>> >>    591	
>> >>    592	  clip_bottom =
>> >>    593	    min (xww->height,
>> >>    594	         WINDOW_BOTTOM_EDGE_Y (s->w) - WINDOW_MODE_LINE_HEIGHT (s->w) - y);
>> >>    595	  clip_top = max (0, WINDOW_TOP_EDGE_Y (s->w) - y);
>> >> 
>> >> I think the calculation of clipping should use the function window_box
>> >> rather than manual calculation with various window macros.  Otherwise,
>> >> xwidget views will cover horizontal scroll bars, for example.
>> >
>> > I agree.
>> >
>> > Thanks.
>> >
>> 
>> I tried to do this, but I'm doing something wrong. How is window_box
>> meant to be used?
>> 
>> This is my attempt to replace the code above:
>> 
>>   //JAVE work in progressing, suggested by YAMAMOTO Mitsuharu
>>   int text_area_x, text_area_y, text_area_width, text_area_height;
>>   
>>   window_box (s->w,
>>               ANY_AREA, //also tried TEXT_AREA
>
> You should use TEXT_AREA here.
>
>>               &text_area_x,
>>               &text_area_y,
>>               &text_area_width,
>>               &text_area_height);
>>   clip_right =
>>     min (xww->width,
>>          text_area_width);
>>   clip_left =
>>     max (0,
>>          text_area_x);
>> 
>>   clip_bottom =
>>     min (xww->height,
>>          text_area_y);
>>   clip_top = max (0, text_area_height);
>
> I think clip_top should use text_area_y and clip_bottom should use
> text_area_height.
>
> Other than those two issues, what other problems do you see?

Well, I saw nothing at all :)

This patch seems to work during some brief testing. 

diff --git a/src/xwidget.c b/src/xwidget.c
index 8ff4c23..fa61f57 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -578,25 +578,24 @@ x_draw_xwidget_glyph_string (struct glyph_string *s)
      other time to know things like window placement etc.  */
   xv = xwidget_init_view (xww, s, x, y);
 
-  /* Calculate clipping, which is used for all manner of onscreen
-     xwidget views.  Each widget border can get clipped by other emacs
-     objects so there are four clipping variables.  */
-  clip_right =
-    min (xww->width,
-         WINDOW_RIGHT_EDGE_X (s->w) - x -
-         WINDOW_RIGHT_SCROLL_BAR_AREA_WIDTH (s->w) -
-         WINDOW_RIGHT_FRINGE_WIDTH (s->w));
-  clip_left =
-    max (0,
-         WINDOW_LEFT_EDGE_X (s->w) - x +
-         WINDOW_LEFT_SCROLL_BAR_AREA_WIDTH (s->w) +
-         WINDOW_LEFT_FRINGE_WIDTH (s->w));
-
-  clip_bottom =
-    min (xww->height,
-         WINDOW_BOTTOM_EDGE_Y (s->w) - WINDOW_MODE_LINE_HEIGHT (s->w) - y);
-  clip_top = max (0, WINDOW_TOP_EDGE_Y (s->w) - y);
-
+  int text_area_x, text_area_y, text_area_width, text_area_height;
+  
+  window_box (s->w,
+              ANY_AREA,
+              &text_area_x,
+              &text_area_y,
+              &text_area_width,
+              &text_area_height);
+  clip_right = min (xww->width,
+                    text_area_width);
+  clip_left = max (0,
+                   text_area_x);
+
+  clip_bottom = min (xww->height,
+                     text_area_height);
+  clip_top = max (0, text_area_y);
+
+  
   /* We are concerned with movement of the onscreen area.  The area
      might sit still when the widget actually moves.  This happens
      when an Emacs window border moves across a widget window.  So, if


-- 
Joakim Verona

Re: About x_draw_xwidget_glyph_string

[Eli Zaretskii]

> From: joakim@verona.se
> Cc: mituharu@math.s.chiba-u.ac.jp,  emacs-devel@gnu.org
> Date: Sat, 09 Apr 2016 13:22:52 +0200
> 
> >> This is my attempt to replace the code above:
> >> 
> >>   //JAVE work in progressing, suggested by YAMAMOTO Mitsuharu
> >>   int text_area_x, text_area_y, text_area_width, text_area_height;
> >>   
> >>   window_box (s->w,
> >>               ANY_AREA, //also tried TEXT_AREA
> >
> > You should use TEXT_AREA here.
> >
> >>               &text_area_x,
> >>               &text_area_y,
> >>               &text_area_width,
> >>               &text_area_height);
> >>   clip_right =
> >>     min (xww->width,
> >>          text_area_width);
> >>   clip_left =
> >>     max (0,
> >>          text_area_x);
> >> 
> >>   clip_bottom =
> >>     min (xww->height,
> >>          text_area_y);
> >>   clip_top = max (0, text_area_height);
> >
> > I think clip_top should use text_area_y and clip_bottom should use
> > text_area_height.
> >
> > Other than those two issues, what other problems do you see?
> 
> Well, I saw nothing at all :)

Thanks, pushed to the emacs-25 branch.

Re: About x_draw_xwidget_glyph_string

[YAMAMOTO Mitsuharu]

>>>>> On Sat, 09 Apr 2016 13:22:52 +0200, joakim@verona.se said:

>> I think clip_top should use text_area_y and clip_bottom should use
>> text_area_height.
>> 
>> Other than those two issues, what other problems do you see?

> Well, I saw nothing at all :)

> This patch seems to work during some brief testing.

C-x 2 or C-x 3 makes the display incorrect.

I'm not sure if I understand the intended meaning of clip_* variables
correctly, but shouldn't this part be something like below?

			     YAMAMOTO Mitsuharu
			mituharu@math.s.chiba-u.ac.jp

diff --git a/src/xwidget.c b/src/xwidget.c
index 7e96307..0777777 100644
--- a/src/xwidget.c
+++ b/src/xwidget.c
@@ -580,20 +580,14 @@ x_draw_xwidget_glyph_string (struct glyph_string *s)
 
   int text_area_x, text_area_y, text_area_width, text_area_height;
 
-  window_box (s->w,
-              ANY_AREA,
-              &text_area_x,
-              &text_area_y,
-              &text_area_width,
-              &text_area_height);
-  clip_right = min (xww->width,
-                    text_area_width);
-  clip_left = max (0,
-                   text_area_x);
-
-  clip_bottom = min (xww->height,
-                     text_area_height);
-  clip_top = max (0, text_area_y);
+  window_box (s->w, TEXT_AREA, &text_area_x, &text_area_y,
+              &text_area_width, &text_area_height);
+  clip_left = max (0, text_area_x - x);
+  clip_right = max (clip_left,
+		    min (xww->width, text_area_x + text_area_width - x));
+  clip_top = max (0, text_area_y - y);
+  clip_bottom = max (clip_top,
+		     min (xww->height, text_area_y + text_area_height - y));
 
   /* We are concerned with movement of the onscreen area.  The area
      might sit still when the widget actually moves.  This happens
@@ -622,8 +616,7 @@ x_draw_xwidget_glyph_string (struct glyph_string *s)
       || xv->clip_bottom != clip_bottom
       || xv->clip_top != clip_top || xv->clip_left != clip_left)
     {
-      gtk_widget_set_size_request (xv->widgetwindow, clip_right + clip_left,
-                                   clip_bottom + clip_top);
+      gtk_widget_set_size_request (xv->widgetwindow, clip_right, clip_bottom);
       gtk_fixed_move (GTK_FIXED (xv->widgetwindow), xv->widget, -clip_left,
                       -clip_top);
 

Re: About x_draw_xwidget_glyph_string

[YAMAMOTO Mitsuharu]

>>>>> On Sun, 10 Apr 2016 17:29:26 +0900, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:

>>> I think clip_top should use text_area_y and clip_bottom should use
>>> text_area_height.
>>> 
>>> Other than those two issues, what other problems do you see?

>> Well, I saw nothing at all :)

>> This patch seems to work during some brief testing.

> C-x 2 or C-x 3 makes the display incorrect.

> I'm not sure if I understand the intended meaning of clip_* variables
> correctly, but shouldn't this part be something like below?

> 			     YAMAMOTO Mitsuharu
> 			mituharu@math.s.chiba-u.ac.jp

> diff --git a/src/xwidget.c b/src/xwidget.c
> index 7e96307..0777777 100644
> --- a/src/xwidget.c
> +++ b/src/xwidget.c
> @@ -580,20 +580,14 @@ x_draw_xwidget_glyph_string (struct glyph_string *s)
 
>    int text_area_x, text_area_y, text_area_width, text_area_height;
 
> -  window_box (s->w,
> -              ANY_AREA,
> -              &text_area_x,
> -              &text_area_y,
> -              &text_area_width,
> -              &text_area_height);
> -  clip_right = min (xww->width,
> -                    text_area_width);
> -  clip_left = max (0,
> -                   text_area_x);
> -
> -  clip_bottom = min (xww->height,
> -                     text_area_height);
> -  clip_top = max (0, text_area_y);
> +  window_box (s->w, TEXT_AREA, &text_area_x, &text_area_y,
> +              &text_area_width, &text_area_height);
> +  clip_left = max (0, text_area_x - x);
> +  clip_right = max (clip_left,
> +		    min (xww->width, text_area_x + text_area_width - x));
> +  clip_top = max (0, text_area_y - y);
> +  clip_bottom = max (clip_top,
> +		     min (xww->height, text_area_y + text_area_height - y));
 
>    /* We are concerned with movement of the onscreen area.  The area
>       might sit still when the widget actually moves.  This happens
> @@ -622,8 +616,7 @@ x_draw_xwidget_glyph_string (struct glyph_string *s)
>        || xv->clip_bottom != clip_bottom
>        || xv->clip_top != clip_top || xv->clip_left != clip_left)
>      {
> -      gtk_widget_set_size_request (xv->widgetwindow, clip_right + clip_left,
> -                                   clip_bottom + clip_top);
> +      gtk_widget_set_size_request (xv->widgetwindow, clip_right, clip_bottom);
>        gtk_fixed_move (GTK_FIXED (xv->widgetwindow), xv->widget, -clip_left,
>                        -clip_top);
 
Oops, I meant "gtk_widget_set_size_request (xv->widgetwindow,
clip_right - clip_left, clip_bottom - clip_top);" for the last hunk.
Could you double check if this matches your intended meaning of clip_*
variables?  Adding some comments to the corresponding members in
src/xwidget.h would be nice.

Also, could you also take a look at the other issue of
"xwidget-view-list gets longer and longer" I mentioned in the original
message?

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Re: About x_draw_xwidget_glyph_string

[joakim]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

>>>>>> On Sun, 10 Apr 2016 17:29:26 +0900, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:
>
>>>> I think clip_top should use text_area_y and clip_bottom should use
>>>> text_area_height.
>>>> 
>>>> Other than those two issues, what other problems do you see?
>
>>> Well, I saw nothing at all :)
>
>>> This patch seems to work during some brief testing.
>
>> C-x 2 or C-x 3 makes the display incorrect.
>
>> I'm not sure if I understand the intended meaning of clip_* variables
>> correctly, but shouldn't this part be something like below?
>
>> 			     YAMAMOTO Mitsuharu
>> 			mituharu@math.s.chiba-u.ac.jp
>
>> diff --git a/src/xwidget.c b/src/xwidget.c
>> index 7e96307..0777777 100644
>> --- a/src/xwidget.c
>> +++ b/src/xwidget.c
>> @@ -580,20 +580,14 @@ x_draw_xwidget_glyph_string (struct glyph_string *s)
>  
>>    int text_area_x, text_area_y, text_area_width, text_area_height;
>  
>> -  window_box (s->w,
>> -              ANY_AREA,
>> -              &text_area_x,
>> -              &text_area_y,
>> -              &text_area_width,
>> -              &text_area_height);
>> -  clip_right = min (xww->width,
>> -                    text_area_width);
>> -  clip_left = max (0,
>> -                   text_area_x);
>> -
>> -  clip_bottom = min (xww->height,
>> -                     text_area_height);
>> -  clip_top = max (0, text_area_y);
>> +  window_box (s->w, TEXT_AREA, &text_area_x, &text_area_y,
>> +              &text_area_width, &text_area_height);
>> +  clip_left = max (0, text_area_x - x);
>> +  clip_right = max (clip_left,
>> +		    min (xww->width, text_area_x + text_area_width - x));
>> +  clip_top = max (0, text_area_y - y);
>> +  clip_bottom = max (clip_top,
>> +		     min (xww->height, text_area_y + text_area_height - y));
>  
>>    /* We are concerned with movement of the onscreen area.  The area
>>       might sit still when the widget actually moves.  This happens
>> @@ -622,8 +616,7 @@ x_draw_xwidget_glyph_string (struct glyph_string *s)
>>        || xv->clip_bottom != clip_bottom
>>        || xv->clip_top != clip_top || xv->clip_left != clip_left)
>>      {
>> -      gtk_widget_set_size_request (xv->widgetwindow, clip_right + clip_left,
>> -                                   clip_bottom + clip_top);
>> +      gtk_widget_set_size_request (xv->widgetwindow, clip_right, clip_bottom);
>>        gtk_fixed_move (GTK_FIXED (xv->widgetwindow), xv->widget, -clip_left,
>>                        -clip_top);
>  
> Oops, I meant "gtk_widget_set_size_request (xv->widgetwindow,
> clip_right - clip_left, clip_bottom - clip_top);" for the last hunk.
> Could you double check if this matches your intended meaning of clip_*
> variables?  Adding some comments to the corresponding members in
> src/xwidget.h would be nice.

At the moment I'm a bit confused myself. But to recap, the basic idea is, as
you already noticed, to find out if the widget needs to be
clipped against the edge of an emacs object.

The clip values are compared to the ones from the last run in order to
minimize flicker.

> Also, could you also take a look at the other issue of
> "xwidget-view-list gets longer and longer" I mentioned in the original
> message?

Yes I will attempt to do so.

>
> 				     YAMAMOTO Mitsuharu
> 				mituharu@math.s.chiba-u.ac.jp

-- 
Joakim Verona

Re: About x_draw_xwidget_glyph_string

[joakim]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:


> Also, could you also take a look at the other issue of
> "xwidget-view-list gets longer and longer" I mentioned in the original
> message?

I think the original intention was to only run initialization if the
lookup first returned NULL, like below. (Which of course was what you
wrote in your original email, albeit more eloquently)

  /* Do initialization here in the display loop because there is no
     other time to know things like window placement etc.  */
  if(xv == NULL)
    xv = xwidget_init_view (xww, s, x, y);


>
> 				     YAMAMOTO Mitsuharu
> 				mituharu@math.s.chiba-u.ac.jp

-- 
Joakim Verona