Skip to content
This repository has been archived by the owner on Jun 8, 2021. It is now read-only.

Issue #862 - Add size_allocate vfunc to gtk::Widget #961

Merged
merged 1 commit into from Feb 26, 2020

Conversation

hfiguiere
Copy link
Contributor

Replaces PR #959 (on master this time)

@@ -209,6 +210,10 @@ pub trait WidgetImpl: WidgetImplExt + ObjectImpl + 'static {
fn get_preferred_height_for_width(&self, widget: &Widget, width: i32) -> (i32, i32) {
self.parent_get_preferred_height_for_width(widget, width)
}

fn size_allocate(&self, widget: &Widget, allocation: &Allocation) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function gtk_widget_size_allocate() says in the docs:

In this function, the allocation may be adjusted. It will be forced to a 1x1 minimum size, and the adjust_size_allocation virtual method on the child will be used to adjust the allocation. Standard adjustments include removing the widget’s margins, and applying the widget’s Widget:halign and Widget:valign properties.

So maybe this needs to be mutable after all and subclasses need to be able to modify it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I have no idea how this function is supposed to work, I'm just guessing from the docs :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my initial guess (see previous PR).

Looking at https://gitlab.gnome.org/GNOME/gtk/blob/gtk-3-24/gtk/gtkwidget.c#L6443
The GtkWidget vfunc doesn't modify it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does any of the subclasses? Maybe worth asking on #gtk :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can see they don't. At best they call gtk_widget_set_allocation or the parent class size allocate vfunc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be useful to get a definite answer from a GTK developer :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ebassi maybe? :)

let instance = &*(ptr as *mut T::Instance);
let imp = instance.get_impl();
let wrap: Widget = from_glib_borrow(ptr);
let allocate: Allocation = from_glib_none(allocation);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the allocation should be possible to modify then you need to write it back afterwards.

Copy link

@ebassi ebassi Feb 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The allocation is not an in-out parameter; you're supposed to call gtk_widge_set_allocation() or chain up to the parent class's allocation in your widget implementation, in which case you can tweak the argument for those.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, so the allocation should actually be an immutable reference. And new allocations could be passed to childs, etc.

@sdroege
Copy link
Member

sdroege commented Feb 22, 2020

So as part of this WidgetExt::size_allocate() should also be changed to not take a mutable reference.

@EPashkin
Copy link
Member

@ebassi Thanks for the clarification.
@hfiguiere Thanks for PR
I restarted failed job in CI
👍

@hfiguiere
Copy link
Contributor Author

So as part of this WidgetExt::size_allocate() should also be changed to not take a mutable reference.

This is auto generated.

@sdroege
Copy link
Member

sdroege commented Feb 22, 2020

That's true, but it can be fixed like here

gtk/Gir.toml

Lines 680 to 684 in 2fcc486

[[object.function]]
name = "apply_attributes"
[[object.function.parameter]]
name = "iter"
const = true
:)

@hfiguiere
Copy link
Contributor Author

OK. Added it as a separate commit as this one we might want to skip cherry-picking to to 0.8

@sdroege
Copy link
Member

sdroege commented Feb 22, 2020

OK. Added it as a separate commit as this one we might want to skip cherry-picking to to 0.8

Both can be backported :) Changing from &mut to & is not a breaking change.

unsafe {
let data = self.get_type_data();
let parent_class = data.as_ref().get_parent_class() as *mut gtk_sys::GtkWidgetClass;
let f = (*parent_class).size_allocate.unwrap();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let f = (*parent_class).size_allocate.unwrap();
let f = (*parent_class).size_allocate.expect("No parent class impl for \"size_allocate\"");

@sdroege
Copy link
Member

sdroege commented Feb 22, 2020

Otherwise looks good to me, thanks!

@sdroege
Copy link
Member

sdroege commented Feb 26, 2020

@GuillaumeGomez please merge :)

@GuillaumeGomez
Copy link
Member

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit 1e26d0a into gtk-rs:master Feb 26, 2020
@hfiguiere hfiguiere deleted the vfuncs branch March 16, 2020 22:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants