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 766372 - Scale omits value in various size calculations, causing insufficient allocation and clipping/spillover of value text
Scale omits value in various size calculations, causing insufficient allocati...
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
3.20.x
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-05-13 12:56 UTC by Daniel Boles
Modified: 2016-10-27 08:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
mcve showing orientation/value_pos combinations causing clipping OR overlap (2.51 KB, text/plain)
2016-05-14 13:32 UTC, Daniel Boles
  Details
pic of MCVE on GTK+ 3.14 shows clipping by certain orient/pos combos (10.64 KB, image/png)
2016-05-14 13:36 UTC, Daniel Boles
  Details
pic of MCVE on GTK+ 3.20 shows value/widget overlap by certain orient/pos combos (13.40 KB, image/png)
2016-05-14 13:37 UTC, Daniel Boles
  Details
proposed impl, diff @ master 4dbe7c99c2390a9613fb79a436c6e8b6384808f8 (5.73 KB, patch)
2016-05-31 22:21 UTC, Daniel Boles
committed Details | Review
quick'n'lazy hack to widget-factory showing the results of the patch (103.09 KB, image/png)
2016-05-31 22:22 UTC, Daniel Boles
  Details
alternative patch to gtkscale.c - don't probe value_gadget size if not needed (2.97 KB, patch)
2016-05-31 23:22 UTC, Daniel Boles
none Details | Review
alternative patch to gtkscale.c - don't probe value_gadget size if not needed (2.92 KB, patch)
2016-05-31 23:28 UTC, Daniel Boles
none Details | Review
screencast showing that a variant of this actually IS a problem in master (381.53 KB, video/webm)
2016-06-02 20:27 UTC, Daniel Boles
  Details
patch: request min value size from base range. include mid value in width calc (7.73 KB, patch)
2016-06-02 22:24 UTC, Daniel Boles
none Details | Review

Description Daniel Boles 2016-05-13 12:56:28 UTC
Hello,

Scale value text (drawn to Pango, not a Label) is not included when calculating width for allocation, expansion, etc. This means it's very easy to end up with a Scale whose allocation is narrower than its value 'label'. This causes the value text to be truncated at the edges, which isn't exactly visually useful!

Can there be a way/option add the value's width into size calculations done for the Scale? This is available as it is (now, after my last ticket) used to align the text in the layout (set min then max; take max text width). This would then enable such sizing operations to be done correctly for value-showing Scales.

Most importantly, those adjustments would then be made natively and cleanly by the widget itself. At the moment, I set a minimal width that /seems/ to cover all /current/ cases on this OS, font, etc. But I feel like this is a bad hack. I also can't think of any pleasant way to pull the width out of the Pango layout and incorporate it myself. So, it would be great if we didn't have to!

Thanks again.
Comment 1 Daniel Boles 2016-05-13 13:15:12 UTC
Possibly relevant old bug, still open: https://bugzilla.gnome.org/show_bug.cgi?id=578626
Comment 2 Timm Bäder 2016-05-13 15:31:55 UTC
The scale value node is currently being reevaluated in its current position (since it's causing some situation to be impossible to style correctly using CSS), so we might fix this case with it.

Anyway, since nobody has stumbled upon the value node being underallocated so far, can you provide a minimal test case?
Comment 3 Matthias Clasen 2016-05-13 15:57:16 UTC
thats an independent issue. lets not conflate them
Comment 4 Daniel Boles 2016-05-13 16:11:31 UTC
Hi Timm, great to hear, and would be happy to upload one tonight or tomorrow.

As mentioned, the non-allocation is most noticeable on vertical Scales, whose values get cut off at sides. But it also causes horizontal Scales to have their troughs 'eaten' by the text. While I'd prefer this didn't happen for my current project, I appreciate it can be good or bad, depending on context and taste.

So, an option to have either behaviour would be great. I've been pondering more and think it may require a set_trough_length method - to allow setting a 'size request' for the trough that isn't stolen by the value. That would internally do something like set_size_request(passed_length + priv->value_width, -1) or vice-versa for a v-scale. Can't help but feel I read about this method before, but I can't find it anywhere now! If I'm pondering correctly, it seems having both set_size_request and _trough_length would neatly support both preferences.
Comment 5 Matthias Clasen 2016-05-13 16:15:35 UTC
I don't think any options or new api are needed here. The scale size request and allocation code simply needs to be fixed to take the value into account.
Comment 6 Daniel Boles 2016-05-13 16:34:10 UTC
You're probably right, of course. Since that is a distinct issue from the value clipping, and would act much the same (i.e. no distinct 'trough length') before and after the proposed fix, I guess I should request it as a separate exercise?
Comment 7 Daniel Boles 2016-05-14 13:32:45 UTC
Created attachment 327884 [details]
mcve showing orientation/value_pos combinations causing clipping OR overlap

This is interesting as it reveals that SOME cases already cause appropriate size to be allocated for the value. I hadn't noticed these in my normal development.

Screenshots to follow. The light themed screenshot is 3.14 from Debian stable. The dark is 3.20 from Debian unstable. (note the latter has differing value text alignments as per my previous bug, now fixed in master - thanks again)

In older GTK+, 'unallocated' values are clipped, whereas with 3.20, they're still not allocated, but they're not clipped, so they just spill into the space of other widgets! I'm guessing the transparent background is intentional, and fixing this bug will make this difference moot. Seemed worth mentioning anyway.

So, we get these:

vscale1: orientation vertical, value pos top: CLIPPED/OVERLAPPED at left/right
vscale2: vertical, left: ALLOCATED horizontal size for text

hscale1: vertical, left: I haven't yet figured out how to get OpenBox to set the font height sufficiently large, but according to the prior ticket, this would cause the text to be clipped at the top and bottom. By symmetry with the vscale/top value, this seems like a fair assumption.
hscale2: horizontal, left, newlines in text to force height (absent font size): CLIPPED/OVERLAPPED at top/bottom
hscale3: horizontal, top: ALLOCATED vertical size for the text, including newlines (not just font size per line)

HTH and let me know if anything further is required.
Comment 8 Daniel Boles 2016-05-14 13:36:13 UTC
Created attachment 327885 [details]
pic of MCVE on GTK+ 3.14 shows clipping by certain orient/pos combos

Debian stable/OpenBox
Comment 9 Daniel Boles 2016-05-14 13:37:10 UTC
Created attachment 327886 [details]
pic of MCVE on GTK+ 3.20 shows value/widget overlap by certain orient/pos combos

Debian unstable/GNOME
Comment 10 Daniel Boles 2016-05-14 13:39:22 UTC
Comment on attachment 327885 [details]
pic of MCVE on GTK+ 3.14 shows clipping by certain orient/pos combos

Please note the 3.14 screenshot uses a slightly earlier version of the test.c, in which the only difference is the label is "big\nlabel:\n%f", whereas to confirm what was going on in 3.20, I added an extra line "REALLY\n" to the start of that. So rest assured 3.14 isn't just omitting the entire 1st line :-)
Comment 11 Daniel Boles 2016-05-14 14:12:43 UTC
Comment on attachment 327884 [details]
mcve showing orientation/value_pos combinations causing clipping OR overlap

I always forget something. This was compiled with:
gcc `pkg-config --cflags --libs gtk+-3.0` -std=c11 test.c
Comment 12 Matthias Clasen 2016-05-15 15:25:29 UTC
So in fact, nothing changed here in GtkScale's behavior. It has never taken the size of the value fully into account. What changed is that we no longer clip the drawing. This is not easy to fix properly due to the way the size allocation and layout is split between GtkRange and GtkScale. The value is maintained in GtkScale, but we can only communicate a border size, which lets us make sure that the range is wide enough for the value label, but not that it is high enough (if the value is positioned horizontally).
Comment 13 Daniel Boles 2016-05-15 16:02:23 UTC
Yeah, I didn't think much has changed, just that the established behaviour was lacking.

I don't quite follow your last sentence. How is it that, regardless of whether the scale is oriented vertically or horizontally - size is allocated if the value is 'floating' but not when it's fixed? I'm probably thinking about this oversimplistically, but it seems to indicate that the required size is available, just not taken into account in the fixed case.

I assume there's some crucial difference that makes the latter difficult as you've said, but I don't know the internals well enough to guess what that is.
Comment 14 Matthias Clasen 2016-05-15 20:31:27 UTC
The difficulty is entirely in the way the code is structured. Here is 
how the range looks, schematically. It has an api for subclasses to say
how big the border area should be. 

+-------------------------------------+
| border                              |
|  +-------------------------------+  |
|  | marks                         |  |
|  +-------------------------------+  |
|  | range trough + slider         |  |
|  +-------------------------------+  |
|  | marks                         |  |
|  +-------------------------------+  |
|                                     |
+-------------------------------------+

The scale uses that api to reserve enough space to put the value
in one of the places P1,...,P4:

+-------------------------------------+
|                P1                   |
|  +-------------------------------+  |
|  | marks                         |  |
|  +-------------------------------+  |
|P2| range trough + slider         |P3|
|  +-------------------------------+  |
|  | marks                         |  |
|  +-------------------------------+  |
|               P4                    |
+-------------------------------------+

But since the area we request is a border, it can only ensure that we
have enough height (for P1 and P4) or enough width (for P2 and P3). 

This could be fixed by making the API between range and scale more complicated, but I don't think this is a high priority. Putting large labels for the value is just not how this api is meant to be used.
Comment 15 Daniel Boles 2016-05-15 20:57:49 UTC
Thanks for the explanation. So, it seems like this border is configurable for each of the 4 sides - as in the 'good' cases I've shown, the vscale with value_pos left gets an appropriate left border, and the hscale with value_pos top a top border - without leaving empty space on the other 3 sides of either.

So I deduce that this border offers either a separate width setting for each side - OR a selection of one side only. If the former, I still don't see why this couldn't be solved for for the other cases: give the vscale with value_pos top appropriate left and right borders, and the hscale with value_pos left appropriate top and bottom borders. If the latter, then yeah, that's difficult.

Still, for the perhaps-most-common case of a vscale with value at top, I don't think it's esoteric or outwith the API to expect that the scale will have appropriate width for most of its values, without the user having to hack it in?
Comment 16 Daniel Boles 2016-05-31 05:35:41 UTC
Matthias, when you get a moment, could you comment on my understanding (or the lack thereof) above and whether any of this is feasible to improve? Thanks.
Comment 17 Matthias Clasen 2016-05-31 18:17:33 UTC
We can't really use the border values to ensure the width in the 'vertical + value on top' case, since the border always *adds* to the existing width. What we need is a way to MAX() the the width of the scale with the width of the value label.

One way this could be done would be to add a way for GtkRange subclasses to provide a min-width/min-height, and make GtkRange MAX() those values with the size that it computed in the current way.

A scale with draw-value == TRUE could then pass the width+height of the value label for min-width/min-height.
Comment 18 Daniel Boles 2016-05-31 22:19:30 UTC
Thanks, I looked at the source files a bit more and combined with your explanation, could understand how the border works and why it's not the answer.

More importantly, your very intuitive summary and excellent proposal started to make this all seem relatively trivial! So I couldn't stop my curiosity, started reading the source, and ended up trying to implement your idea...

git master compiled, hacked the range and scale files, then did a quick and lazy hack to demos/widget-factory. Looks like it works for a trivial case of the ol' 'vertical scale with wide label'! and I'll test in more depth after some sleep...

Obviously all your idea, but maybe I save you the work :-) Feedback appreciated. Not a bad start for my first bit of OSS hacking and since I've only ever used GTK+ not via GTKmm for bug reports - haha - well, maybe that'll change now :-)

