Bug 261932 - Indicator line in GtkVolumeButton is not displayed in oxygen-gtk
Summary: Indicator line in GtkVolumeButton is not displayed in oxygen-gtk
Status: CLOSED FIXED
Alias: None
Product: Oxygen
Classification: Plasma
Component: gtk2-engine (show other bugs)
Version: unspecified
Platform: Unlisted Binaries Linux
: NOR normal
Target Milestone: ---
Assignee: Hugo Pereira Da Costa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-03 09:05 UTC by Gökçen Eraslan
Modified: 2011-07-29 23:35 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gökçen Eraslan 2011-01-03 09:05:52 UTC
Version:           unspecified
OS:                Linux

In clearlooks style, there is a progress-like line to indicate the sound level (the blue line here[1][2]), but in oxygen-gtk[3], there is no such indicator. 

It would be good if there is such an indicator in oxygen-gtk style, but I don't know if this is a decision or a bug.

[1] http://upload.wikimedia.org/wikipedia/commons/1/1b/GtkVolumeButton.png
[2] http://origin.arstechnica.com/reviews/os/gnome-2-20-review.media/gtk-volume.png
[3] http://i1012.hizliresim.com/2010/12/26/3610.png

Reproducible: Always
Comment 1 Hugo Pereira Da Costa 2011-01-03 09:27:27 UTC
Definitely a bug.
To be consistent with Qt/Kde, the design should not have a blue line like in Clearlooks, but there should be a vertical grey sunken line, like for regular sliders. 

To help debug this: what is the name of the application that displays the incorrect widget ? 

Thanks,

