! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
background image not working
Status:
RESOLVED: FIXED
Product:
Xfce4-terminal
Component:
General

Comments

Description Ian 2016-09-14 01:19:54 CEST
Selecting either a background image or transparent background has no effect - I just get a solid color instead.

Fedora 24, Gnome desktop (both Wayland and non-wayland)
Comment 1 Igor editbugs 2016-09-14 11:17:31 CEST
Background image setting not working is a known issue.

Transparent background, however, should be (as is) working if window manager supports compositing feature - it's working for me in Cinnamon and Xfce environments.
Do Fedora desktops support compositing?
Comment 2 Igor editbugs 2016-09-14 11:18:15 CEST
(In reply to Igor from comment #1)
> Transparent background, however, should be (as is) working if window manager

"as is" should read "and is".
Comment 3 Egmont Koblinger 2016-09-14 14:07:26 CEST
Just FYI: See the Terminix project how a background image can be implemented. VTE no longer supports this directly, but you can have a transparent vte, and hijack the draw signal's handler to paint there something before vte gets to paint its own stuff.
Comment 4 Igor editbugs 2016-09-14 15:18:58 CEST
(In reply to Egmont Koblinger from comment #3)
> Just FYI: See the Terminix project how a background image can be
> implemented. VTE no longer supports this directly, but you can have a
> transparent vte, and hijack the draw signal's handler to paint there
> something before vte gets to paint its own stuff.

Yeah, I've seen their code, thanks.
Just need some time to get into it.
Comment 5 Ian 2016-09-14 22:52:46 CEST
> Do Fedora desktops support compositing?

Yes, AFAIK Gnome3's Mutter is a compositing window manager.
Comment 6 Igor editbugs 2016-09-15 13:25:15 CEST
Okay, for the transparency it seems that gtk_widget_set_app_paintable() should be set to TRUE for the window widget (https://bugzilla.gnome.org/show_bug.cgi?id=729884).
But this somehow makes menubar become transparent, and I didn't find a way to set it opaque yet.
Comment 7 Igor editbugs 2016-09-15 13:58:44 CEST
(In reply to Igor from comment #6)
> Okay, for the transparency it seems that gtk_widget_set_app_paintable()
> should be set to TRUE for the window widget
> (https://bugzilla.gnome.org/show_bug.cgi?id=729884).
> But this somehow makes menubar become transparent, and I didn't find a way
> to set it opaque yet.

This commit made it work for me: https://git.xfce.org/apps/xfce4-terminal/commit/?id=e1d6fd3d497508e293240ce2d9835db42aa7cc5f
Could you please check transparency with it?
Comment 8 Igor editbugs 2016-09-15 16:22:51 CEST
(In reply to Egmont Koblinger from comment #3)
> Just FYI: See the Terminix project how a background image can be
> implemented. VTE no longer supports this directly, but you can have a
> transparent vte, and hijack the draw signal's handler to paint there
> something before vte gets to paint its own stuff.

Egmont, I've managed to implement a similar solution but drawing vte widget to cairo surface seems to be taking a considerable CPU resource - 7-8 per cent on my machine. Do you have any idea why this is happening?
Comment 9 Egmont Koblinger 2016-09-15 19:03:24 CEST
How are you measuring this exactly? :)

I usually do a "time cat bigfile" to see cat's wall clock time. This on its own shows a much bigger than 7-8% variance by itself, don't quite know why. Also this measures cat's wall clock rather than vte's cpu time, and in case of a computation- and disk-heavy operation with relatively few screen updates.

Something like time'ing vte (or xfce4-terminal) whereas you do let's say scroll in mcview is, on the other hand, a much more proper measure of vte itself, and within that much rather the screen update than the internal logic.

Terminix used to have a terribly poor performance with background image. That was because on each screen repaint it did rescale the image. I hope (I assume) you're not doing this. You should rescale-crop the image once per resize to match the widget's size, and cache it and just copy on each paint.

You might want to repeat your measurement on Terminix with and without background image.

If that one also suffers from the same degradation... well then I think it's fair to say that 7-8% performance regression is a fair price to pay for this feature.
Comment 10 Egmont Koblinger 2016-09-15 19:05:51 CEST
Not sure what I had in mind when I typed "in case of".

[...] Also this measures cat's wall clock rather than vte's cpu time, and _is_ a computation- and disk-heavy operation with relatively few screen updates.

Sorry for bad English ;)
Comment 11 Igor editbugs 2016-09-15 22:20:29 CEST
(In reply to Egmont Koblinger)
> Not in Ambiance/Radiance. In Adwaita it's okay.

AUR comments are saying these themes are broken on gtk3.20.
Could you be seeing one of the broken things?
Comment 12 Egmont Koblinger 2016-09-15 22:33:12 CEST
I have gtk-3.18. That said, the themes could still be broken, I don't know.

Without understanding the details of what's going on, all I know is that both gnome-terminal's unofficial Fedora+Ubuntu transparency patch as well as Terminix suffered quite a lot from these kinds of issues. I'm not sure how they managed to fix them. See Terminix issues 146 & 303 for further details / vague pointers. I kinda have the feeling (I'm not sure) that gnome-terminal applied some magic hammer that once and for all solves all these kinds of issues, whereas Terminix went for individual CSS overrides especially for the Ambi/Radi themes.

https://github.com/gnunn1/terminix/issues/146
https://github.com/gnunn1/terminix/issues/303
Comment 13 Gerald Nunn 2016-09-15 23:06:26 CEST
I'm the author of Terminix, just to chime him in with some details/thoughts that might be helpful as follows (or not!) in addition to what Egmont has already mentioned.

In terms of performance, while caching the image certainly helped performance another big factor was selecting the right filter when scaling the image in cairo. I ended up going with the bilinear as it seemed to be on the best weighing performance against quality.

For the transparency issue, as Egmont pointed out I played a bit of whack a mole getting it sorted out but in the end it was relatively straight forward. Just to be clear though, there is no theme specific CSS that fixes this in terminix, it was addressed generically but doing so did reveal variations in the way themes reacted to transparency requiring that I try different solutions to find a generic solution. 

To fix transparency issues, I just added a CSS class to my notebook pages that provided a background, see terminix-background here (https://github.com/gnunn1/terminix/blob/master/data/resources/css/terminix.base.css#L24). I'm no GTK guru, but it appears to me like GTK ignores the widget hierarchy when painting. So having a parent with a CSS class that adds a background does not cause the child widgets to get that background when using a transparent color, the widget remains transparent.

This is also why you can't just paint the background on the window and have it show through to the transparent widget.

The theme specific files I have are to address theme specific issues (looking at you Ubuntu themes!) or to support transparent scrollbars which give a look similar to an overlay scrollbar. In particular, in Ubuntu the default scrollbar look is hideous and making the scrollbar transparent in terminix along with CSS tweaks really improved the look. See this issue for more information:

https://github.com/gnunn1/terminix/issues/239

Any questions feel free to ask or ping me offline, I'm always happy to collaborate.
Comment 14 Ian 2016-09-15 23:46:36 CEST
(In reply to Igor from comment #7)
> This commit made it work for me:
> https://git.xfce.org/apps/xfce4-terminal/commit/
> ?id=e1d6fd3d497508e293240ce2d9835db42aa7cc5f
> Could you please check transparency with it?

I've tested this commit - transparency works, background image does not.
Comment 15 Igor editbugs 2016-09-16 11:35:02 CEST
(In reply to Egmont Koblinger from comment #9)
> How are you measuring this exactly? :)
> 
> I usually do a "time cat bigfile" to see cat's wall clock time. This on its
> own shows a much bigger than 7-8% variance by itself, don't quite know why.
> Also this measures cat's wall clock rather than vte's cpu time, and in case
> of a computation- and disk-heavy operation with relatively few screen
> updates.
> 
> Something like time'ing vte (or xfce4-terminal) whereas you do let's say
> scroll in mcview is, on the other hand, a much more proper measure of vte
> itself, and within that much rather the screen update than the internal
> logic.
> 
> Terminix used to have a terribly poor performance with background image.
> That was because on each screen repaint it did rescale the image. I hope (I
> assume) you're not doing this. You should rescale-crop the image once per
> resize to match the widget's size, and cache it and just copy on each paint.
> 
> You might want to repeat your measurement on Terminix with and without
> background image.
> 
> If that one also suffers from the same degradation... well then I think it's
> fair to say that 7-8% performance regression is a fair price to pay for this
> feature.

I was just running htop in another terminal. When there was no background image set, the terminal was consuming 0% CPU in idle state; but when an image got set, it started consuming 7-8% while still doing nothing, just drawing.

There is a caching mechanism already implemented in the code but, apparently, it has stopped working. It seems to be related to GObject refs; when I removed unref calls the CPU consumption fell down to something less that a per cent (but still more than 0); this also introduced memory leaks, naturally. 

I need to fix the caching and then see if any further improvements can be made.

I also wanted to measure terminix CPU consumption but for some reason it wouldn't set background image for me. Since Gerald is here, maybe he could help me.
Comment 16 Igor editbugs 2016-09-16 11:35:32 CEST
(In reply to Egmont Koblinger from comment #10)
> Not sure what I had in mind when I typed "in case of".
> 
> [...] Also this measures cat's wall clock rather than vte's cpu time, and
> _is_ a computation- and disk-heavy operation with relatively few screen
> updates.
> 
> Sorry for bad English ;)

Don't be sorry - your English is better than mine :)
Comment 17 Igor editbugs 2016-09-16 11:36:43 CEST
(In reply to Egmont Koblinger from comment #12)
> I have gtk-3.18. That said, the themes could still be broken, I don't know.
> 
> Without understanding the details of what's going on, all I know is that
> both gnome-terminal's unofficial Fedora+Ubuntu transparency patch as well as
> Terminix suffered quite a lot from these kinds of issues. I'm not sure how
> they managed to fix them. See Terminix issues 146 & 303 for further details
> / vague pointers. I kinda have the feeling (I'm not sure) that
> gnome-terminal applied some magic hammer that once and for all solves all
> these kinds of issues, whereas Terminix went for individual CSS overrides
> especially for the Ambi/Radi themes.
> 
> https://github.com/gnunn1/terminix/issues/146
> https://github.com/gnunn1/terminix/issues/303

Thanks! Would you mind giving me a link to that gnome-terminal patch?
Comment 18 Igor editbugs 2016-09-16 11:37:21 CEST
(In reply to Ian from comment #14)
> (In reply to Igor from comment #7)
> > This commit made it work for me:
> > https://git.xfce.org/apps/xfce4-terminal/commit/
> > ?id=e1d6fd3d497508e293240ce2d9835db42aa7cc5f
> > Could you please check transparency with it?
> 
> I've tested this commit - transparency works, background image does not.

Thanks for the update. The background image support is still in work.
Comment 19 Igor editbugs 2016-09-16 11:49:08 CEST
(In reply to Gerald Nunn from comment #13)
> I'm the author of Terminix, just to chime him in with some details/thoughts
> that might be helpful as follows (or not!) in addition to what Egmont has
> already mentioned.
> 
> In terms of performance, while caching the image certainly helped
> performance another big factor was selecting the right filter when scaling
> the image in cairo. I ended up going with the bilinear as it seemed to be on
> the best weighing performance against quality.
> 
> For the transparency issue, as Egmont pointed out I played a bit of whack a
> mole getting it sorted out but in the end it was relatively straight
> forward. Just to be clear though, there is no theme specific CSS that fixes
> this in terminix, it was addressed generically but doing so did reveal
> variations in the way themes reacted to transparency requiring that I try
> different solutions to find a generic solution. 
> 
> To fix transparency issues, I just added a CSS class to my notebook pages
> that provided a background, see terminix-background here
> (https://github.com/gnunn1/terminix/blob/master/data/resources/css/terminix.
> base.css#L24). I'm no GTK guru, but it appears to me like GTK ignores the
> widget hierarchy when painting. So having a parent with a CSS class that
> adds a background does not cause the child widgets to get that background
> when using a transparent color, the widget remains transparent.
> 
> This is also why you can't just paint the background on the window and have
> it show through to the transparent widget.
> 
> The theme specific files I have are to address theme specific issues
> (looking at you Ubuntu themes!) or to support transparent scrollbars which
> give a look similar to an overlay scrollbar. In particular, in Ubuntu the
> default scrollbar look is hideous and making the scrollbar transparent in
> terminix along with CSS tweaks really improved the look. See this issue for
> more information:
> 
> https://github.com/gnunn1/terminix/issues/239
> 
> Any questions feel free to ask or ping me offline, I'm always happy to
> collaborate.

Thank you for your support! I really appreciate you joining the discussion.

As for the image scaling, the code already existed in the terminal - it's using gdk_pixbuf_composite() and GDK_INTERP_BILINEAR filter, and I guess should be quite optimized in this area.
But, even if using image caching, the CPU consumption is still more than 0%. Is this also the case for terminix (I couldn't measure it - see my reply to Egmont)? The gtk2 version of xfce4-terminal is still using 0% CPU when drawing a background image, according to htop.

As for the transparency, this is a bit unclear to me as I'm no CSS expert either. I wouldn't like messing with CSS styles and would prefer using gnome-terminal's silver hammer if one exists :)
However, thanks for sharing your experience - I may have to come back to you with it.
Comment 20 Egmont Koblinger 2016-09-16 11:53:35 CEST
(In reply to Igor from comment #15)

> I was just running htop in another terminal. When there was no background
> image set, the terminal was consuming 0% CPU in idle state; but when an
> image got set, it started consuming 7-8% while still doing nothing, just
> drawing.

Oh, I misunderstood you. So it's not 7-8% degradation but way worse than that.

> I need to fix the caching and then see if any further improvements can be
> made.

I'm pretty sure that will solve it. The image should not be resized-scaled on each paint (up to 40-ish FPS), it should be resized-scaled-probably-cropped-too only once per window resize (since it's an expensive operation), and then just copied from the cached version on each draw event so that vte can draw on that.
Comment 21 Egmont Koblinger 2016-09-16 11:59:08 CEST
(In reply to Igor from comment #17)

> Thanks! Would you mind giving me a link to that gnome-terminal patch?

See https://github.com/gnunn1/terminix/issues/146#issuecomment-196995094.

> As for the transparency, this is a bit unclear to me as I'm no CSS expert
> either. I wouldn't like messing with CSS styles and would prefer using
> gnome-terminal's silver hammer if one exists :)

As Gerald points out somewhere, this silver hammer might break the scrollbar's transparency... at this point I'm lost :D
Comment 22 Igor editbugs 2016-09-16 12:08:06 CEST
(In reply to Egmont Koblinger from comment #21)
> (In reply to Igor from comment #17)
> 
> > Thanks! Would you mind giving me a link to that gnome-terminal patch?
> 
> See https://github.com/gnunn1/terminix/issues/146#issuecomment-196995094.
> 
> > As for the transparency, this is a bit unclear to me as I'm no CSS expert
> > either. I wouldn't like messing with CSS styles and would prefer using
> > gnome-terminal's silver hammer if one exists :)
> 
> As Gerald points out somewhere, this silver hammer might break the
> scrollbar's transparency... at this point I'm lost :D

xfce4-terminal is not using overlay scrollbars... so I'm hoing that gtk scrollbar will survive it :)
Comment 23 Egmont Koblinger 2016-09-16 12:08:29 CEST
(In reply to Igor from comment #19)

> As for the image scaling, the code already existed in the terminal - it's
> using gdk_pixbuf_composite() and GDK_INTERP_BILINEAR filter, and I guess
> should be quite optimized in this area.

With image caching this really shouldn't make a big difference, as it's executed only once per window resize, not on each paint.

> But, even if using image caching, the CPU consumption is still more than 0%.
> Is this also the case for terminix (I couldn't measure it - see my reply to
> Egmont)? The gtk2 version of xfce4-terminal is still using 0% CPU when
> drawing a background image, according to htop.

On my machine, Terminix 80x24 running htop:
- with transparency (but no bg image) Terminix's CPU% (as reported by this very htop) keeps jumping between 0.0 and 0.7;
- with transparency + bg image it keeps jumping between 0.7 and 1.3.

Obviously CPU utilization should be bigger than 0.0 since it's Terminix that handles htop's output. It should be precisely 0.0 only when the terminal doesn't process any input, doesn't blink the cursor, doesn't get expose events etc.

I think this performance is absolutely okay.
Comment 24 Egmont Koblinger 2016-09-16 12:15:18 CEST
(In reply to Igor from comment #22)
> xfce4-terminal is not using overlay scrollbars... so I'm hoing that gtk
> scrollbar will survive it :)

Here's a gnome-terminal link to show that we have problems, too :) I don't understand the technical details too much. Also we've now diverted quite far from this bug's topic, sorry :)

https://bugzilla.gnome.org/show_bug.cgi?id=754796
Comment 25 Igor editbugs 2016-09-16 12:38:41 CEST
(In reply to Igor from comment #11)
> (In reply to Egmont Koblinger)
> > Not in Ambiance/Radiance. In Adwaita it's okay.
> 
> AUR comments are saying these themes are broken on gtk3.20.
> Could you be seeing one of the broken things?

Could you please try this patch? https://git.xfce.org/apps/xfce4-terminal/commit/?id=2a4707f1267c0b5ebbe6e9a9ad193eeb82498fd5
It should resolve notebook transparency issue under Unity.
Comment 26 Egmont Koblinger 2016-09-16 12:47:31 CEST
Yup it does, thanks!
Comment 27 Gerald Nunn 2016-09-16 15:15:05 CEST
(In reply to Igor from comment #15)
> I also wanted to measure terminix CPU consumption but for some reason it
> wouldn't set background image for me. Since Gerald is here, maybe he could
> help me.

You also need to make the terminal itself transparent, in the Preferences dialog go to the Profiles page and edit the current profile. In the color tab, use the slider to make the terminal transparent and you should see the image appear (hopefully!).
Comment 28 Igor editbugs 2016-09-16 15:23:31 CEST
(In reply to Gerald Nunn from comment #27)
> You also need to make the terminal itself transparent, in the Preferences
> dialog go to the Profiles page and edit the current profile. In the color
> tab, use the slider to make the terminal transparent and you should see the
> image appear (hopefully!).
Ah, thanks. Yes, it's showing the image now.
htop running in another terminal shows 0% of CPU usage when terminix is idle, so I guess this is the goal that should be achieved by xfce4-terminal.
Comment 29 Igor editbugs 2016-09-16 17:34:57 CEST
Gerald, it seems to me there are serious memory leaks in terminix when using background image.

I'm struggling with them myself: for now, I can either create a quick solution with memory leaks, or a heavier one (consuming CPU) but with no leaks.
The latter one is available if someone wants to test it: https://github.com/f2404/xfce4-terminal3/commit/9ccde7c3dc8e85d5a0ed6a2b5a4d5ebd14f6dc45
Comment 30 Gerald Nunn 2016-09-16 17:59:04 CEST
(In reply to Igor from comment #29)
> Gerald, it seems to me there are serious memory leaks in terminix when using
> background image.
> 
> I'm struggling with them myself: for now, I can either create a quick
> solution with memory leaks, or a heavier one (consuming CPU) but with no
> leaks.
> The latter one is available if someone wants to test it:
> https://github.com/f2404/xfce4-terminal3/commit/
> 9ccde7c3dc8e85d5a0ed6a2b5a4d5ebd14f6dc45

I'm not seeing that myself but it's certainly possible I'm missing something. Keep in mind that D is a GC language so memory will go up and may take awhile to come back down again. When I'm testing for this, I simply gran a corner of the terminal and then re-size it constantly which causes new ImageSurface objects to be constaintly generated for the new sizes. Doing this I can see the memory starts going up from 27 MB to eventually 46 MB at which point it steady states, it occasionally jumps up more but then falls back down again.

Can you provide more info about what you are doing to test it and I'll try to repeat it. I'll freely admit I don't think this feature is used very much so I'm not sure how well tested it is in the field though I did spend quite a bit of time making sure there were no leaks when I first developed it.
Comment 31 Egmont Koblinger 2016-09-16 18:01:04 CEST
(In reply to Igor from comment #28)

> htop running in another terminal shows 0% of CPU usage when terminix is
> idle, so I guess this is the goal that should be achieved by xfce4-terminal.

If the terminal is inactive, I'm pretty sure its CPU usage is 0%. You'd have to work real hard to intentionally break it in xfce4-terminal (e.g. continuously call a vte method), wouldn't you?

The goal should be not to significantly increase CPU usage when the terminal actually does some painting.

(In reply to Igor from comment #29)

> Gerald, it seems to me there are serious memory leaks in terminix when using
> background image.

I can't see any. Terminix git master as of a few days ago, with bg image, running htop for a few minutes: VIRT, RES & SHR remained exactly the same.
Comment 32 Igor editbugs 2016-09-16 18:14:30 CEST
(In reply to Gerald Nunn from comment #30)
> I'm not seeing that myself but it's certainly possible I'm missing
> something. Keep in mind that D is a GC language so memory will go up and may
> take awhile to come back down again. When I'm testing for this, I simply
> gran a corner of the terminal and then re-size it constantly which causes
> new ImageSurface objects to be constaintly generated for the new sizes.
> Doing this I can see the memory starts going up from 27 MB to eventually 46
> MB at which point it steady states, it occasionally jumps up more but then
> falls back down again.
> 
> Can you provide more info about what you are doing to test it and I'll try
> to repeat it. I'll freely admit I don't think this feature is used very much
> so I'm not sure how well tested it is in the field though I did spend quite
> a bit of time making sure there were no leaks when I first developed it.

Well, I just ran `valgrind --tool=memcheck --leak-check=full --show-leak-kinds=all --leak-resolution=high --track-origins=yes terminix` and closed terminix window after it appeared.
The output was
==8023== LEAK SUMMARY:
==8023==    definitely lost: 39,325 bytes in 94 blocks
==8023==    indirectly lost: 146,443 bytes in 2,844 blocks
==8023==      possibly lost: 39,135,238 bytes in 617 blocks
==8023==    still reachable: 6,285,445 bytes in 36,613 blocks

Please see here: https://gist.github.com/f2404/0fbd4a7bf2959de1a71aa1d8666a1d33

When run with no background image set, the reported leaks reduce to the following:
==8066== LEAK SUMMARY:
==8066==    definitely lost: 38,468 bytes in 90 blocks
==8066==    indirectly lost: 145,843 bytes in 2,840 blocks
==8066==      possibly lost: 41,850 bytes in 612 blocks
==8066==    still reachable: 5,887,511 bytes in 34,493 blocks
Comment 33 Igor editbugs 2016-09-16 18:18:17 CEST
(In reply to Egmont Koblinger from comment #31)
> If the terminal is inactive, I'm pretty sure its CPU usage is 0%. You'd have
> to work real hard to intentionally break it in xfce4-terminal (e.g.
> continuously call a vte method), wouldn't you?
> 
> The goal should be not to significantly increase CPU usage when the terminal
> actually does some painting.

Background is painted in the draw signal handler of the terminal widget, and it gets called pretty often. Now it's like it was before when we called something like vte_set_backgroung_image only once.
Comment 34 Gerald Nunn 2016-09-16 18:22:25 CEST
(In reply to Igor from comment #32)
> Well, I just ran `valgrind --tool=memcheck --leak-check=full
> --show-leak-kinds=all --leak-resolution=high --track-origins=yes terminix`
> and closed terminix window after it appeared.
> The output was

Unfortunately valgrind isn't reliable with the D language for memory leaks.
Comment 35 Egmont Koblinger 2016-09-17 00:05:07 CEST
(In reply to Igor from comment #33)
> Background is painted in the draw signal handler of the terminal widget, and
> it gets called pretty often. Now it's like it was before when we called
> something like vte_set_backgroung_image only once.

Well, it shouldn't get called if there's no activity whatsoever.
Comment 36 Igor editbugs 2016-09-20 14:07:45 CEST
Please check https://git.xfce.org/apps/xfce4-terminal/commit/?id=fb0409e56c9cf305e7ad89f8f76a5a90e6017799 for the background image support.
Image cashing it there so it shouldn't use CPU much.
Comment 37 Egmont Koblinger 2016-09-20 23:55:58 CEST
- The Good:

"Normal" usage of the terminal, e.g. cat'ing a giant file works great. It's fast and it doesn't leak.

- The Bad:

Resizing the terminal seems to leak. More precisely, the numbers in VIRT and RES columns of "top" keep increasing when you resize xfce4-terminal, and they never shrink back.

Interestingly, Terminix suffers from the same problem. I wonder if it's a leak in some underlying library perhaps...??

The leak only occurs if there's a background image. So the culprit is not vte.

- The Ugly:

The two black (or white) stripes at the two sizes with "Scaled" mode. In my opinion, it's better to crop the image so that the entire terminal background is filled with the picture (and its aspect ratio is kept).

Also, the slidebar's label "Image darkening" is misleading in case of white background: then it's actually "Image lightening" or something. Not sure what could be a better label. Maybe "Image intensity" (with reversed slider)??
Comment 38 Igor editbugs 2016-09-21 12:04:07 CEST
(In reply to Egmont Koblinger from comment #37)
> - The Good:
> 
> "Normal" usage of the terminal, e.g. cat'ing a giant file works great. It's
> fast and it doesn't leak.
First, thanks for testing!

> - The Bad:
> 
> Resizing the terminal seems to leak. More precisely, the numbers in VIRT and
> RES columns of "top" keep increasing when you resize xfce4-terminal, and
> they never shrink back.
> 
> Interestingly, Terminix suffers from the same problem. I wonder if it's a
> leak in some underlying library perhaps...??
> 
> The leak only occurs if there's a background image. So the culprit is not
> vte.
As for xfce4-terminal, I think the memory consumption increase is expected as it's storing cached images in a list. New images are added on each size change, and should be used when returning to one of previous sizes.
The cache is freed on closing the terminal; I've just verified again that each image created gets freed on exit. Should be no memory leaks here.

> - The Ugly:
> 
> The two black (or white) stripes at the two sizes with "Scaled" mode. In my
> opinion, it's better to crop the image so that the entire terminal
> background is filled with the picture (and its aspect ratio is kept).
This is xfce4-terminal-0.6.3 behavior and I don't want to change it. In fact, I like the way scaling has been implemented as I'm sure that I'm seeing the entire image even though it's not taking entire terminal window.
Maybe another option could be added - something like "fit window width/height" - what do you think?

> Also, the slidebar's label "Image darkening" is misleading in case of white
> background: then it's actually "Image lightening" or something. Not sure
> what could be a better label. Maybe "Image intensity" (with reversed
> slider)??
Yeah, this setting should be revised, I guess. I changed the label just yesterday as its previous name "opacity" was even more confusing for the background image case.
It probably requires a different name, and a separate setting perhaps (now it's still the same "opacity" setting) - I think opacity should remain for the transparent background option, and a new setting should appear for the image intensity or whatever.
Comment 39 poma 2016-09-21 17:51:35 CEST
s/darkening/shading/
Ref.
https://en.wikipedia.org/wiki/Shading

Besides it works with:
$ xfce4-terminal --version | head -1
xfce4-terminal 0.7.0git-20160921git432ed2a (Xfce 4.12)
Comment 40 poma 2016-09-21 17:58:28 CEST
(In reply to poma from comment #39)
> s/darkening/shading/
> Ref.
> https://en.wikipedia.org/wiki/Shading
> 
> Besides it works with:
> $ xfce4-terminal --version | head -1
> xfce4-terminal 0.7.0git-20160921git432ed2a (Xfce 4.12)

Of course, if you want to be more precise, you can call it "plain shading" rather than "area shading", however "shading" is sufficient.
Comment 41 Egmont Koblinger 2016-09-21 22:35:01 CEST
(In reply to Igor from comment #38)

> First, thanks for testing!

Thanks to you for working on this! :)

> As for xfce4-terminal, I think the memory consumption increase is expected
> as it's storing cached images in a list. New images are added on each size
> change, and should be used when returning to one of previous sizes.

I don't see the point in this design. Resizing-cropping is a cheap operation relatively to the cost of a user-initiated resize. Storing previous versions (especially without a limit) keeps adding up to the overall memory usage.

I don't know how xfce environment resizes windows by default, but most WMs do opaque resize, that is, the window gets many resizes during a single user-initiated mouse-dragging resize. There's absolutely no point in storing the intermediate scaled images, it's quite unlikely that they will be used again, those exact terminal sizes probably won't be seen again.

IMO maybe it makes sense to cache 1 or maybe at most 2 previous versions (but not more than that), this saves CPU at maximize/restore or fullscreen/restore events.

Sanity check (I haven't checked the code): if you really cache some previous ones, make sure they're indexed by the terminal's pixel size rather than the character size, so that it doesn't break when you change the font size, or the window is forced to a non-grid-aligned size.

> The cache is freed on closing the terminal; I've just verified again that
> each image created gets freed on exit. Should be no memory leaks here.

What do you mean by closing the "terminal" - by "terminal" do you refer to a vte widget, or the entire app?

Closing a window that was resized many times (whereas another window of the app remains open) does not shrink the memory usage for me.

If you explicitly call free() when exiting the entire app, then who cares? It's given back to the OS anyway. I mean sure it avoids a valgrind warning, but it's still a memleak. A memleak is not defined by valgrind or similar tools giving a warning, or something not getting explicitly free()'d on exit. A memleak is a constantly growing memory usage of the app over time, which does happen now.

> Maybe another option could be added - something like "fit window
> width/height" - what do you think?

Up to you, sounds good to me.

> Yeah, this setting should be revised, I guess. I changed the label just
> yesterday as its previous name "opacity" was even more confusing for the
> background image case.

I think poma's recommendation is great.
Comment 42 Igor editbugs 2016-09-22 06:25:26 CEST
(In reply to poma from comment #40)
> (In reply to poma from comment #39)
> > s/darkening/shading/
> > Ref.
> > https://en.wikipedia.org/wiki/Shading
> > 
> > Besides it works with:
> > $ xfce4-terminal --version | head -1
> > xfce4-terminal 0.7.0git-20160921git432ed2a (Xfce 4.12)
> 
> Of course, if you want to be more precise, you can call it "plain shading"
> rather than "area shading", however "shading" is sufficient.

Thanks for the suggestion!
Comment 43 Igor editbugs 2016-09-22 06:45:14 CEST
(In reply to Egmont Koblinger from comment #41)
> > As for xfce4-terminal, I think the memory consumption increase is expected
> > as it's storing cached images in a list. New images are added on each size
> > change, and should be used when returning to one of previous sizes.
> 
> I don't see the point in this design. Resizing-cropping is a cheap operation
> relatively to the cost of a user-initiated resize. Storing previous versions
> (especially without a limit) keeps adding up to the overall memory usage.
> 
> I don't know how xfce environment resizes windows by default, but most WMs
> do opaque resize, that is, the window gets many resizes during a single
> user-initiated mouse-dragging resize. There's absolutely no point in storing
> the intermediate scaled images, it's quite unlikely that they will be used
> again, those exact terminal sizes probably won't be seen again.
I've been also thinking about this. Now, a resize can generate hundreds of images stored in the cache, and you are right, most of them will never be loaded again.

> IMO maybe it makes sense to cache 1 or maybe at most 2 previous versions
> (but not more than that), this saves CPU at maximize/restore or
> fullscreen/restore events.
Do you know if it's possible to check if a resize is still in progress from a draw signal handler?
I mean, I would maybe cache image before a resize (that is, the original one) and another (final) one after the resize has been finished. All intermediate images would be dropped.

> Sanity check (I haven't checked the code): if you really cache some previous
> ones, make sure they're indexed by the terminal's pixel size rather than the
> character size, so that it doesn't break when you change the font size, or
> the window is forced to a non-grid-aligned size.
Yes, what gets checked is size in pixels, not in characters.

> > The cache is freed on closing the terminal; I've just verified again that
> > each image created gets freed on exit. Should be no memory leaks here.
> 
> What do you mean by closing the "terminal" - by "terminal" do you refer to a
> vte widget, or the entire app?
> 
> Closing a window that was resized many times (whereas another window of the
> app remains open) does not shrink the memory usage for me.
I mean the entire app as the cache is common for all terminal windows.

> If you explicitly call free() when exiting the entire app, then who cares?
> It's given back to the OS anyway. I mean sure it avoids a valgrind warning,
> but it's still a memleak. A memleak is not defined by valgrind or similar
> tools giving a warning, or something not getting explicitly free()'d on
> exit. A memleak is a constantly growing memory usage of the app over time,
> which does happen now.
I agree, memory consumption increase rate the terminal is showing now is not reasonable. I will reduce it.

> > Maybe another option could be added - something like "fit window
> > width/height" - what do you think?
> 
> Up to you, sounds good to me.
Okay.

> > Yeah, this setting should be revised, I guess. I changed the label just
> > yesterday as its previous name "opacity" was even more confusing for the
> > background image case.
> 
> I think poma's recommendation is great.
All right. I will add a new setting called "shading" for background image mode.
Comment 44 Egmont Koblinger 2016-09-22 09:15:56 CEST
(In reply to Igor from comment #43)

> Do you know if it's possible to check if a resize is still in progress from
> a draw signal handler?

I don't think it's possible to check it at all. AFAIK, the concept of a window "being resized" (e.g. the window manager's handlebar being dragged) doesn't exist in X11 or Waylayd for apps.

Think about it: You drag the window, resize it, then keep it dragged but no longer resize. You redraw vte, but can't yet know if it's going to remain the final state. Then at one point you release the handle. So even this information was accessible, it wouldn't yet be available from the draw signal.

What you may have access to (I don't know if you do) is if subsequent resize events are already queued up while the current one is being handled.

Irrelevant here, but I know that intermediate resize events are dropped while new ones arrive and the old ones still weren't handled yet. Example: Let's say the user increases the width 80->81->82->83->84->85 with 0.1 seconds between each step, and let's say it takes 1 second for vte to update (maybe scrollback is very large and takes such a long time to rewrap). Vte will resize like 80->81->85, because it immediately starts to handle the first resize event it receives (to 81), but by the time it finishes handling this the resize events all through 85 have arrived, and there's no point handling the intermediate ones.

> I mean, I would maybe cache image before a resize (that is, the original
> one) and another (final) one after the resize has been finished. All
> intermediate images would be dropped.

How about remembering the timestaps and using heuristics on the lifetime of a given size? E.g. drop if it was used for less than a second?

I'd still combine it with a reasonable upper limit on the size of the cache (maybe at most as many not-currently-visible sizes as the number of windows? ... or something similar).
Comment 45 Ian 2016-09-24 09:37:45 CEST
(In reply to Igor from comment #36)
> Please check
> https://git.xfce.org/apps/xfce4-terminal/commit/
> ?id=fb0409e56c9cf305e7ad89f8f76a5a90e6017799 for the background image
> support.
> Image cashing it there so it shouldn't use CPU much.

Works for me! Thanks :)
Comment 46 Igor editbugs 2016-09-26 13:10:03 CEST
(In reply to Ian from comment #45)
> (In reply to Igor from comment #36)
> > Please check
> > https://git.xfce.org/apps/xfce4-terminal/commit/
> > ?id=fb0409e56c9cf305e7ad89f8f76a5a90e6017799 for the background image
> > support.
> > Image cashing it there so it shouldn't use CPU much.
> 
> Works for me! Thanks :)
You are welcome :)
Comment 47 Igor editbugs 2016-09-26 13:14:19 CEST
I've added BackgroundImageShading setting: https://git.xfce.org/apps/xfce4-terminal/commit/?id=f992fa02f289d61cce25ddc88d5fcb9428030857
Comment 48 poma 2016-09-26 22:43:05 CEST
Created attachment 6866 
xfce4-terminal

Here's some nice screenie
Comment 49 Ian 2016-09-26 23:21:24 CEST
Created attachment 6867 
missing slider
Comment 50 Ian 2016-09-26 23:22:44 CEST
(In reply to Igor from comment #47)
> I've added BackgroundImageShading setting:
> https://git.xfce.org/apps/xfce4-terminal/commit/
> ?id=f992fa02f289d61cce25ddc88d5fcb9428030857

Unfortunately, for me the opacity/shading slider setting has disappeared now - see screenshot 'missing slider'.
Comment 51 Igor editbugs 2016-09-27 15:49:25 CEST
(In reply to Ian from comment #50)
> (In reply to Igor from comment #47)
> > I've added BackgroundImageShading setting:
> > https://git.xfce.org/apps/xfce4-terminal/commit/
> > ?id=f992fa02f289d61cce25ddc88d5fcb9428030857
> 
> Unfortunately, for me the opacity/shading slider setting has disappeared now
> - see screenshot 'missing slider'.

It seems you need to properly install xfce4-terminal; or just manually copy terminal-preferences.ui file that has been generated during the build from ./terminal/ to /usr/share/xfce4/terminal/ directory.
Comment 52 Igor editbugs 2016-09-27 15:50:35 CEST
(In reply to poma from comment #48)
> Created attachment 6866 
> xfce4-terminal
> 
> Here's some nice screenie

Looking nice, indeed :)
Comment 53 Ian 2016-09-28 00:25:57 CEST
(In reply to Igor from comment #51)
> 
> It seems you need to properly install xfce4-terminal; or just manually copy
> terminal-preferences.ui file that has been generated during the build from
> ./terminal/ to /usr/share/xfce4/terminal/ directory.

Thanks, working now.
Comment 54 Ian 2016-10-08 12:59:48 CEST
Background image and transparent background not working issues have been fixed

Bug #12845

Reported by:
Ian
Reported on: 2016-09-14
Last modified on: 2016-10-08

People

CC List:
4 users

Version

Version:
0.6.92

Attachments

xfce4-terminal (494.66 KB, image/png)
2016-09-26 22:43 CEST , poma
no flags
missing slider (229.02 KB, image/jpeg)
2016-09-26 23:21 CEST , Ian
no flags

Additional information