* About x_draw_xwidget_glyph_string
@ 2016-01-25 2:07 YAMAMOTO Mitsuharu
2016-01-25 6:59 ` joakim
2016-01-25 15:46 ` Eli Zaretskii
0 siblings, 2 replies; 11+ messages in thread
From: YAMAMOTO Mitsuharu @ 2016-01-25 2:07 UTC (permalink / raw)
To: emacs-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: About x_draw_xwidget_glyph_string
2016-01-25 2:07 About x_draw_xwidget_glyph_string YAMAMOTO Mitsuharu
@ 2016-01-25 6:59 ` joakim
2016-01-25 15:46 ` Eli Zaretskii
1 sibling, 0 replies; 11+ messages in thread
From: joakim @ 2016-01-25 6:59 UTC (permalink / raw)
To: YAMAMOTO Mitsuharu; +Cc: emacs-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: About x_draw_xwidget_glyph_string
2016-01-25 2:07 About x_draw_xwidget_glyph_string YAMAMOTO Mitsuharu
2016-01-25 6:59 ` joakim
@ 2016-01-25 15:46 ` Eli Zaretskii
2016-04-08 15:35 ` joakim
1 sibling, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-01-25 15:46 UTC (permalink / raw)
To: YAMAMOTO Mitsuharu; +Cc: emacs-devel
> 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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: About x_draw_xwidget_glyph_string
2016-01-25 15:46 ` Eli Zaretskii
@ 2016-04-08 15:35 ` joakim
2016-04-09 7:38 ` Eli Zaretskii
0 siblings, 1 reply; 11+ messages in thread
From: joakim @ 2016-04-08 15:35 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: YAMAMOTO Mitsuharu, emacs-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: About x_draw_xwidget_glyph_string
2016-04-08 15:35 ` joakim
@ 2016-04-09 7:38 ` Eli Zaretskii
2016-04-09 11:22 ` joakim
0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2016-04-09 7:38 UTC (permalink / raw)
To: joakim; +Cc: mituharu, emacs-devel
> 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?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: About x_draw_xwidget_glyph_string
2016-04-09 7:38 ` Eli Zaretskii
@ 2016-04-09 11:22 ` joakim
2016-04-09 12:06 ` Eli Zaretskii
2016-04-10 8:29 ` YAMAMOTO Mitsuharu
0 siblings, 2 replies; 11+ messages in thread
From: joakim @ 2016-04-09 11:22 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: mituharu, emacs-devel
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: About x_draw_xwidget_glyph_string
2016-04-09 11:22 ` joakim
@ 2016-04-09 12:06 ` Eli Zaretskii
2016-04-10 8:29 ` YAMAMOTO Mitsuharu
1 sibling, 0 replies; 11+ messages in thread
From: Eli Zaretskii @ 2016-04-09 12:06 UTC (permalink / raw)
To: joakim; +Cc: mituharu, emacs-devel
> 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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: About x_draw_xwidget_glyph_string
2016-04-09 11:22 ` joakim
2016-04-09 12:06 ` Eli Zaretskii
@ 2016-04-10 8:29 ` YAMAMOTO Mitsuharu
2016-04-11 0:04 ` YAMAMOTO Mitsuharu
1 sibling, 1 reply; 11+ messages in thread
From: YAMAMOTO Mitsuharu @ 2016-04-10 8:29 UTC (permalink / raw)
To: joakim; +Cc: Eli Zaretskii, emacs-devel
>>>>> 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);
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: About x_draw_xwidget_glyph_string
2016-04-10 8:29 ` YAMAMOTO Mitsuharu
@ 2016-04-11 0:04 ` YAMAMOTO Mitsuharu
2016-04-12 20:46 ` joakim
2016-04-12 21:13 ` joakim
0 siblings, 2 replies; 11+ messages in thread
From: YAMAMOTO Mitsuharu @ 2016-04-11 0:04 UTC (permalink / raw)
To: joakim; +Cc: Eli Zaretskii, emacs-devel
>>>>> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: About x_draw_xwidget_glyph_string
2016-04-11 0:04 ` YAMAMOTO Mitsuharu
@ 2016-04-12 20:46 ` joakim
2016-04-12 21:13 ` joakim
1 sibling, 0 replies; 11+ messages in thread
From: joakim @ 2016-04-12 20:46 UTC (permalink / raw)
To: YAMAMOTO Mitsuharu; +Cc: Eli Zaretskii, emacs-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: About x_draw_xwidget_glyph_string
2016-04-11 0:04 ` YAMAMOTO Mitsuharu
2016-04-12 20:46 ` joakim
@ 2016-04-12 21:13 ` joakim
1 sibling, 0 replies; 11+ messages in thread
From: joakim @ 2016-04-12 21:13 UTC (permalink / raw)
To: YAMAMOTO Mitsuharu; +Cc: Eli Zaretskii, emacs-devel
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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-04-12 21:13 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-25 2:07 About x_draw_xwidget_glyph_string YAMAMOTO Mitsuharu
2016-01-25 6:59 ` joakim
2016-01-25 15:46 ` Eli Zaretskii
2016-04-08 15:35 ` joakim
2016-04-09 7:38 ` Eli Zaretskii
2016-04-09 11:22 ` joakim
2016-04-09 12:06 ` Eli Zaretskii
2016-04-10 8:29 ` YAMAMOTO Mitsuharu
2016-04-11 0:04 ` YAMAMOTO Mitsuharu
2016-04-12 20:46 ` joakim
2016-04-12 21:13 ` joakim
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.