Hugo
Comment 2 Gökçen Eraslan 2011-01-03 09:35:59 UTC
(In reply to comment #1)
> Definitely a bug.
> To be consistent with Qt/Kde, the design should not have a blue line like in
> Clearlooks, but there should be a vertical grey sunken line, like for regular
> sliders. 
> 
> To help debug this: what is the name of the application that displays the
> incorrect widget ? 

I'm using gnome-mplayer[1].

> 
> Thanks,
> 
> Hugo

[1] http://code.google.com/p/gnome-mplayer/
Comment 3 Hugo Pereira Da Costa 2011-01-03 11:43:19 UTC
commit 06bf6559d5f515323f8819b19d04ea932c2dc6aa
branch 1.0
Author: Hugo Pereira Da Costa <hugo@oxygen-icons.org>
Date:   Mon Jan 3 11:41:28 2011 +0100

    Use GtkOrientable interface to get slider orientation for GtkScale widgets.
    CCBUG: 261932

diff --git a/src/oxygenstylewrapper.cpp b/src/oxygenstylewrapper.cpp
index c94241d..229cb4e 100644
--- a/src/oxygenstylewrapper.cpp
+++ b/src/oxygenstylewrapper.cpp
@@ -812,21 +812,26 @@ namespace Oxygen
             StyleOptions options = StyleOptions( widget, state, shadow );
             Style::instance().renderMenuItemRect( window, clipRect, widget, x, y, w, h, options );
 
-        } else if( d.isTroughAny() && GTK_IS_VSCALE( widget ) ) {
+        } else if( d.isTroughAny() && GTK_IS_SCALE( widget ) ) {
 
-            // TODO: calculate this value from the style "slider-width" property
+            const bool vertical( gtk_orientable_get_orientation( GTK_ORIENTABLE( widget ) ) == GTK_ORIENTATION_VERTICAL );
             const int offset( 6 );
-            if( d.isTrough() ) Style::instance().renderSliderGroove( window, clipRect, x, y + offset, w, h - 2*offset, Vertical );
-            else if( d.isTroughLower() ) Style::instance().renderSliderGroove( window, clipRect, x, y + offset, w, h, Vertical );
-            else if( d.isTroughUpper() ) Style::instance().renderSliderGroove( window, clipRect, x, y, w, h - offset, Vertical );
+            if( vertical ) {
 
-        } else if( d.isTroughAny() && GTK_IS_HSCALE( widget ) ) {
+                // TODO: calculate this value from the style "slider-width" property
+                if( d.isTrough() ) Style::instance().renderSliderGroove( window, clipRect, x, y + offset, w, h - 2*offset, Vertical );
+                else if( d.isTroughLower() ) Style::instance().renderSliderGroove( window, clipRect, x, y + offset, w, h, Vertical );
+                else if( d.isTroughUpper() ) Style::instance().renderSliderGroove( window, clipRect, x, y, w, h - offset, Vertical );
 
-            // TODO: calculate this value from the style "slider-width" property
-            const int offset( 6 );
-            if( d.isTrough() ) Style::instance().renderSliderGroove( window, clipRect, x + offset, y, w - 2*offset, h, StyleOptions() );
-            else if( d.isTroughLower() ) Style::instance().renderSliderGroove( window, clipRect, x + offset, y, w, h, StyleOptions() );
-            else if( d.isTroughUpper() ) Style::instance().renderSliderGroove( window, clipRect, x, y, w - offset, h, StyleOptions() );
+            } else {
+
+                // TODO: calculate this value from the style "slider-width" property
+                const int offset( 6 );
+                if( d.isTrough() ) Style::instance().renderSliderGroove( window, clipRect, x + offset, y, w - 2*offset, h, StyleOptions() );
+                else if( d.isTroughLower() ) Style::instance().renderSliderGroove( window, clipRect, x + offset, y, w, h, StyleOptions() );
+                else if( d.isTroughUpper() ) Style::instance().renderSliderGroove( window, clipRect, x, y, w - offset, h, StyleOptions() );
+
+            }
 
         } else if( d.isTrough() && shadow == GTK_SHADOW_IN ) {
Comment 4 Hugo Pereira Da Costa 2011-01-03 11:47:55 UTC
That should fix it
(Damn: someone should really limit the size of the diff on bugzilla).

Anyway, thanks for reporting. 
Closing the bug.
Comment 5 Gökçen Eraslan 2011-01-19 08:48:44 UTC
I can reproduce this bug with 1.0.1, is the patch included?
Comment 6 Hugo Pereira Da Costa 2011-01-19 10:23:02 UTC
Well, it was, and somehow the bug reappeared (a regression). Both in 1.0 and master.
Investigating.
Sorry.
Comment 7 Hugo Pereira Da Costa 2011-01-19 10:26:11 UTC
... must be an incorrect conflict resolution. The changes are simply gone from the current code. Bisecting.
Comment 8 Hugo Pereira Da Costa 2011-01-19 10:43:12 UTC
commit 88b21265822c7120eb5ae50012c78c42bb83d044
branch 1.0
Author: Hugo Pereira Da Costa <hugo@oxygen-icons.org>
Date:   Wed Jan 19 10:35:00 2011 +0100

    Re-added 06bf6559d5f515323f8819b19d04ea932c2dc6aa
    (not sure how it got lost)
    Use GtkOrientable interface to get slider orientation for GtkScale widgets.
    CCBUG: 261932

diff --git a/src/oxygenstylewrapper.cpp b/src/oxygenstylewrapper.cpp
index c21fc6f..8eb34c5 100644
--- a/src/oxygenstylewrapper.cpp
+++ b/src/oxygenstylewrapper.cpp
@@ -830,21 +830,26 @@ namespace Oxygen
             StyleOptions options = StyleOptions( widget, state, shadow );
             Style::instance().renderMenuItemRect( window, clipRect, widget, x, y, w, h, options );
 
-        } else if( d.isTroughAny() && GTK_IS_VSCALE( widget ) ) {
+        } else if( d.isTroughAny() && GTK_IS_SCALE( widget ) ) {
 
-            // TODO: calculate this value from the style "slider-width" property
+            const bool vertical( gtk_orientable_get_orientation( GTK_ORIENTABLE( widget ) ) == GTK_ORIENTATION_VERTICAL );
             const int offset( 6 );
-            if( d.isTrough() ) Style::instance().renderSliderGroove( window, clipRect, x, y + offset, w, h - 2*offset, Vertical );
-            else if( d.isTroughLower() ) Style::instance().renderSliderGroove( window, clipRect, x, y + offset, w, h, Vertical );
-            else if( d.isTroughUpper() ) Style::instance().renderSliderGroove( window, clipRect, x, y, w, h - offset, Vertical );
+            if( vertical ) {
 
-        } else if( d.isTroughAny() && GTK_IS_HSCALE( widget ) ) {
+                // TODO: calculate this value from the style "slider-width" property
+                if( d.isTrough() ) Style::instance().renderSliderGroove( window, clipRect, x, y + offset, w, h - 2*offset, Vertical );
+                else if( d.isTroughLower() ) Style::instance().renderSliderGroove( window, clipRect, x, y + offset, w, h, Vertical );
+                else if( d.isTroughUpper() ) Style::instance().renderSliderGroove( window, clipRect, x, y, w, h - offset, Vertical );
 
-            // TODO: calculate this value from the style "slider-width" property
-            const int offset( 6 );
-            if( d.isTrough() ) Style::instance().renderSliderGroove( window, clipRect, x + offset, y, w - 2*offset, h, StyleOptions() );
-            else if( d.isTroughLower() ) Style::instance().renderSliderGroove( window, clipRect, x + offset, y, w, h, StyleOptions() );
-            else if( d.isTroughUpper() ) Style::instance().renderSliderGroove( window, clipRect, x, y, w - offset, h, StyleOptions() );
+            } else {
+
+                // TODO: calculate this value from the style "slider-width" property
+                const int offset( 6 );
+                if( d.isTrough() ) Style::instance().renderSliderGroove( window, clipRect, x + offset, y, w - 2*offset, h, StyleOptions() );
+                else if( d.isTroughLower() ) Style::instance().renderSliderGroove( window, clipRect, x + offset, y, w, h, StyleOptions() );
+                else if( d.isTroughUpper() ) Style::instance().renderSliderGroove( window, clipRect, x, y, w - offset, h, StyleOptions() );
+
+            }
 
         } else if( d.isTrough() && shadow == GTK_SHADOW_IN ) {
Comment 9 Gökçen Eraslan 2011-01-19 12:22:14 UTC
Thank you, I love fast upstreams :)