/caja

GTK3: port GtkStyle->GtkStyleContext #559

Merged
merged 7 commits into from May 31, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@lukefromdc @raveit65
@lukefromdc
Member

lukefromdc commented May 28, 2016

Port over all the remaining instances of GtkStyle I could find in Caja, keep caja-sidebar-title.c from fixes6 branch. Details below:

caja-sidebar-title.c Same file from https://github.com/lukefromdc/caja/tree/gtk3.21fixes6 GTK3: Ports update_title_font to GtkStyleContext and limit style_set function to calling update_title_font in GTK3 builds as latter function interferes with resizing fonts and does little else in GTK2.

caja-places-sidebar.c: Port caja_places_sidebar_style_set to GtkStyleContext in GTK3 builds

caja-pathbar.c: GtkStyleContext is already used to style the pathbar, so removed GtkStyle variables and unneeded caja_path_bar_style_set function

Note that this disables the most common call to caja_path_bar_check_icon_theme but icons update fine without it in GTK3. That being so, remove it and the call to it in caja_path_bar_screen_changed as well, so two functions not needed in GTK3 are removed. Theme and icon updating no change observed in testing

caja-information-panel.c: Port caja_information_panel_style_set to GtkStyleContext in GTK3 builds

caja-history-sidebar.c: port caja_history_sidebar_style_set GtkStyleContext in GTK3 builds

caja-location-bar.c: Port one variable in style_set_handler to GtkStyleContext in GTK3 builds

caja-zoom-control.c: label_style_set_callback to GtkStyleContext in GTK3 builds

Note that *previous_style is still used, unknown if this is having any effect or validity in GtkStyleContext. Not sure how to remove it but in this case it may simply be a variable name and thus valid anyway.

There is still an issue with icons in the places sidebar except information that only update after a theme change on restarting Caja, this is not new, predates any of these changes,can be observed with caja from Master over GTK 3.20, and is not affected by these commits. No other theme issues in tests with Gtk 3.16, 3.18, 3.20, or 3.21

lukefromdc added some commits May 27, 2016

@lukefromdc
GTK3: Fix two deprecations, stop 3.21 segfault 
In GTK 3.21, the use of GtkStyle in function style_set in caja-sidebar-title.c results in a segfault if the sidebar is showing, even though it is used only when the "information" sidebar is selected. GtkStyle also appears in update_title_font  in the same file.

The second function resizes the bold headline label font in the information sidebar when either the length of the text or the width of the sidebar changes. Port it to GtkStyleContext and keep it. 

The first function (style_set) does two things: It invokes the second function when the style is set up, so port its second input variable (which seems to receive only NULL anyway) to GtkStyleContext. The rest of the function is supposed to set the font for the "more information" label, but mostly seems to block updating that font with the system font and cause the size of the font to be different between GTK 3.20 or earlier and GTK 3.21. Disable that portion entirely in GTK3 builds, as that way the font is consistant across GTK3 versions(an appropriate size in all cases tested), updates with changes in the system font, and cannot segfault in GTK 3.21. Porting it to GtkStyleContext has been tested and stops the segfaults but leaves the other two problems mentioned. Disable that portion of style_set in GTK3 and be done with it.
a6c32f4
@lukefromdc
GTK3: port history sidebar to GtkStyleContext 
caja-history-sidebar.c: port caja_history_sidebar_style_set GtkStyleContext in GTK3 builds
aff48d2
@lukefromdc
GTK3:caja-information-panel.c use GtkStyleContext 
caja-information-panel.c: Port caja_information_panel_style_set to GtkStyleContext in GTK3 builds
9f4ab08
@lukefromdc
GTK3: caja-location-bar.c use GtkStyleContext 
 Port one variable in style_set_handler to GtkStyleContext in GTK3 builds
74a1a7a
@lukefromdc
GTK3:caja-pathbar remove GtkStyle vars 
caja-pathbar.c: GtkStyleContext is already used to style the pathbar, so removed GtkStyle variables and unneeded caja_path_bar_style_set function
9e97cf8
@lukefromdc
GTK3: caja-places-sidebar.c use GtkStyleContext 
caja-places-sidebar.c: Port caja_places_sidebar_style_set to GtkStyleContext from GtkStyle in GTK3 builds
a540850
@lukefromdc
GTK3:caja-zoom-control.c GtkStyle>GtkStyleContext 
caja-zoom-control.c: label_style_set_callback to GtkStyleContext in GTK3 builds
469c6d8

This was referenced May 28, 2016

Closed
Closed
Closed
@raveit65
Member

raveit65 commented May 29, 2016

Whoow, thank you for this huge work.
I can confirm that caja crashes with 3.21.2 and that this PR fixes the issue 😄
But i found some build warnings, not sure if they're related to this changes, maybe they was there before.

caja-history-sidebar.c: In function 'caja_history_sidebar_class_init':
caja-history-sidebar.c:329:41: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]
     GTK_WIDGET_CLASS (class)->style_set = caja_history_sidebar_style_set;
                                         ^

caja-information-panel.c: In function 'caja_information_panel_class_init':
caja-information-panel.c:214:29: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]
     widget_class->style_set = caja_information_panel_style_set;
                             ^


caja-places-sidebar.c: In function 'caja_places_sidebar_class_init':
caja-places-sidebar.c:3397:41: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]
     GTK_WIDGET_CLASS (class)->style_set = caja_places_sidebar_style_set;
                                         ^

caja-sidebar-title.c: In function 'caja_sidebar_title_class_init':
caja-sidebar-title.c:255:29: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]
     widget_class->style_set = style_set;
                             ^

