Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

More changes towards getting ImageView running #10

Closed
wants to merge 3 commits into from
Closed

More changes towards getting ImageView running #10

wants to merge 3 commits into from

Conversation

timholy
Copy link
Member

@timholy timholy commented Dec 26, 2013

The most important of these---and the one that deserves the most careful scrutiny---is the patch adding connect. I added it because I was finding it awkward to pass extra data to callbacks. Usage of connect would take either of the following patterns:

connect(upbutton, "clicked", obj -> upbutton_cb(obj, state, displayfunc))
connect(upbutton, "button-press-event", (obj, event) -> upbutton_cb(obj, event, state, displayfunc))

The latter is used when there is a GdkEvent associated with the signal.

Overall, I think this is a fairly nice interface, but there are alternatives. Rather than using an anonymous function to store the extra arguments, we could presumably define it as

connect(object, signal, func, args...)

where func gets called as func(obj, event, args...), and set the args tuple as the data argument of g_signal_connect_data. However, one problem with that interface is how to handle gconnectflags. So the anonymous-function interface seemed the better way to go.

I should also mention that do find the current signal_connect slightly confusing:

  • The data argument is named closure.
  • The data argument is last, after gconnectflags, which is presumably a rarely-used option. That propagates to on_signal_button_press and friends, meaning that most usages of this function might look like this:
on_signal_button_press(func, widget, 0, extradata)

where extradata contains whatever state information the widget callback needs in order to do its job. If one could pass it as

on_signal_button_press(func, widget, 0, extradata1, extradata2, ...)

then that argument order might make more sense, but that doesn't work either unless signal_connect's function declaration were written in terms of closure....

I suspect I'm simply not understanding how you intended to use it, and perhaps that's part of why I decided to create a wrapper.

The other patches in this PR (so far) should be fairly straightforward, as long as you like the names. The key constants are a bit awkward to use in practice,

if event.keyval == Gtk.GdkKeySyms.GDK_KEY_Up
    # do something
end

which is quite a lot of characters to type in front of the key constant. Though naturally, one can say

const KSYM = Gtk.GdkKeySyms

and shorten it. (Seemingly, one can't say using for a baremodule.)

GdkEventKey does not contain the current pointer location, so this is
needed for handling keypress events whose action depends on the
current pointer position.
This also
- fixes a library-specification problem in some signal-handling functions
- adds tests on signal callbacks
const _gpy = Cint[-1]
const _gpmask = Cint[-1]
function get_pointer(w::GtkWidgetI)
ccall((:gdk_window_get_pointer,libgtk), Ptr{Void},
Copy link
Contributor

Choose a reason for hiding this comment

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

this function is deprecated in Gtk-3

Copy link
Member Author

Choose a reason for hiding this comment

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

I noticed that. Does that mean it's available until gtk4? Is there a horizon for that?

If we want to avoid using this, it's 4 ccalls rather than 1. That's no big deal, of course, and will change it if you think we should.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since deprecated means "do not use in new code", I would rather not provide it to be used at all. All get_ (and set_) methods should be in GAccessor, without the get prefix, since they can be auto-generated.

Is GdkWindow useful from a client level? I can just add it to the GAccessor list of filter objects.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since deprecated means "do not use in new code", I would rather not provide it to be used at all.

Fair enough.

Re GAccessor, I haven't dug into this yet. The only use I have so far for GdkWindow is getting the pointer position. But of course time may encourage me to add more. I can imagine wanting to hide or unhide window in response to a key binding, and a destroyall---not sure yet what those would require.

Copy link
Contributor

Choose a reason for hiding this comment

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

Those operations would be done at the GtkWidget level

Copy link
Member Author

Choose a reason for hiding this comment

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

I might have to ask you to tackle get_pointer, since I'm not sure I understand how you want to implement this. My use case is the following: Ctrl-Scroll in ImageView zooms in & out, centered on the current mouse position. To support people who don't have wheel mice, I currently map Ctrl-Up and Ctrl-Down to the same zoom operation. Since an EventKey does not contain the pointer position, I need a mechanism to extract the current position of the pointer in a window.

Don't worry about setting up a merge conflict for me, just go ahead and do it and I'll deal.

@timholy
Copy link
Member Author

timholy commented Dec 26, 2013

I'll close this in favor of #12, since the other two items require more extensive direction from @vtjnash about the right solution.

FYI I view the get_pointer issue as minor. In contraast, the proper way to specify callbacks is clearly a pretty major design decision. I will simply halt any further attempts at porting ImageView until this is resolved.

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.

2 participants