After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 767575 - Add draw-background signal
Add draw-background signal
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
unspecified
Other Linux
: Normal enhancement
: vte-0-48
Assigned To: VTE Maintainers
VTE Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-06-12 19:16 UTC by Gerald Nunn
Modified: 2018-02-06 00:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Screenshot of POC (170.24 KB, image/png)
2016-06-12 19:16 UTC, Gerald Nunn
  Details
Patch to disable the background draw in VTE (7.01 KB, patch)
2016-10-15 18:01 UTC, Gerald Nunn
none Details | Review

Description Gerald Nunn 2016-06-12 19:16:39 UTC
Created attachment 329647 [details]
Screenshot of POC

I'm looking at adding a similar feature to iterm2's badges (https://www.iterm2.com/documentation-badges.html) into Terminix. Doing so involves being able to draw text on the background of the vte widget but before the text itself is drawn as you would typically want the text to overlay the badge.

After some discussion with Egmont (https://github.com/gnunn1/terminix/issues/299), it was felt that this could be accomplished by adding a new signal, say draw-background, to the VTE that would give the application an opportunity to render a custom background. This would adress two potential use cases as follows:

a. Drawing badges or other indicator style artifacts

b. Drawing background images

I did a POC of this in the current VTE source code by drawing a blue square on the background. To do this I just inserted it after the call to _vte_draw_clear at line 9736 in the VteTerminalPrivate::widget_draw method. 

// GBN - Test painting background to see if it would stick
cairo_rectangle (cr, 50, 50, 100, 100);
cairo_set_source_rgba (cr, 0.0, 0.0, 0.8, 1.0);
cairo_fill(cr);

This POC seems to work well, I tried a few obvious things, like scrolling the text up and down, selecting text, etc and it seems to work reasonably well. When selecting text, the background square I draw gets overwritten by the block highlight, presumably because the _vte_draw_fill_background is called in these cases. As well if you use escape sequences to set text to have background colors these will overwrite the blue block as well. However, I think that this would be the desired behavior in these scenarios.

I would be happy to create a proper patch for this that added a new signal. I was thinking the signal would follow the same syntax as the draw signal and would pass the cairo context to the application in the signal. The signal would be emitted before the _vte_draw_clear and if true is returned, indicating a custom background was drawn, would skip the _vte_draw_clear call. The signal would be emitted again as the _after signal after the call to _vte_draw_clear.

Does this seem like a reasonable approach and something that could be accepted as a patch?
Comment 1 Christian Persch 2016-06-26 10:59:55 UTC
I don't quite get what the point of these watermarks is... but if it's not too intrusive I'd accept something to make this possible.

As for a signal, I recall that emitting signals during draw can be quite bad for performance (e.g. in gtksourceview they were), so maybe something simpler could be needed... like just providing some API to set a watermark cairo_pattern_t and a x,y position where to draw it, and let vte handle this itself?
Comment 2 Gerald Nunn 2016-06-26 16:44:19 UTC
Thanks for the feedback, I'll do some additional research on whether emitting signals from draw is problematic.

With regards to an API, I'm not sure that setting a cairo_pattern_t with coordinates would work very well since wouldn't the application have to constantly update the coordinates and potentially the pattern during resize operations?

Egmont and I discussed creating a badge API (i.e. badgeText, badgeFont, badgeColor, badgePosition, etc) in the terminix bug report but I think he was concerned about API sprawl in terms of all the things that would need to be added to provide maximum flexibility to applications. I can understand where he is coming from and if feasible I think an event would provide maximum flexibility.
Comment 3 Egmont Koblinger 2016-06-27 12:09:27 UTC
I'm not in favor of adding badge text support, for two main reasons:

- Requires tons of arbitrary API methods: specify the text, color, font, font size, alignment, justification, cropping method for overlong text and so on (including whatever future requests such as vertical text or whatnot).

- It has very limited arbitrary usage and is not extendible. E.g. I used to work on a server farm where hosts were named after South Park characters. How cool would it have been to have a picture of them in the corner rather than their name written as text!

I am, however, in favor of a way more generic approach that happens to enable badge text support as well as much more, whatever the container app decides to implement.

This I believe would basically mean resurrecting the background image support (whether it's via an explicit API call to set a bg image, or by hijacking the draw-background signal, this is a technical detail).
Comment 4 Gerald Nunn 2016-10-15 18:01:05 UTC
Created attachment 337764 [details] [review]
Patch to disable the background draw in VTE
Comment 5 Gerald Nunn 2016-10-15 18:10:00 UTC
I've attached a patch for consideration that disables the background draw of the VTE widget by setting a new method vte_terminal_set_disable_bg_draw to true. When this is set to draw, the vte no longer draws it's background and defers it to the application which can draw it by handling the VTE draw signal.

I opted for this over what I planned originally because it eliminates the complexity of the nested signals that Christian was concerned about. 

I have tested this feature in Terminix by adding support for badges and it works well as far as I can tell. This patch can also be used to draw a background image in a much simpler and more performant fashion then what I'm doing now in terminix.
Comment 6 Igor 2016-10-17 16:04:31 UTC
Hi Gerald,
How would it simplify the background image drawing?
I'm also interested as I'm using the same approach in xfce4-terminal as you in terminix.
Comment 7 Gerald Nunn 2016-10-17 16:59:45 UTC
Igor right now to draw the background image you have to the following steps in the draw signal handler:

a. Draw the image on the context passed by the draw signal
b. Draw the VTE onto a separate ImageSurface using propagateDraw
c. Paint the ImageSurface from step B onto the context passed by the draw signal preserving the image previously painted.

If you try to do this in two steps (paint the image onto the context, paint the VTE onto the context) the VTE overwrites the image when it paints it's background. Having the VTE not paint it's background should allow us to simplify this by eliminating the need for step B. I'll admit though I have not explicitly tested this as of yet but is the behavior I see from the badge support I added.

Christian if you want me to enhance the vala terminal test app to include a basic implementation having a background image as part of this patch so you can test it out let me know and I'll be happy to do it.
Comment 8 Christian Persch 2016-10-17 17:10:32 UTC
Can't you do the same already by just setting vte's bg colour's alpha to 0, and draw the real bg colour yourself ?
Comment 9 Gerald Nunn 2016-10-17 17:23:44 UTC
(In reply to Christian Persch from comment #8)
> Can't you do the same already by just setting vte's bg colour's alpha to 0,
> and draw the real bg colour yourself ?

I had this thought too and tried it out two different ways, unfortunately neither worked for me.

The first method is to set the vte alpha to 0 and then draw the background image in the vte draw signal. This doesn't work as per my comment 7 in my reply to Igor.

The other option I tried was to paint the image into the container which vte is parented to. This doesn't work either because GTK appears to be smart enough that it doesn't paint widgets which are obscured in the heirarchy. If you try to paint the image on a lower layer, i.e. the box or whatever container the vte widget is parented to, it never appears. This is actually a good thing otherwise every widget in the view hierarchy would need to be transparent for VTE transparency to work.
Comment 10 Gerald Nunn 2016-11-22 23:28:13 UTC
Just checking in, can this patch be merged or is there additional work that I need to do to make it so? If folks are busy and haven't had time to look at it no worries, but if there are things I can do to help get it merged just let me know.
Comment 11 Christian Persch 2016-12-24 20:39:06 UTC
I think the only thing that prevents the method in comment 8 from working is that _vte_draw_clear() uses operator SOURCE instead of OVER. It doesn't seem that changing that to OVER introduces any problems, so maybe that's the simplest solution here.
Comment 12 Gerald Nunn 2017-01-07 15:28:47 UTC
Thank you for the suggestion and my apologies for the delay in replying. I just tested changing _vte_draw_clear from SOURCE to OVER and it works fine for painting the badge. However, it should be noted that this change does break transparency in the sense that the terminal is no longer transparent to the background but is rather transparent to the container it is parented it to.

What I mean, is that when you change it from SOURCE to OVER and the background color has alpha, you no longer see the desktop under the terminal, instead the the underlying container is what comes through. This can be corrected by painting the background yourself in on_draw using the SOURCE operator but results in a double draw (VTE painting a color with 0 alpha and then me painting it again). 

It may also be possible to fix it by making the containers themselves transparent but I didn't test that, if you need me too let me know. Either way though it would be a breaking change for terminals depending on this for transparency.
Comment 13 Christian Persch 2017-01-28 19:14:35 UTC
Back when gnome-terminal supported transparency, it was either transparent or had a background image, not both...

I think it should work for you to draw the whole area with SOURCE and alpha=0, then your bg image / badge, and then chain up with vte using OVER, and get the expected result of transparency and bg image/badge, right?

If so, I think the API should be a single entry point
- vte_terminal_set_background_operator (VteTerminal *terminal, cairo_operator_t op) taking one of these two operators, or even simpler an 
- vte_terminal_set_erase_background(VteTerminal *, gboolean erase) to switch between OVER and SOURCE,
- but there should be no gobject properties or anything else, since this really is a special and rarely-to-be-used case.

I really don't want to export a signal or callback, since it would expose the internals too much esp. when we don't know yet in which direction we'll go wrt gtk4 etc.
Comment 14 Gerald Nunn 2017-01-28 19:33:46 UTC
Thanks Christian, I'll try that and submit a patch when it looks good.
Comment 15 Christian Persch 2017-10-24 18:34:38 UTC
I added vte_terminal_set_background_operator() and test for it in vte-2.91.

Please try if this solves your problem; re-open if not.
Comment 16 Egmont Koblinger 2017-10-24 20:37:11 UTC
Just wondering:

 * If the operator is changed while the widget is already shown,
 * you need to queue a redraw to get it applied.

Shouldn't it be done in vte itself? It's just as simple as an invalidate_all() as we do for other similar stuff (e.g. palette change, allow-bold change).

I don't know how to trigger a full redraw externally, and whether it's possible at all (I guess it's possible, still, I think it's cleaner if we do it in vte).
Comment 17 Christian Persch 2017-10-24 20:43:54 UTC
Hmm right writing that sentence was as much (or more) effort than just invalidating directly :-)
Comment 18 Gerald Nunn 2017-10-26 11:57:29 UTC
Thank you so much for doing this, my apologies for not getting it done myself. I'll implement support for it in Tilix over the weekend.
Comment 19 Gerald Nunn 2017-10-28 01:47:26 UTC
I have updated Tilix to use this feature to draw the badge and it works great, so thanks very much for implementing this. 

One small difference between this and my original patch was that the original patch used _vte_draw_clear to clear the background whereas now the background color is being drawn. It was easy to fix though by setting the VTE background color to a fully transparent color.
Comment 20 Christian Persch 2018-02-05 19:11:52 UTC
Just in time for the API freeze, I've changed my mind here; the operator is too generic. I've reverted that approach and committed a simpler one which is equivalent to your patch here (only the setting is negated). Sorry if that causes an inconvenience.
Comment 21 Gerald Nunn 2018-02-06 00:08:49 UTC
No worries as it's a minor change to accommodate on my end, thanks again for adding this feature to VTE.