Re: [osg-users] GUIEventHandler called twice - possible bug(s)

[Robert Osfield]

Hi Riccardo,

On 10 October 2016 at 12:17, Riccardo Corsi  wrote:
> both double call and crash fixed on my side as well.

Good to hear things are now resolved.

> Now that one can safely implement a callback by deriving directly from
> osg::Callback, I think most users should go that way for their new code.
> If that's the case, the not-very-intuitive path of implementing operator()
> rather than run() in the NodeCallback should be a minor issue.

Yep, have to agree it's a bit convoluted having the old API's around.

> Maybe a note in the comments suggesting to derive from osg::Callback rather
> than the legacy classes could be worth it?

The subclasses still have an advantage that they often adapt the
parameters to something easier to work with in the context of the
subclass. Using osg::Callback can require a bit more legwork to cast
types.

As a general rule, subclassing from osg::Callback is what I'd prefer.

Robert.
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Re: [osg-users] GUIEventHandler called twice - possible bug(s)

[Riccardo Corsi]

Hi Robert,

both double call and crash fixed on my side as well.

Now that one can safely implement a callback by deriving directly from
osg::Callback, I think most users should go that way for their new code.
If that's the case, the not-very-intuitive path of implementing operator()
rather than run() in the NodeCallback should be a minor issue.

Maybe a note in the comments suggesting to derive from osg::Callback rather
than the legacy classes could be worth it?

Thank you,
Riccardo






On Thu, Oct 6, 2016 at 7:30 PM, Robert Osfield 
wrote:

> Hi Riccardo,
>
> I have found the cause of the double call, fixed it and have now
> checked in this fix and the other fixes to event callback handling.
> Changes checked into OSG git master.
>
> Could you test them out on your application to make sure that things
> are now behaving themselves.
>
> Thanks,
> Robert
> ___
> osg-users mailing list
> osg-users@lists.openscenegraph.org
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
>
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Re: [osg-users] GUIEventHandler called twice - possible bug(s)

[Robert Osfield]

Hi Riccardo,

I have found the cause of the double call, fixed it and have now
checked in this fix and the other fixes to event callback handling.
Changes checked into OSG git master.

Could you test them out on your application to make sure that things
are now behaving themselves.

Thanks,
Robert
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Re: [osg-users] GUIEventHandler called twice - possible bug(s)

[Robert Osfield]

HI Riccardo,

I have been back tackling the outstanding OS work today and have a
chance to build and test your callback test. I can recreate the issues
with the crash and double calling.

So far I've fixed a bug in the osgGA/EventVisitor that was causing the
crash and have replaced a couple of dynamic_cast<> in EventHandler
with more efficient as*() calls, and currently investigating the
double calling issue.  I hope to get a fix for this soon, once I do
I'll check in these fixes/refinements to the event callback handling.

FYI, the lack of the bool being passed back by the callback methods
has different meaning depending upon the context.  An event handler
that is handling a single event it returns true if that event is
handled, then the calling code should then set the EventHandled to
true on that event.  For a method handling all the events assigned to
an EventVisitor it will need to do this task itself, and it's return
value is not intended to be used in this way.

The duality is an unfortunate consequence of retaining backwards
compatibility as the various underlying OSG classes have evolved to
become more flexible.  To fix it would require breaking lots of OSG
user code so I've taken the decision to do what I can to offer the new
functionality without breaking backwards compatibility and just have
had to accept it's not as clean as it would have been if we have
started from scratch with the callback API.

Robert.
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Re: [osg-users] GUIEventHandler called twice - possible bug(s)

[Robert Osfield]

Hi Riccardo,

On 5 October 2016 at 13:51, Riccardo Corsi  wrote:
> I know there are many things already going on toward OSG 3.6.0,
> but if you get a change to take a look to the sample I've sent, it might be
> worth considering possible fixes before the new release is out.

I've been busy with client work of late so haven't had a chance to
look at your test example yet.

Once I'm back doing more work in prep for 3.6, if I don't get back to
this thread please feel free to raise the topic again.

Robert.
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Re: [osg-users] GUIEventHandler called twice - possible bug(s)

[Robert Osfield]

Thanks Riccardo.  I'm currently submerged in refactoring osgParticle,
once this is completed I'll have a look at the modified
osgcallback.cpp.

