Thread: [Java-gnome-developer] OptionMenu events etc..
Brought to you by:
afcowie
From: Jonas B. <jb...@ni...> - 2003-10-02 11:42:43
|
Hi.. I was adding a OptionMenu, but I noticed no event listener interface was=20 available for the class. I looked at the superclass Button and saw it was= =20 mostly java code required, but that there were some native stuff involved= =20 which seems to be autogenerated. So, I was thinking, could someone maybe in short describe - the basics about adding event listener support for a widget - the pitfalls - what is autogenerated and whatnot - etc I could then try to implement the stuff needed in OptionMenu (and maybe=20 send a patch).. and maybe later other components as well. - xkr47 |
From: Mark H. <mh...@de...> - 2003-10-02 15:02:30
|
Hi, Before you do anything, please add a bug entry for this to sourceforge.net - we seem to have lots of little unfinished bits like this, but aren't tracking them very well. Also, add reports for anything else you find :) On Thu, Oct 02, 2003 at 02:41:42PM +0300, Jonas Berlin wrote: > - what is autogenerated and whatnot Everyting was initially autogenerated. In the latest automatic generation, some jni/c code was autogenerated and java files were generated containing the native methods. Everything else has been added by hand. This is basically almost everything in the java files apart from the bottom few lines and either nothing in jni files, or a few functions at the top of the files, or in some cases entire jni files. Adding an event for an option menu will probably be possible without changing any c code. (We try to do as little c work as possible since it is far easier to debug java and it usually works better that way) Three things are needed for an event: events/eventnameEvent.java e.g. ButtonEvent Each event has a static nested Type class. This allows us to have varients of an event handled by the same methods. This should extend GtkEventType and the static final fields of it have a unique identifier and a name. This part should be easy to construct. (Option menu will probably only have one type - CHANGE) The rest of the class can be basically copied events/eventnameListener.java This is the listener which the user's code must implement. Should be kept as simple as possible. We tend to prefer a single method for a group of related events rather than lots of methods which probably won't all get implemented. Any objections to what events go in which methods can be discussed in the mailing lists. The widget. Take Button.java as an example: We can have multiple objects listening for an event, so need: private Vector buttonListeners = null; We then have methods for adding and removing events: public void addListener(ButtonListener listener) { public void removeListener(ButtonListener listener) { We then have a convenience function for calling the methods of the objects which are listening: protected void fireButtonEvent(ButtonEvent event) { Everything so far can be more or less copied (changing the Event and listener class names. The next stage is to actually receive notification of the events from the native layer. This is done through the addEvents method. The jni code is done in other parts of java-gnome, so for things like Buttons and OptionMenus, it's just a case of adding: protected static void addEvents(EventMap anEvtMap) { Widget.addEvents(anEvtMap); anEvtMap.addEvent("clicked", "handleClick", ButtonEvent.Type.CLICK, ButtonListener.class); activate is the name of the gtk signal. handleActivate is a method in this class: private void handleClick() { fireButtonEvent(new ButtonEvent(this, ButtonEvent.Type.CLICK)); } Copying code like this should be fairly straight forward (for simple widgets at least). We will of course review any patches to look out for problems. > I could then try to implement the stuff needed in OptionMenu (and maybe > send a patch).. and maybe later other components as well. I look forward to seeing it. Note that many of the signals have not been implemented for a good reason; we are trying not to copy the deprecated or internal parts of gtk (option menu changing does not come into this category). -- .''`. Mark Howard : :' : `. `' http://www.tildemh.com `- mh...@de... | mh...@ti... | mh...@ca... |
From: Jonas B. <jb...@ni...> - 2003-10-03 20:47:37
|
On Thu, 2 Oct 2003 13:18:32 +0100, Mark Howard <mh...@de...> wrote: > The next stage is to actually receive notification of the events from > the native layer. This is done through the addEvents method. The jni > code is done in other parts of java-gnome, so for things like Buttons > and OptionMenus, it's just a case of adding: > protected static void addEvents(EventMap anEvtMap) { > Widget.addEvents(anEvtMap); > anEvtMap.addEvent("clicked", "handleClick", ButtonEvent.Type.CLICK,=20 > ButtonListener.class); Hi.. I took a look at some of the classes and thought they had some weird= =20 initialization style: public class ToggleButton extends Button { protected void initializeEventHandlers() { super.initializeEventHandlers(); evtMap.initialize(this); } protected static void addEvents(EventMap evtMap) { Button.addEvents(evtMap); // .. add togglebutton-specific events } } public class Button extends Bin { protected void initializeEventHandlers() { super.initializeEventHandlers(); evtMap.initialize(this); } protected static void addEvents(EventMap evtMap) { Widget.addEvents(evtMap); // .. add button-specific events } } public class Bin extends Container { // no initializeEventHandlers(), so it uses the one from Container protected static void addEvents(EventMap evtMap) { Container.addEvents(evtMap); } } public class Container extends Widget { protected void initializeEventHandlers() { evtMap.initialize(this); } protected static void addEvents(EventMap evtMap) { Widget.addEvents(evtMap); // .. add container-specific events } } public class HBox extends Box { // no initializeEventHandlers(), so it uses the one from Container protected static void addEvents(EventMap evtMap) { Box.addEvents(evtMap); } } public class Box extends Container { // no initializeEventHandlers(), so it uses the one from Container protected static void addEvents(EventMap evtMap) { Container.addEvents(evtMap); } } Now first off, ToggleButton.addEvents() calls Button.addEvents() i.e. the= =20 superclass' addEvents(). But Button.addEvents() call Widget.addEvents()=20 while its superclass is Bin. Bin again calls its superclass' addEvents(),= =20 and so does Container. As a first impression, one would think the=20 Widget.addEvents() in the Button class should be Bin.addEvents(). On the other hand, one can see that the initializeEventHandlers() always=20 also calls initializeEventHandlers() for it's superclass. And since all=20 implementations of initializeEventHandlers() call their superclasses'=20 initializeEventHandlers(), one would think that calling for example=20 Container.addEvents() from Bin.addEvents() wouldn't be necessary. If I understood the code correctly however, this will just result in some= =20 signals being registered twice. I didn't have time to figure out whether=20 there are some checks to avoid duplicate registration. But anyway, becaus= e=20 of the subclass calling the superclass, all events seem to get registered= =20 anyway. As far as I understood, all calls to the parent's addEvents() method coul= d=20 be removed, since the super.initializeEventHandlers() will call the=20 initializatieEventHandlers() for its superclasses as well, eventually=20 initializing all superclasses' signals for the object in question. Now if I got something wrong, please correct me. If you give guidelines=20 for how these really should be, I can see if I could create a patch for=20 the javas too.. Anyway I'll try to implement the OptionMenu events according to=20 ToggleButton's example and see what I can do.. - xkr47 |
From: Jonas B. <jb...@ni...> - 2003-10-04 23:04:31
|
On Fri, 03 Oct 2003 23:47:19 +0300, Jonas Berlin <jb...@ni...>= =20 wrote: > Hi.. I took a look at some of the classes and thought they had some=20 > weird initialization style: > > public class ToggleButton extends Button { > protected void initializeEventHandlers() { > super.initializeEventHandlers(); > evtMap.initialize(this); > } > > protected static void addEvents(EventMap evtMap) { > Button.addEvents(evtMap); > // .. add togglebutton-specific events > } > } I did some research, a bunch debug prints in the signal-related codes to=20 test & verify the behaviour. I found out that my previous statements were at least partially correct. The situation is like this: 1. initializeEventHandlers() methods call super.initializeEventHandlers= ()=20 (except for some cases) 2. most addEvents() methods call addEvents() of some superclass, usuall= y=20 <superclass>.addEvents() or then Widget.addEvents. The result from this duplicate behaviour is that some event handlers get=20 registered multiple times. One easy way to test this is to do: LifeCycleListener lcl =3D new LifeCycleListener() { public void lifeCycleEvent(LifeCycleEvent evt) { System.out.println(evt); } }; Button b =3D new Button("foo"); b.addListener(lcl); b.show(); This will display something like: org.gnu.gtk.event.LifeCycleEvent[source=3Dorg.gnu.gtk.Button@18a992f,id=3D= SHOW] org.gnu.gtk.event.LifeCycleEvent[source=3Dorg.gnu.gtk.Button@18a992f,id=3D= SHOW] Now if you change Button to ToggleButton, it instead shows: org.gnu.gtk.event.LifeCycleEvent[source=3Dorg.gnu.gtk.ToggleButton@f72617= ,id=3DSHOW] org.gnu.gtk.event.LifeCycleEvent[source=3Dorg.gnu.gtk.ToggleButton@f72617= ,id=3DSHOW] org.gnu.gtk.event.LifeCycleEvent[source=3Dorg.gnu.gtk.ToggleButton@f72617= ,id=3DSHOW] ----------- Now I suggest the following: 1. make sure all initializeEventHandlers() call=20 super.initializeEventHandlers(), with the only exception being Widget,=20 which is the topmost widget class. 2. remove all <anywidgetclass>.addEvents() calls from the addEvents()=20 methods of the various classes. This way, events should be registered only once, and as a bonus, the code= =20 will most likely be easier to read also =3D) I'll start working on a patch that will fix all widget classes according=20 to my suggestion. If you have some better solution for this then I can tr= y=20 that as well :) - xkr47 |
From: Jonas B. <jb...@ni...> - 2003-10-05 09:17:38
|
On Sun, 05 Oct 2003 02:04:19 +0300, Jonas Berlin <jb...@ni...>= =20 wrote: > Now I suggest the following: > > 1. make sure all initializeEventHandlers() call=20 > super.initializeEventHandlers(), with the only exception being Widget,=20 > which is the topmost widget class. > 2. remove all <anywidgetclass>.addEvents() calls from the addEvents()=20 > methods of the various classes. > > This way, events should be registered only once, and as a bonus, the=20 > code will most likely be easier to read also =3D) > > I'll start working on a patch that will fix all widget classes accordin= g=20 > to my suggestion. If you have some better solution for this then I can=20 > try that as well :) Now I'm back a bit later with a patch according to the above suggestions,= =20 and additionally: 3. make the addEvents() methods private so nobody else can call them by=20 accident I tested the patch with the same Button & ToggleButton code I presented i= n=20 my last mail, and both now yield only one LifeCycleEvent. I didn't test=20 the other classes, but it's pretty simple a modification and I tried to=20 make sure I fixed them all the same way. Now then, the patch is here: http://xkr47.outerspace.dyndns.org/patches/java-gnome/cvs-031005-2-ini= tializeEvent-addEvents.diff The sourceforge cvs was inoperational, or at least I could not log in to=20 it and check out the project, so I did a dirty perl script that downloade= d=20 the entire java-gnome cvs through the cvsweb interface, which was still=20 operational. Thus, the patch is against the cvs version that was present=20 in sourceforge yesterday evening. Some final notes about the patch: - some classes had addEvents() methods that only called=20 <superclass>.addEvents(), which after my modification (2.) became empty,=20 so I removed those methods (and unnecessary EventMap imports that became=20 unnecessary) altogether. - some classes didn't have an addEvents() method at all, instead they d= id=20 what addEvents() used to do directly in the static { ... } block. I=20 thought consistency is bliss, created new addEvents() methods and moved=20 the code there. - xkr47 |
From: Mark H. <mh...@ca...> - 2003-10-07 16:27:05
|
On Sun, Oct 05, 2003 at 12:16:37PM +0300, Jonas Berlin wrote: > http://xkr47.outerspace.dyndns.org/patches/java-gnome/cvs-031005-2-initializeEvent-addEvents.diff Thanks. This looks good. I've tested on my large app and everything seems to work fine. Did you go through every single widget? Or is there more still to be done. Also, did you take a look at the gnome widgets? > The sourceforge cvs was inoperational, or at least I could not log in to > in sourceforge yesterday evening. Were you using anonymous cvs? The developer one where you have to login requires special cvs privilidges. In any case, sourceforge is generally rubbish at cvs; trying again a few minutes later often helps. > - some classes had addEvents() methods that only called > <superclass>.addEvents(), which after my modification (2.) became empty, > so I removed those methods (and unnecessary EventMap imports that became > unnecessary) altogether. I'd have prefered to eave them with a comment saying they do anything. This shows that you've actually thought about it, rather than it being mistakenly missed out. Not important though. Leave things as they are now. > - some classes didn't have an addEvents() method at all, instead they did > what addEvents() used to do directly in the static { ... } block. I > thought consistency is bliss, created new addEvents() methods and moved > the code there. I can't remember exactly when add events was implemented. I think Tom did it all when working on the glade support. Probably just missed a few classes. -- .''`. Mark Howard : :' : `. `' http://www.tildemh.com `- mh...@de... | mh...@ti... | mh...@ca... |
From: Jonas B. <jb...@ni...> - 2003-10-05 10:14:10
|
On Thu, 2 Oct 2003 13:18:32 +0100, Mark Howard <mh...@de...> wrote: > Hi, > Before you do anything, please add a bug entry for this to > sourceforge.net - we seem to have lots of little unfinished bits like > this, but aren't tracking them very well. Also, add reports for anythin= g > else you find :) Ok did that, and will do in the future =3D) > Copying code like this should be fairly straight forward (for simple > widgets at least). We will of course review any patches to look out for > problems. Yes, it was pretty simple.. And I felt confident as soon as I figured out= =20 that event initialization hassle I posted a patch to just an hour ago.. =3D= ) >> I could then try to implement the stuff needed in OptionMenu (and mayb= e >> send a patch).. and maybe later other components as well. > I look forward to seeing it. > Note that many of the signals have not been implemented for a good > reason; we are trying not to copy the deprecated or internal parts of > gtk (option menu changing does not come into this category). Well, I noticed later there is this other option to register an event=20 listener for each of the items in the pull-down menu (like can be done in= =20 C as well), but I think it is a bit overkill for simple setups, and thus = I=20 think implementing the "changed" event is motivated. I named the event "CHANGE" in the java code, along the lines of (what I=20 thought was) the common practice. The patch is here: http://xkr47.outerspace.dyndns.org/patches/java-gnome/cvs-031005-1-Opt= ionMenuEvent.diff I tried to make the code in the patch look the same way as other widgets=20 were implemented in the cvs checkout I did last night. This patch does no= t=20 overlap with the patch I sent an hour ago regarding event registration=20 fixes. I continued following the same event registration model in this=20 patch. I tested it and it worked beautifully. Thanks for posting the extensive=20 advice on how to implement event handlers. - xkr47 |
From: Mark H. <mh...@ca...> - 2003-10-07 16:27:10
|
On Sun, Oct 05, 2003 at 01:14:07PM +0300, Jonas Berlin wrote: > http://xkr47.outerspace.dyndns.org/patches/java-gnome/cvs-031005-1-OptionMenuEvent.diff Looks good apart from one mistake: + public boolean isOfType(OptionMenuEvent.Type type){ + return (type.getID() == type.getID()); + } My quick search shows that the other classes look ok. If you copied this code, could you please check this. I've fixed and applied to cvs. Fell free to write more patches :) (BTW: I also applied the previous patch; forget to say that in the reply email). -- .''`. Mark Howard : :' : `. `' http://www.tildemh.com `- mh...@de... | mh...@ti... | mh...@ca... |
From: Jonas B. <jb...@ni...> - 2003-10-07 22:12:42
|
On Tue, 7 Oct 2003 17:27:04 +0100, Mark Howard <mh...@ca...> wrote: > Looks good apart from one mistake: > + public boolean isOfType(OptionMenuEvent.Type type){ > + return (type.getID() =3D=3D type.getID()); > + } Oh gosh.. I originally worked on the 0.8.0 source and then switched over=20 to making the patches compatible with cvs.. in 0.8.0 the code was=20 this.type.getID() =3D=3D type.getID() and in cvs, at first glance, it loo= ked=20 like the "this" had just been removed (didn't really look into what it wa= s=20 doing, even if it was that simple).. So I removed the "this" in front and= =20 though "great, now it looks like the others", but then again, not.. Very=20 nice catch there, sorry that it slipped.. I'll try to be more careful in=20 the future. > My quick search shows that the other classes look ok. If you copied thi= s > code, could you please check this. I checked the other classes (while pondering on how that could have=20 slipped) and they seemed ok.. Some classes had slightly differing naming=20 convetions there (calling the method argument test instead of aType), but= =20 otherwise the same. > I've fixed and applied to cvs. Cool =3D) > Fell free to write more patches :) I do. =3D) And some test code also.. I put a textview test already there,= =20 looks like a tricky one (unless my test is faulty somehow).. But I can=20 make the java process grow just as well by "just" creating 100000 (or som= e=20 insane number) labels and then letting the garbage collector pick them=20 up.. Even if I call Widget.destroy() on them, the native code seems to=20 keep memory references somewhere.. I haven't tested any other components=20 yet, or even looked at how the objects are handled in the native code par= t=20 of java-gnome.. Generally I don't know how to handle data allocated by native code when=20 the object gets garbage collected.. is the only way to implement a native= =20 void finalize() method that is then called when the object is garbage=20 collected, or is there a better way? I have heard implementing finalize()= =20 methods in classes slows down execution, so hopefully there's some other=20 way to do it without the performance degradation.. Do you in general have plans to write some unit testing or similar on the= =20 components? JUnit could be one way, but with java-gnome, it's quite easy=20 to pull together even the simplest test application, in fact so easy that= =20 i find myself rather writing a gui for it than some text output =3D) > (BTW: I also applied the previous patch; forget to say that in the repl= y > email). Nice nice.. Your comment regarding putting an comment there (and maybe a=20 commented out method body for easier grepping).. I agree, that would have= =20 been better. - xkr47 |
From: Mark H. <mh...@ca...> - 2003-10-08 09:02:08
|
On Wed, Oct 08, 2003 at 12:43:34AM +0300, Jonas Berlin wrote: > I do. =) And some test code also.. I put a textview test already there, > looks like a tricky one (unless my test is faulty somehow).. But I can > make the java process grow just as well by "just" creating 100000 (or some > insane number) labels and then letting the garbage collector pick them > up.. Even if I call Widget.destroy() on them, the native code seems to > keep memory references somewhere.. I haven't tested any other components > yet, or even looked at how the objects are handled in the native code part > of java-gnome.. I haven't looked in detail, but I think the problems are most likely caused by us not clearing up strings after passing them to gtk, rather than not destroying the widgets. Gtk/Glib should destroy them when they are no longer used (unless there has been a g_object_ref call). There is still probably work to do in this area. My main focus so far has been to get things working, then work on improving them. Performance and memory usage haven't been an issue in the large apps I've written so far, so I was hoping there weren't any major problems. > Generally I don't know how to handle data allocated by native code when > the object gets garbage collected.. is the only way to implement a native > void finalize() method that is then called when the object is garbage > collected, or is there a better way? I have heard implementing finalize() > methods in classes slows down execution, so hopefully there's some other > way to do it without the performance degradation.. We don't want to destroy native objects when java objects get GCd. Glib will sort these out at the right time. It is common to create an object, add it to a form and then remove all java references to it - the native widget must still exist. > Do you in general have plans to write some unit testing or similar on the > components? JUnit could be one way, but with java-gnome, it's quite easy > to pull together even the simplest test application, in fact so easy that > i find myself rather writing a gui for it than some text output =) See the tests directory in cvs. I think Jeff started work on it. Again, priorities have been to work on implementing features and fixing visible bugs rather than work on this, although I agree it is important. > Nice nice.. Your comment regarding putting an comment there (and maybe a > commented out method body for easier grepping).. I agree, that would have > been better. I think one of the items in Effective Java (a useful book) is about this. -- .''`. Mark Howard : :' : `. `' http://www.tildemh.com `- mh...@de... | mh...@ti... | mh...@ca... |