attachments coming up next
Comment 19 Daniel Boles 2016-05-31 22:21:14 UTC
Created attachment 328844 [details] [review]
proposed impl, diff @ master 4dbe7c99c2390a9613fb79a436c6e8b6384808f8
Comment 20 Daniel Boles 2016-05-31 22:22:15 UTC
Created attachment 328845 [details]
quick'n'lazy hack to widget-factory showing the results of the patch
Comment 21 Daniel Boles 2016-05-31 23:22:16 UTC
Created attachment 328850 [details] [review]
alternative patch to gtkscale.c - don't probe value_gadget size if not needed

fwiw, I wondered if it might be better to avoid the overhead of calcing the value_gadget size in cases where the border already allocates space for us - if so, here's an example (which again seems to work fine so far) of how we could avoid that, by exiting early in cases that don't require the extra size request
Comment 22 Daniel Boles 2016-05-31 23:28:40 UTC
Created attachment 328851 [details] [review]
alternative patch to gtkscale.c - don't probe value_gadget size if not needed

(attempt #2: fixed the pointless double nested if statement)

fwiw, I wondered if it might be better to avoid the overhead of calcing the value_gadget size in cases where the border already allocates space for us - if so, here's an example (which again seems to work fine so far) of how we could avoid that, by exiting early in cases that don't require the extra size request
Comment 23 Daniel Boles 2016-06-02 18:57:19 UTC
If this were to go anywhere, I'd also ask that we make the value measurement a bit more resilient - by testing the middle value of the range too. Case: I have several scales that run from "0.000 dB" to "-95.625 dB" at (upper - 1), and then max is "OFF", i.e. denotes special handling. I'm sure there are other examples.

With the current implementation, only lower and upper are considered, meaning that the larger value strings in the interim are not accommodated and get line-broken, spilling over the trough.

Of course, it's trivial to solve for such cases - which admittedly are probably edge ones anyway - by extending gtk_scale_measure_value() to perform its whole

txt = gtk_scale_format_value ( /* ...
... */
height = MAX (height, logical_rect.height);

routine additionally for the value (upper - lower) / 2. Since it's a simple 6 lines of code, hopefully it could accompany the more elaborate stuff up above. Seems to be doing the trick here :-)
Comment 24 Daniel Boles 2016-06-02 20:27:46 UTC
Created attachment 328999 [details]
screencast showing that a variant of this actually IS a problem in master

haha, wow, while using queue_resize() fixes my original non-bug in 3.20, that one is now ACTUALLY broken by something else changed in master! I feel simultaneously vindicated and disappointed ;-)

See the attached video. Basically, when toggling value widths, the split of space between the value and trough IS renegotiated successfully, BUT the value text remains in its pre-toggle position and alignment, now wrong, till I focus away/back to the window containing the Scale. This is a different symptom, and unlike originally, hovering DOESN'T fix it this time - only a change in focus (away and back to the parent Window) kicks it into its proper final state.

I feel like I must be hallucinating at this point. Can someone take the original example, s/queue_draw/queue_resize/, compile against master, and confirm this?

I'd look into this myself if I knew where to start. Which event occurs on the window losing/gaining focus? It must handle some part of the value redraw that is omitted by queue_draw() and queue_resize(). Tomorrow with fresher eyes I might try to find this in the log, unless someone involved can do me a favour!
Comment 25 Daniel Boles 2016-06-02 20:29:59 UTC
Comment on attachment 328999 [details]
screencast showing that a variant of this actually IS a problem in master

wrong thread
Comment 26 Daniel Boles 2016-06-02 22:24:12 UTC
Created attachment 329007 [details] [review]
patch: request min value size from base range. include mid value in width calc
Comment 27 Daniel Boles 2016-06-13 19:01:40 UTC
Thanks for pushing my patch. I guess the later 'fast' version was too much code for not enough gain.

Can this be pushed to 3.20?