On 29 August 2016 at 16:20, Riccardo Corsi  wrote:
> Hi Robert,
>
> please find attached a simple example that shows the issues I was trying to
> explain:
> - launch with no args to see the initial issue
> - launch with "--crash" to get the crash i ran into when implementing the
> callback deriving from osg::Callback
> - launch with "--fix" to see the way I fixed it with the NodeCallback:
> comments on the run() and operator() methods explain some concerns for this
> solution
>
> Hope this clarifies things :)
> Ricky
>
>
> On Mon, Aug 29, 2016 at 4:29 PM, Robert Osfield 
> wrote:
>>
>> Hi Ricky,
>>
>> The changes to Drawable and the knock on effect to the callbacks is an
>> awkward one - it would be easy if it we could just discard backwards
>> compatibility, so it's a far from ideal code, but alas you can't
>> rewrite history once it's happened.
>>
>> From your description I don't have a clear idea of what is being
>> called twice and whether this is a problem.  Could you create a small
>> example that illustrates the problem, once I can see first hand I can
>> review the behaviour and decide whether this is acceptable or is an
>> bug.
>>
>> Thanks,
>> Robert.
>>
>> On 29 August 2016 at 15:00, Riccardo Corsi 
>> wrote:
>> > Hi all,
>> >
>> > I report what I believe could be considered a bug, probably introduced
>> > after
>> > the promotion of the Drawable class to a node, together with the new
>> > callback system.
>> >
>> > I have a GUIEventHandler derived class installed on a Drawable.
>> > After some digging I've realized that it's called twice for every event
>> > due
>> > to the EventVisitor code here:
>> >
>> > https://github.com/openscenegraph/OpenSceneGraph/blob/master/include/osgGA/EventVisitor#L86
>> >
>> > GUIEventHandler happens to derive both from NodeCallback and
>> > DrawableEventCallback, resulting in it being called twice. I think this
>> > might be considered as a bug if the intended behavior was to keep using
>> > the
>> > old GUIEventHandler interface the way it was (when attached to the
>> > viewer
>> > for instance, it is called only once as expected).
>> >
>> > Relate issue:
>> > I've re-implemented my callback deriving from osg::Callback, but this
>> > results in a crash as the osg::CallbackObject interface is used even if
>> > the
>> > cast does not succeed (first if condition)
>> >
>> > https://github.com/openscenegraph/OpenSceneGraph/blob/master/include/osgGA/EventVisitor#L93
>> >
>> > I fixed that by deriving from osg::NodeCallback, but I had to
>> > reimplement
>> > the operator() method instead of run(), as the second is not called by
>> > the
>> > visitor - that also it's a bit counter-intuitive with respect to the
>> > osg::Callback class interface.
>> >
>> > Ricky
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> > ___
>> > osg-users mailing list
>> > osg-users@lists.openscenegraph.org
>> >
>> > http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
>> >
>> ___
>> osg-users mailing list
>> osg-users@lists.openscenegraph.org
>> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
>
>
>
> ___
> osg-users mailing list
> osg-users@lists.openscenegraph.org
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
>
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Re: [osg-users] GUIEventHandler called twice - possible bug(s)

[Riccardo Corsi]

Hi Robert,

please find attached a simple example that shows the issues I was trying to
explain:
- launch with no args to see the initial issue
- launch with "--crash" to get the crash i ran into when implementing the
callback deriving from osg::Callback
- launch with "--fix" to see the way I fixed it with the NodeCallback:
comments on the run() and operator() methods explain some concerns for this
solution

Hope this clarifies things :)
Ricky


On Mon, Aug 29, 2016 at 4:29 PM, Robert Osfield 
wrote:

> Hi Ricky,
>
> The changes to Drawable and the knock on effect to the callbacks is an
> awkward one - it would be easy if it we could just discard backwards
> compatibility, so it's a far from ideal code, but alas you can't
> rewrite history once it's happened.
>
> From your description I don't have a clear idea of what is being
> called twice and whether this is a problem.  Could you create a small
> example that illustrates the problem, once I can see first hand I can
> review the behaviour and decide whether this is acceptable or is an
> bug.
>
> Thanks,
> Robert.
>
> On 29 August 2016 at 15:00, Riccardo Corsi 
> wrote:
> > Hi all,
> >
> > I report what I believe could be considered a bug, probably introduced
> after
> > the promotion of the Drawable class to a node, together with the new
> > callback system.
> >
> > I have a GUIEventHandler derived class installed on a Drawable.
> > After some digging I've realized that it's called twice for every event
> due
> > to the EventVisitor code here:
> > https://github.com/openscenegraph/OpenSceneGraph/
> blob/master/include/osgGA/EventVisitor#L86
> >
> > GUIEventHandler happens to derive both from NodeCallback and
> > DrawableEventCallback, resulting in it being called twice. I think this
> > might be considered as a bug if the intended behavior was to keep using
> the
> > old GUIEventHandler interface the way it was (when attached to the viewer
> > for instance, it is called only once as expected).
> >
> > Relate issue:
> > I've re-implemented my callback deriving from osg::Callback, but this
> > results in a crash as the osg::CallbackObject interface is used even if
> the
> > cast does not succeed (first if condition)
> > https://github.com/openscenegraph/OpenSceneGraph/
> blob/master/include/osgGA/EventVisitor#L93
> >
> > I fixed that by deriving from osg::NodeCallback, but I had to reimplement
> > the operator() method instead of run(), as the second is not called by
> the
> > visitor - that also it's a bit counter-intuitive with respect to the
> > osg::Callback class interface.
> >
> > Ricky
> >
> >
> >
> >
> >
> >
> >
> > ___
> > osg-users mailing list
> > osg-users@lists.openscenegraph.org
> > http://lists.openscenegraph.org/listinfo.cgi/osg-users-
> openscenegraph.org
> >
> ___
> osg-users mailing list
> osg-users@lists.openscenegraph.org
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
>
/* OpenSceneGraph example, osgcallback.
*
*  Permission is hereby granted, free of charge, to any person obtaining a copy
*  of this software and associated documentation files (the "Software"), to deal
*  in the Software without restriction, including without limitation the rights
*  to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
*  copies of the Software, and to permit persons to whom the Software is
*  furnished to do so, subject to the following conditions:
*
*  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
*  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
*  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
*  AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
*  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
*  OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
*  THE SOFTWARE.
*/

#include 

#include 
#include 

#include 
#include 

#include 

class TwiceEventCallback : public osgGA::GUIEventHandler
{
public:
   virtual bool handle(const osgGA::GUIEventAdapter& ea, osgGA::GUIActionAdapter& aa, osg::Object*, osg::NodeVisitor* nv) 
   { 
  if (ea.getEventType() == osgGA::GUIEventAdapter::PUSH)
  {
 OSG_ALWAYS << "[Twice] Mouse PUSH on frame: " << nv->getFrameStamp()->getFrameNumber() << std::endl;
  }

  return false;
   }
};


class CrashEventCallback : public osg::Callback
{
   virtual bool run(osg::Object* object, osg::Object* data)
   {
  osgGA::EventVisitor* ev = data->asNodeVisitor()->asEventVisitor();

  if (ev)
  {
 for (osgGA::EventQueue::Events::iterator itr = ev->getEvents().begin();
itr != ev->getEvents().end();
++itr)
 {
osgGA::GUIEventAdapter* ea = (*itr)->asGUIEventAdapter();
if (ea->getEventType() == 

Re: [osg-users] GUIEventHandler called twice - possible bug(s)

[Robert Osfield]

Hi Ricky,

The changes to Drawable and the knock on effect to the callbacks is an
awkward one - it would be easy if it we could just discard backwards
compatibility, so it's a far from ideal code, but alas you can't
rewrite history once it's happened.

>From your description I don't have a clear idea of what is being
called twice and whether this is a problem.  Could you create a small
example that illustrates the problem, once I can see first hand I can
review the behaviour and decide whether this is acceptable or is an
bug.

Thanks,
Robert.

On 29 August 2016 at 15:00, Riccardo Corsi  wrote:
> Hi all,
>
> I report what I believe could be considered a bug, probably introduced after
> the promotion of the Drawable class to a node, together with the new
> callback system.
>
> I have a GUIEventHandler derived class installed on a Drawable.
> After some digging I've realized that it's called twice for every event due
> to the EventVisitor code here:
> https://github.com/openscenegraph/OpenSceneGraph/blob/master/include/osgGA/EventVisitor#L86
>
> GUIEventHandler happens to derive both from NodeCallback and
> DrawableEventCallback, resulting in it being called twice. I think this
> might be considered as a bug if the intended behavior was to keep using the
> old GUIEventHandler interface the way it was (when attached to the viewer
> for instance, it is called only once as expected).
>
> Relate issue:
> I've re-implemented my callback deriving from osg::Callback, but this
> results in a crash as the osg::CallbackObject interface is used even if the
> cast does not succeed (first if condition)
> https://github.com/openscenegraph/OpenSceneGraph/blob/master/include/osgGA/EventVisitor#L93
>
> I fixed that by deriving from osg::NodeCallback, but I had to reimplement
> the operator() method instead of run(), as the second is not called by the
> visitor - that also it's a bit counter-intuitive with respect to the
> osg::Callback class interface.
>
> Ricky
>
>
>
>
>
>
>
> ___
> osg-users mailing list
> osg-users@lists.openscenegraph.org
> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
>
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

[osg-users] GUIEventHandler called twice - possible bug(s)

[Riccardo Corsi]

Hi all,

I report what I believe could be considered a bug, probably introduced
after the promotion of the Drawable class to a node, together with the new
callback system.

I have a GUIEventHandler derived class installed on a Drawable.
After some digging I've realized that it's called twice for every event due
to the EventVisitor code here: https://github.com/
openscenegraph/OpenSceneGraph/blob/master/include/osgGA/EventVisitor#L86

GUIEventHandler happens to derive both from NodeCallback
and DrawableEventCallback, resulting in it being called twice. I think this
might be considered as a bug if the intended behavior was to keep using the
old GUIEventHandler interface the way it was (when attached to the viewer
for instance, it is called only once as expected).

Relate issue:
I've re-implemented my callback deriving from osg::Callback, but this
results in a crash as the osg::CallbackObject interface is used even if the
cast does not succeed (first if condition)
https://github.com/openscenegraph/OpenSceneGraph/blob/master/include/osgGA/EventVisitor#L93

I fixed that by deriving from osg::NodeCallback, but I had to reimplement
the operator() method instead of run(), as the second is not called by the
visitor - that also it's a bit counter-intuitive with respect to the
osg::Callback class interface.

Ricky
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org