GNOME Bugzilla – Bug 783649
negative content width warning in GtkLevelbar
Last modified: 2017-08-28 23:36:15 UTC
When we set the Gtklevelbar value to zero the following warning is shown: (test.py:6736): Gtk-WARNING **: Negative content width -2 (allocation 0, extents 1x1) while allocating gadget (node block, owner GtkLevelBar) This is the test code that generates this warning: #!/usr/bin/env python3 import gi gi.require_version('Gtk', '3.0') from gi.repository import Gtk class LevelBar(Gtk.Window): def __init__(self): Gtk.Window.__init__(self) self.set_default_size(150, -1) self.connect("destroy", Gtk.main_quit) levelbar = Gtk.LevelBar() levelbar.set_min_value(0) levelbar.set_max_value(10) levelbar.set_value(0) self.add(levelbar) window = LevelBar() window.show_all() Gtk.main() After performing some tests I found out that this warning is not triggered if I change the gtk theme from Adwaita to another one like Adapta. So maybe this problem is not in the GtkLevel but in the Adwaita theme. I am using gtk 3.22.15 in Arch Linux.
Still happens on 3.22.17
Fixed by cherry-picking Timm's commit fe5f650088414a3a29a4dd77e896989dcd0e2c30 from master to gtk-3-22
wut. I meant https://git.gnome.org/browse/gtk+/commit/gtk/gtklevelbar.c?id=e25e1c54a44264de628fb3919fb47afd44e21ba5
Hi I have just updated to gtk 3.22.18 and the fix to this issue seems to have added another one. Now the levelbar is always partially full even if we do levelbar.set_value(0). It is like there is an offset to the value.
I have just noticed that the levelbar is also always partially full inside Glade too.
Thanks. I've reverted the commit in GTK+ 3. The fact that this adds a discrete block at the min side when value == 0 is a problem in GTK+ 4 too. I just assumed that commit had been tested there and was known to be valid... That should probably be reverted in GTK+ 4 too. We'll need to find a fix that resolves both problems.
Ultimately the fact that the themes set min-width|height to 32 px on the oriented dimension seems wrong to me - at least without support from the widget to enforce that the value can't be set (barring 0) small enough to require a narrower bar.
I knew that master had that styling problem, but left it to lapo. That commit is still valid of course, whatever other solution you find will involve not underallocating widgets/gadgets.
Sure. It would be great if, when committing to master with problems known at the time, the commit message could include that, so that no one is at risk of cherry-picking under an assumption that no problem exists. My bad for not testing personally, of course, but still. :)
I'll take a look, I'm not sure requiring theme changes is ok for gtk+3, if that warning is innocuous (no idea, Timm?), maybe it's better to live with it?
I never saw a problem that seemed to be related to this warning. But as I use 18 levelbar in my application these messages have become a huge spam in my system log. Is there a way to disable only this warning?
Ok, with the current state of things, we can either apply Timm's patch and fix Adwaita to adhere (already fixed in master with 96062ffeae5245fa165a96a2af86d5645f5e8569) or leave this issue alone. The first option requires theme changes or the first levelbar block would be filled even with a 0 value (right Daniel?), in my humble opinion is better to live with that warning frankly speaking, Daniel what do you think?
Thanks for taking a look, Lapo! (In reply to Lapo Calamandrei from comment #12) > Ok, with the current state of things, we can either apply Timm's patch and > fix Adwaita to adhere (already fixed in master with > 96062ffeae5245fa165a96a2af86d5645f5e8569) If no one else reapplies Timm's patch beforehand, I'll have a look over the weekend. > The first option requires theme changes or the first levelbar block would be > filled even with a 0 value (right Daniel?) Yeah, applying Timm's patch on GTK+ 3 or 4, without corresponding theme changes, results in the warnings disappearing - but there always being a certain width of filled block at the low end - even when the value is 0 or otherwise too small that to justify that level of fill. > in my humble opinion is better > to live with that warning frankly speaking, Daniel what do you think? You'd need to check with a maintainer how they feel, but personally I think it would be nice if we can remove the warnings in GTK+ 3 - assuming that the act of doing so doesn't result in any visual changes anywhere else. This assumes that currently, the min-(width|height) are equally unnecessary/unwanted in GTK+ 3, but we are (as we were before cherry-picking Timm's patch) saved by the lack of checks for underallocation in the code, which result in the block being hidden/shrunk appropriately for the value, but GTK+ not being happy about that fact. So I'll let someone else say what is appropriate for GTK+ 3 in this case.
the min size is pretty clear here, it's the minimum size of the widget, period. That's nothing new, if a theme ever sets a min-height of 200px for a scrollbar slider, that's just fucked up but you still can't underallocate it. You have to check for the min size in any case, since border, padding and margin count into that as well so even if you remove the min-width/height, allocating at width 0 or 1 would underallocate currently since the block has a 1px border on both sides resulting in a min width of 2px. The solution is basically to revert the revert and let themes do what they want, you can't keep them from applying padding/border/margin/min-width/min-height to the levelbar blocks.
Right, but GTK+ 3 presents a problem because the previous/current behavior is clearly wrong but had the right result... i.e. that if the value is 0, no block should be shown, as neither should it if the value is > 0 but too small (relative to the current widget size) for it to occupy any pixels (in continuous mode, 1 px, not 32 px) or the min size of a block (in discrete mode) Isn't the real fix, then, not to show the filled block widget, if the value is too small for it to be appropriate? (I was experimenting with this but didn't crack it yet.) That way it can't be underallocated, but it won't show up when it's unwelcome. Simply having it always show and letting themes make it whatever weird size they want doesn't seem right to me, at least with my current understanding of this widget.
(In reply to Daniel Boles from comment #15) > Isn't the real fix, then, not to show the filled block widget, if the value > is too small for it to be appropriate? (I was experimenting with this but > didn't crack it yet.) That way it can't be underallocated, but it won't show > up when it's unwelcome. I just realized GtkRange presents an ideal model of what should be done here, and will presumably be educational in trying this out as a proposed solution.
(In reply to Daniel Boles from comment #15) > Right, but GTK+ 3 presents a problem because the previous/current behavior > is clearly wrong but had the right result... i.e. that if the value is 0, no > block should be shown, as neither should it if the value is > 0 but too > small (relative to the current widget size) for it to occupy any pixels (in > continuous mode, 1 px, not 32 px) or the min size of a block (in discrete > mode) Something I wonder about all this: Is the problem simply that we didn't consider the prospect of continuous mode when setting that minimal size on the blocks? i.e. we were not selecting on levelbar.discrete block, just levelbar block In continuous mode, you get an unfilled block with a filled block drawn over it. The filled block should be proportional to the fill level, not clamped to any minimum size. If so, it would make sense for themes to want to set a minimal size for discrete-mode blocks, but that should not be applied in continuous mode. I guess we'd still need to avoid allocating/drawing the continuous filled block if the value == 0, anyway
Created attachment 357473 [details] [review] LevelBar: Really fix underallocation of blocks Themes should not enforce min sizes on blocks in continuous mode; in this case, the filled block should be as large as it needs to be to reflect the current value, and no larger or smaller than that. So, the fact that the minimal size was selected on just levelbar block is wrong: we should also require the levelbar.discrete class to apply min sizes. The widget should enforce the corrected minimum size resulting from the above fix, by reapplying commit 78b4885fe8850e132d8bb06df8ab90ac6c2033e0 Except: we should not allocate/draw the filled block if the value is 0, as in this case, the LevelBar should be empty, not have a min-size fill. -- What do you guys think of this?
Created attachment 357474 [details] [review] LevelBar: Really fix underallocation of blocks Themes should not enforce min sizes on blocks in continuous mode; in this case, the filled block should be as large as it needs to be to reflect the current value, and no larger or smaller than that. So, the fact that the minimal size was selected on just levelbar block is wrong: we should also require the levelbar.discrete class to apply min sizes. The widget should enforce the corrected minimum size resulting from the above fix, by reapplying commit 78b4885fe8850e132d8bb06df8ab90ac6c2033e0 Except: we should not allocate/draw the filled block if the value is 0, as in this case, the LevelBar should be empty, not have a min-size fill. -- I accidentally attached an old version of the patch. This one avoids drawing the block if the current value is 0, which seems to be required.
Created attachment 357475 [details] [review] LevelBar: Really fix underallocation of blocks Themes should not enforce min sizes on blocks in continuous mode; in this case, the filled block should be as large as it needs to be to reflect the current value, and no larger or smaller than that. So, the fact that the minimal size was selected on just levelbar block is wrong: we should also require the levelbar.discrete class to apply min sizes. The widget should enforce whatever correct minimum size results from the above fix, by reapplying commit 78b4885fe8850e132d8bb06df8ab90ac6c2033e0 Except: we should not allocate/draw the filled block if the value is 0, as in this case, the LevelBar should be empty, not have a min-size fill. -- This adds back the missing min of 1px in the opposite dimension, which probably shouldn't be dropped (at least not as part of this). I think I've caught all the gremlins in this one now, and my (admittedly not very elaborate) tests all display correctly without any of the offending warning or any others - but reviews would be much appreciated.
Created attachment 357478 [details] [review] LevelBar: Really fix underallocation of blocks Themes should not enforce min sizes on blocks in continuous mode; in this case, the filled block should be as large as it needs to be to reflect the current value, and no larger or smaller than that. So, the fact that the minimal size was selected on just levelbar block is wrong: we should also require the levelbar.discrete class to apply min sizes. The widget should enforce whatever correct minimum size results from the above fix, by reapplying commit 78b4885fe8850e132d8bb06df8ab90ac6c2033e0 Except: we should not allocate/draw the filled block if the value is 0, as in this case, the LevelBar should be empty, not have a min-size fill. -- now with no unrelated changes caused by another commit picking stuff from master and not regenerating the CSS (which I have just done)
Lapo said to go for it, once I eventually provided a better explanation than I had in the commit message. :P Pushed to gtk-3-22 and master.