Re: [PATCH] GTK theme toolbar button rendering

Hi,

On Mon, May 28, 2012 at 11:08 AM, David Tardon <dtardon@redhat.com> wrote:
> On Sat, May 26, 2012 at 09:40:49PM +0400, Ruslan Kabatsayev wrote:
> > Hello,
> >
> > Current GTK theming plugin renders toolbar buttons as non-flat
> > unconditionally. But some themes, e.g. oxygen-gtk expect toolbuttons
> > to be flat, and the desktop integration fails.
> > I'm attaching a patch to fix this. The patch has been tested with
> > oxygen-gtk(current master), QtCurve-GTK, Glossy and Simple themes.
>
> Hi,

> > +    if(GTK_IS_TOGGLE_BUTTON(button))
> > +    {
> > +       if(gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(button)))
>
> This is equivalent to "if(nState & CTRL_STATE_PRESSED)", is it not?

It's not equivalent since this code checks if the button is
toggled-down (like e.g. Align Left button in Formatting toolbar).
CTRL_STATE_PRESSED, OTOH, means that the button is being pressed by
the mouse, and doesn't take toggled state into account.
This state of toggle button is set in NWPaintGTKToolbar() on
aValue.getTristateVal()==BUTTONVALUE_ON to make rendering correct.
Togglebuttons in toolbars have a bit more states, e.g. toggled and
prelit, toggled and not prelit, pressed, etc., so they should be
handled differently from usual press buttons.

> > +           shadowType=GTK_SHADOW_IN;
> > +       else
> > +           shadowType=GTK_SHADOW_OUT;
> > +
> > +       if(nState & CTRL_STATE_ROLLOVER)
> > +           stateType=GTK_STATE_PRELIGHT;
> > +       else
> > +           stateType=GTK_STATE_NORMAL;
> > +
> > +       if(nState & CTRL_STATE_PRESSED)
> > +       {
> > +           stateType=GTK_STATE_ACTIVE;
> > +           shadowType=GTK_SHADOW_IN;
> > +       }
> > +    }
>
> It seems to me that this code could be used in
> NWConvertVCLStateToGTKState (with the above modification), so there
> would be no need for toggle button-specific code branch. Or am I missing
> something?

If this is moved to NWConvertVCLStateToGTKState, a new argument (set
default to 0 for other callers) would be needed for this function,
namely, GtkWidget* button. Or, if we don't want to rely on toggle
button state, we'll have to pass ImplControlValue& aValue and do this
check there.
Not sure if this is really better... Do you think this still should be done?

> D.
> _______________________________________________
> LibreOffice mailing list
> LibreOffice@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice