* 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.