I will dig into it for myself, looks like this needs to be change to style_updated.
Again, thank you for this hard work.

@monsta
can you do a code review please?
@lukefromdc
Member

lukefromdc commented May 29, 2016

Possibly a GtkStyleContext variable is getting in input that is structurally a
GtkStyle format? I looked for inputs to these functions, generally finding
NULL or nothing going into the previous_style variables, which are what
I suspect. A test build with master will find if those build warnings were
already there.

I wish to hell I had tools to analyze code in one file and track what all variables
were doing

On 5/29/2016 at 7:46 AM, "NO NAME" notifications@github.com wrote:

Whoow, thank you for this huge work.
I can confirm that caja crashes with 3.21.2 and that this PR fixes
the issue 😄
But i found some build warnings, not sure if they're related to
this changes, maybe they was there before.

caja-history-sidebar.c: In function 
'caja_history_sidebar_class_init':
caja-history-sidebar.c:329:41: warning: assignment from 
incompatible pointer type [-Wincompatible-pointer-types]
    GTK_WIDGET_CLASS (class)->style_set = 
caja_history_sidebar_style_set;
                                        ^

caja-information-panel.c: In function 
'caja_information_panel_class_init':
caja-information-panel.c:214:29: warning: assignment from 
incompatible pointer type [-Wincompatible-pointer-types]
    widget_class->style_set = caja_information_panel_style_set;
                            ^


caja-places-sidebar.c: In function 
'caja_places_sidebar_class_init':
caja-places-sidebar.c:3397:41: warning: assignment from 
incompatible pointer type [-Wincompatible-pointer-types]
    GTK_WIDGET_CLASS (class)->style_set = 
caja_places_sidebar_style_set;
                                        ^

caja-sidebar-title.c: In function 'caja_sidebar_title_class_init':
caja-sidebar-title.c:255:29: warning: assignment from incompatible 
pointer type [-Wincompatible-pointer-types]
    widget_class->style_set = style_set;
                            ^

I will dig into it for myself, looks like this needs to be change
to style_updated.
Again, thank you for this hard work.

@monsta
can you do a code review please?

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#559 (comment)
222356438

@lukefromdc lukefromdc referenced this pull request May 30, 2016

Closed

GTK3: remove 4 deprecated functions #560

@lukefromdc
Member

lukefromdc commented May 30, 2016

#560 suppresses those 4 (new) build warnings. removing these four "style_set" functions from GTK3 builds had no effect on rendering or theme changes and is slightly smaller code. There is still an ugly mess in eel-background.c , where porting GtkStyle to GtkStyleContext also requires converting or porting GdkColor to GdkRGBA, possibly throughout the file. That needs to be fixed before another GTK update breaks it...
@raveit65
Member

raveit65 commented May 30, 2016

@lukefromdc
I got an enlightenment after i stumbled about this commit https://git.gnome.org/browse/gnome-terminal/commit/?h=gnome-3-8&id=86d5a67ffdc88ffdebb070622e67336875b95bc6
I ported style_set to style_updated on top of this PR and everything works well and i see no build warnings any more. See https://github.com/mate-desktop/caja/commits/lukefromdc-dev-gtkstylecontext
It is tested with gtk+-3.22.2/20.x/18.x/16.x/14.x
I'd would prefer not to drop the style-updated widget if you agree.
Please test.
@lukefromdc
Member

lukefromdc commented May 31, 2016

Fine with me. If yu've got it working go for it. I'm just trying to find the best way
to go about this and I am not wedded to any particular approach, thus both
PR's open while we decide how to deal with this.

On 5/30/2016 at 6:31 PM, "NO NAME" notifications@github.com wrote:

@lukefromdc
I got an enlightenment after i stumbled about this commit
https://git.gnome.org/browse/gnome-terminal/commit/?h=gnome-3-
8&id=86d5a67ffdc88ffdebb070622e67336875b95bc6
I ported style_set to style_updated on top of this PR and
everything works well and i see no build warnings any more. See
https://github.com/mate-desktop/caja/commits/lukefromdc-dev-
gtkstylecontext
It is tested with gtk+-3.22.2/20.x/18.x/16.x/14.x
I'd would prefer not to drop the style-updated widget if you agree.
Please test.

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#559 (comment)
222561623
@lukefromdc
Member

lukefromdc commented May 31, 2016

I just tested this with GTK 3.21 and it works fine

On 5/30/2016 at 6:31 PM, "NO NAME" notifications@github.com wrote:

@lukefromdc
I got an enlightenment after i stumbled about this commit
https://git.gnome.org/browse/gnome-terminal/commit/?h=gnome-3-
8&id=86d5a67ffdc88ffdebb070622e67336875b95bc6
I ported style_set to style_updated on top of this PR and
everything works well and i see no build warnings any more. See
https://github.com/mate-desktop/caja/commits/lukefromdc-dev-
gtkstylecontext
It is tested with gtk+-3.22.2/20.x/18.x/16.x/14.x
I'd would prefer not to drop the style-updated widget if you agree.
Please test.

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#559 (comment)
222561623

@raveit65 raveit65 merged commit 469c6d8 into mate-desktop:master May 31, 2016

@lukefromdc lukefromdc deleted the lukefromdc:dev-gtkstylecontext branch Jun 18, 2016

@lukefromdc lukefromdc referenced this pull request Sep 1, 2016

Closed

Debian Unstable: GTK 3.21.5+Caja 1.14.2 bring back desktop redraw failure and segfaults #627

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment