Port Gimp 2.8 (spinscale) slider widget

[John Smith]

Gimp 2.8 has a new combo widget (called spinscale) that includes a slider, spin button, editable value and label.
Investigate using this in place of the existing spin-slider for dialogs and toolbars.

[John Smith (john-smithi)]

Patch with the new widget being used in the Layers, Fill/Stroke and on the toolbars.

[ScislaC (scislac)]

Wow, such a huge step forward imho. I only did very minimal testing and will hit it again tomorrow (I literally tested this right before going to sleep)

The only thing I noticed was that in the F&S dialog, if an object has a decent blur applied to it, there is pretty noticeable lag when doing the "minimal" adjustment sliding on the opacity slider.

It didn't feel as smooth as the ones in GIMP even w/o blur applied, however, the ones in gimp weren't directly adjusting things in realtime either.

My guess is that my issue will be resolved once we're thread-safe and fully multi-threaded (I seem to recall lag with the other sliders too when blur was involved).

[su_v (suv-lp)]

I couldn't resist testing whether the patch breaks building with GTK3 (./configure --enable-gtk3-experimental):

  CC ui/widget/gimpspinscale.o
../../src/ui/widget/gimpspinscale.c: In function ‘gimp_spin_scale_class_init’:
../../src/ui/widget/gimpspinscale.c:121: error: ‘GtkWidgetClass’ has no member named ‘size_request’
../../src/ui/widget/gimpspinscale.c:123: error: ‘GtkWidgetClass’ has no member named ‘expose_event’
../../src/ui/widget/gimpspinscale.c: In function ‘gimp_spin_scale_size_request’:
../../src/ui/widget/gimpspinscale.c:238: error: ‘GtkWidgetClass’ has no member named ‘size_request’
../../src/ui/widget/gimpspinscale.c: In function ‘gimp_spin_scale_expose’:
../../src/ui/widget/gimpspinscale.c:292: error: ‘GtkWidgetClass’ has no member named ‘expose_event’
../../src/ui/widget/gimpspinscale.c:345: warning: implicit declaration of function ‘gtk_entry_get_text_window’
../../src/ui/widget/gimpspinscale.c:345: warning: comparison between pointer and integer
../../src/ui/widget/gimpspinscale.c:353: error: ‘GtkWidgetClass’ has no member named ‘size_request’
../../src/ui/widget/gimpspinscale.c: In function ‘gimp_spin_scale_change_value’:
../../src/ui/widget/gimpspinscale.c:447: warning: initialization makes pointer from integer without a cast
../../src/ui/widget/gimpspinscale.c: In function ‘gimp_spin_scale_button_press’:
../../src/ui/widget/gimpspinscale.c:497: warning: comparison between pointer and integer
../../src/ui/widget/gimpspinscale.c: In function ‘gimp_spin_scale_motion_notify’:
../../src/ui/widget/gimpspinscale.c:564: warning: comparison between pointer and integer
../../src/ui/widget/gimpspinscale.c:585: warning: ‘gdk_cursor_unref’ is deprecated (declared at /Volumes/cyan/mp-test/with-a-long-long-long-directory-name/include/gtk-3.0/gdk/gdkcursor.h:233)
make[3]: *** [ui/widget/gimpspinscale.o] Error 1
make[2]: *** [all] Error 2
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2

Maybe Alex can help with this part?

[su_v (suv-lp)]

The files added from GIMP are GPLv3+:

 * This program is free software: you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation; either version 3 of the License, or
 * (at your option) any later version.

@ScislaC - do you know whether <http://article.gmane.org/gmane.comp.graphics.inkscape.devel/36732> has been discussed/resolved (re potential conflict between Inkscape's GPLv2 and GPLv3 / GPLv3+)?

[ScislaC (scislac)]

~suv: No, it has not been resolved yet, however, that line has already been crossed. Our licensing issues are not good to say the least, however, we do need to "get our house in order". There will be further discussions in the coming months while we figure out what to do and get the correct 3rd parties in place to help us. For now, since it's not adding an entirely new license to the mess we've already got, I don't see a huge issue. Note: As far as I'm concerned, we're not having the next release until this is resolved (bug-fix release doesn't count).

[Alex Valavanis (valavanisalex)]

I'll happily take a look at the GTK+ 3 compatibility issue, but perhaps it would be better to apply the patch in a new branch if a couple of us are going to be working on it simultaneously.

Another option would be just to commit it to trunk as is, and we can then fix the GTK+ 3 issues. At first glance, the expose function looks pretty easy to port.

[John Smith (john-smithi)]

@Alex, perhaps we dont need a new branch for this.
I dont think there is much else to do apart from the GTK3 issues. Shall we work from this patch or commit to the trunk ?

[Alex Valavanis (valavanisalex)]

Hi John,

I can commit and fix the gtk3 issues at the same time if you like. I can take care of it in the next couple of days if you like. Alternatively, if it's urgent, it's probably ok to just commit as it is and we can fix the build later.

[John Smith (john-smithi)]

Alex, if you have the time to work on the gtk3 issues, that would be great, i dont think there is any hurry on this.
BTW fantastic work on single handedly doing *ALL* the gtk3 conversions *AND* libgdl work .. amazing stuff !! thanks !!!

[Alex Valavanis (valavanisalex)]

OK, this is a little more difficult than I first thought. There's some work needed to match up event regions with redraw regions. I'm pretty busy at the moment, but I'll hopefully be able to play with it again soon if no one beats me too it.

[su_v (suv-lp)]

There seem to be ongoing efforts to port GIMP to GTK3 as well - maybe this has already been done for this special widget?
<http://git.gnome.org/browse/gimp/tree/?h=gtk3-port>

[Alex Valavanis (valavanisalex)]

Well spotted!

http://git.gnome.org/browse/gimp/log/?h=gtk3-port&qt=grep&q=spin

Yes, this should now be a fairly simple case of wrapping this work up as a conditional build.

[John Smith (john-smithi)]

This version should build and work with both GTK2 and GTK3.

[ScislaC (scislac)]

Does indeed compile and work in both gtk2 & gtk3 builds... I would say to commit as-is. A side Q, is it possible to not have the slider steal the focus from the canvas if you only "hover-scroll" (not click the widget, but use mousewheel to adjust while cursor hovers over it)?

[su_v (suv-lp)]

ScislaC wrote:
> Does indeed compile and work in both gtk2 & gtk3 builds...

Confirmed with GTK3/X11 3.4.3 and gtkmm 3.4.0 on OS X 10.7.4.

The new widgets in GTK3 seem to increase the overall height of the sliders much more than in GTK2 AFAICT.

[ScislaC (scislac)]

~suv: Yes, it does increase their height. However, I think that's a minor issue at the moment as the size of many things is off in the gtk3 builds.

[John Smith (john-smithi)]

> is it possible to not have the slider steal the focus from the canvas if you only "hover-scroll"
Has not been implemented yet. This and the focus behavior of the Esc and Enter keys is different to the current toolbar spin buttons.

[jazzynico (jazzynico)]

Looks good on Windows XP, but a bit too tall. Not a big issue with the F&S and other common dialogs, but very annoying if we implement it with no change with the custom predefined filters, some of them, having lots of sliders, may be unusable with low resolutions (I'm almost sure it doesn't work on my notebook).

[John Smith (john-smithi)]

v3 patch with the following changes :
* All Filter dialogs and Filter Effects editor also using the new controls
* Widget height has been reduced a little, should be very close to current scale controls (see next attached)
* Widget height with gtk3 should now be similar to gtk2

[jazzynico (jazzynico)]

Patch v3 tested on Ubuntu 11.04, inkscape revision 11622.
Not tested on Windows, but at least it's very nice on Ubuntu.

[John Smith (john-smithi)]

Filter->Bump->Bump comparison.
New widget appears to be about 1px larger on Ubuntu.

[Guillermo Espertino (Gez) (gespertino-gmail)]

The idea behind those widgets is to made them easy to use with a tablet. The height of the widgets in GIMP is considerably larger than a regular slider because it has to be easy to touch the upper and lower halves with the stylus (for coarse and fine control respectively).
IMO, trying to match the height of the new widgets with the old widgets goes against the benefits of the widget.

[ScislaC (scislac)]

Gez: While I agree about the intent... I still think we don't want to disenfranchise our users on netbooks or that otherwise operate at lower resolutions. I have a tablet and have tested between both Inkscape & Gimp, and the difference, while noticeable, isn't terribly painful. Regardless of us having tablet support, Inkscape isn't very tablet friendly in many ways. We have the calligraphy tool and a special feature in the node tool that supports pressure... aside from that, I'm not aware of any real tablet support offhand (I can be overlooking something, no doubt).

IMHO, supporting more users for more use cases would be more beneficial. We're not limiting tablet users aside from them having to stop and operate slightly more precisely vs potentially cutting out an entire segment of our userbase.

Please Note: The design of the widgets is not lost on me and I think once we transition to GTK3, making the "harder" choice will be easier and we just go with stock whatever The GIMP is providing.

[jazzynico (jazzynico)]

Screenshot on my notepad (1024x600).
The bump filter is too tall to fit. But since it's already too tall with the old sliders, I guess I should try to optimize the filter's dialog differently...

[John Smith (john-smithi)]

The size of the widget is controlled by one line of code (gtk2). So it would be very easy to have it "switch" to an appropriate size depending on a users preference / (is there a tablet mode pref ?) or whatever device / screen size Inkscape detects.

Perhaps the discussion about tablet, notebook and any other device support should happen outside this bug report.

But if there already exists some detection method, am happy to add support for that now.

[ScislaC (scislac)]

There is indeed a very relevant preference (there is detection as well, but it doesn't automatically enable anything). Edit>Input Devices has an option "Use pressure-sensitive tablet (requires restart)".

[John Smith (john-smithi)]

Looks like "Use pressure-sensitive tablet (requires restart)" is on by default for everyone, so not so useful for switching widget sizes as-is ...

[ScislaC (scislac)]

Really? That is a horribly stupid default. I have to go in and manually configure my tablet but that is enabled by default anyway? Well, the detection code is somehow hooked to that dialog otherwise, not that this was helpful info... :-/

[ScislaC (scislac)]

John: I still think this should make it in this release if possible. I say to commit it and we can see if JonCruz can help with the detecting a special input device on startup (to make it larger). I phrase it like that as it seems like many things require inkscape to be restarted to change the size of various widgets.

[John Smith (john-smithi)]

Committing the gimp spin-scale in 2 parts

r11960 : Add the new spin-scale controls
r11961 : Convert existing spin sliders to the new spin-scale widget (revert this to use spin sliders again)

* Added function to set the appearance as "full" (like gimp, useful for tablets) or "compact" (same size as existing spin-sliders)
* Keyboard <Enter> and <Esc> return focus to the canvas

[insaner (insaner)]

was not able to compile on fedora 14 with

 $ rpm -q gtkmm24
gtkmm24-2.22.0-1.fc14.i686

(14:28:35) insaner: $ rpm -q gtk2
gtk2-2.22.0-1.fc14.1.i686

it was throwing a
  CXX inkscape-version.o
  AR libinkversion.a
  CXX main.o
  CXXLD inkscape
libinkscape.a(gimpspinscale.o): In function `gimp_spin_scale_change_value':
/tmp/compile/inkscape/src/ui/widget/gimpspinscale.c:685: undefined reference to `gdk_window_get_width'
libinkscape.a(gimpspinscale.o): In function `gimp_spin_scale_expose':
/tmp/compile/inkscape/src/ui/widget/gimpspinscale.c:435: undefined reference to `gdk_window_get_width'
collect2: ld returned 1 exit status
make[3]: *** [inkscape] Error 1
make[3]: Leaving directory `/tmp/compile/inkscape/src'
make[2]: *** [all] Error 2

so i googled this problem and found that gdk_window_get_width() is new to 2.24 (im using 2.22). heres the fix

line 435:

replace:
 ----------------
    w = gdk_window_get_width (event->window);
 ----------------

with:

 ----------------
#if GTK_CHECK_VERSION(2, 24,0)
    w = gdk_window_get_width (event->window);
#else
    gdk_drawable_get_size (event->window, &w, NULL);
#endif
 ----------------

line 685:

replace:
 ----------------
  width = gdk_window_get_width (text_window);
 ----------------

with:

 ----------------
#if GTK_CHECK_VERSION(2, 24,0)
  width = gdk_window_get_width (text_window);
#else
  gdk_drawable_get_size (text_window, &width, NULL);
#endif

 ----------------

this allowed me to compile

[John Smith (john-smithi)]

@insaner, thanks for that.
Committed to trunk as r11732.

[insaner (insaner)]

no problem, my pleasure.. ill be contributing wherever i can.. thanks to all of you guys for such an excellent job you are doing with inkscape!

[Vladimir Savic (vladimir-firefly-savic)]

Why there is so big white border around slider? It doesn't happen in GIMP.
I think there is no room for even single pixel border, let alone one this big. Background and slider must be of different color, but it is a theme's problem, not ours.

BTW, There is one ingenious aspect of this widget design we're still not exploiting enough. Namely, Its width can be just enough to percentage or value (100,0% part) and it'll still be useful as slider. Not that it should be really that "skinny" in practice... :)

[su_v (suv-lp)]

Follow-up reports requesting to revert the new slider widgets:
- Bug #1184408 “Zoom Slider bar makes it very difficult to type in large zoom values (Win32, bzr r12342)”
  <https://bugs.launchpad.net/inkscape/+bug/1184408>
- Bug #1189846 “Bug with slider textfield”
  <https://bugs.launchpad.net/inkscape/+bug/1189846>

[su_v (suv-lp)]

> Follow-up reports requesting to revert the new slider widgets:

Similar concerns seem to affect users of GIMP too, for example:
<https://bugzilla.gnome.org/show_bug.cgi?id=702372>

[Lebostein (lebostein)]

I click in a spin box to edit the values by typing numbers I change the value because I touch the slider every time. That is frustrating. That is a catastrophic design failure in my eyes. I see no advantage in this function. It makes typing and marking of numbers to a game of patience. I hope this could be deactivated.

Why you don't use a combination of a slider beside a spin box like the RGBA color values inside the Fill and Stroke dialog? You should strive for a consistent structure of the GUI elements!

[ScislaC (scislac)]

If you hover the cursor over the text in a spinscale widget, it turns to a text cursor to edit the text. There is no design failure there. It saves a large amount of space by putting the labels inside the sliders (especially in cases of translations to other languages, of which we have many) and allows for a greater more ways to modify values quickly. I'm sorry you see no value in it, but perhaps someone will look into implementing a preference to disable it in the future.

[Lebostein (lebostein)]

That's the ugliest GUI element I saw/used in my life! THAT IS A DESIGN FAILURE.
That "hover the cursor over the text" don't work. It is a very small area you have to find if the cursor changes to use the gadget as a text input gadget. That is pixel hunting in its purest form.

[Ellisthion (ellisthion)]

This new widget is horrible. I hate it in GIMP, and I don't understand why you would copy something like that without making sure it was actually usable.