[vtk-developers] [VTK 0012044]: Order of called observers

Julien Finet julien.finet at kitware.com
Mon Apr 4 15:39:50 EDT 2011


If people didn't need ordering guarantees, then "passive", "focus" and
observer "priorities" wouldn't be there.

If we define the order to be arbitrary, there is a cost associated to it.
People would need to enforce their order by adding extra code (increased
number of code, extra computation time).

Julien.

On Mon, Apr 4, 2011 at 3:18 PM, tom fogal <tfogal at sci.utah.edu> wrote:

> I was trying to constify that code recently, and I don't think either of
> the explanations is accurate.
>
> The actual ordering appears to be:
>
>  1. invoke all "passive" observers (which cannot change system state)
>  2. invoke "focus" observers
>  3. invoke anything not invoked by the above
>
> (see InvokeEvent for full info)
>
> I'd say it's better to make the order undefined... do people really need
> ordering guarantees (what's a use case?)?
>
> -tom
>
>
> On 04/04/2011 12:18 PM, Julien Finet wrote:
>
>> The question here is:
>>
>> 1) do we change the documentation and claim the order of callbacks is
>> undefined (fix documentation) or
>> 2) do we enforce that the observers are called in the order they were
>> added
>> (fix bug).
>>
>> What are benefits of the later and former solution ?
>>
>> While 1) is definitely safer, I doubt that it can be a safe long-term
>> solution.
>> Moreover, I believe that 2) has a somewhat limited impact.
>>
>> Please let us know what you think,
>>
>> Thanks,
>> Julien.
>> p.s. For information, the Qt framework was documenting an arbitrary order
>> until the last versions 4.5, they are now (since 4.6) following the order
>> when the connections are added.
>>
>>
>> On Mon, Apr 4, 2011 at 2:15 PM, Mantis Bug Tracker<
>> mantis at public.kitware.com>  wrote:
>>
>>
>>> The following issue has been SUBMITTED.
>>> ======================================================================
>>> http://public.kitware.com/Bug/view.php?id=12044
>>> ======================================================================
>>> Reported By:                Julien Finet
>>> Assigned To:
>>> ======================================================================
>>> Project:                    VTK
>>> Issue ID:                   12044
>>> Category:                   (No Category)
>>> Reproducibility:            always
>>> Severity:                   major
>>> Priority:                   normal
>>> Status:                     new
>>> ======================================================================
>>> Date Submitted:             2011-04-04 14:15 EDT
>>> Last Modified:              2011-04-04 14:15 EDT
>>> ======================================================================
>>> Summary:                    Order of called observers
>>> Description:
>>> Nightly build documentation of vtkObject::AddObserver() says: "When
>>> events
>>> are
>>> invoked, the observers are called in the order they were added."
>>>
>>> However the first observer of any given priority (default 0. included) is
>>> always
>>> called last.
>>> For observers added in the following order:
>>> observer1
>>> observer2
>>> observer3
>>> observer4
>>>
>>> The actual calling order is:
>>> observer2
>>> observer3
>>> observer4
>>> observer1<- notice how the first observer is called last
>>>
>>>
>>> Steps to Reproduce:
>>> void objectModified1(vtkObject *caller,unsigned long eid, void
>>> *clientData,
>>> void
>>> *callData) {
>>>  std::cout<<  "modified1"<<  std::endl;
>>> }
>>>
>>> void objectModified2(vtkObject *caller,unsigned long eid, void
>>> *clientData,
>>> void
>>> *callData) {
>>>  std::cout<<  "modified2"<<  std::endl;
>>> }
>>>
>>> void objectModified3(vtkObject *caller,unsigned long eid, void
>>> *clientData,
>>> void
>>> *callData) {
>>>  std::cout<<  "modified3"<<  std::endl;
>>> }
>>>
>>> void objectModified4(vtkObject *caller,unsigned long eid, void
>>> *clientData,
>>> void
>>> *callData) {
>>>  std::cout<<  "modified4"<<  std::endl;
>>> }
>>>
>>> int Test(int,char *[])
>>> {
>>>  vtkObject* obj = vtkObject::New();
>>>
>>>  vtkCallbackCommand* callback1 = vtkCallbackCommand::New();
>>>  callback1->SetCallback(objectModified1);
>>>  obj->AddObserver(vtkCommand::ModifiedEvent, callback1);
>>>
>>>  vtkCallbackCommand* callback2 = vtkCallbackCommand::New();
>>>  callback2->SetCallback(objectModified2);
>>>  obj->AddObserver(vtkCommand::ModifiedEvent, callback2);
>>>
>>>  vtkCallbackCommand* callback3 = vtkCallbackCommand::New();
>>>  callback3->SetCallback(objectModified3);
>>>  obj->AddObserver(vtkCommand::ModifiedEvent, callback3);
>>>
>>>  vtkCallbackCommand* callback4 = vtkCallbackCommand::New();
>>>  callback4->SetCallback(objectModified4);
>>>  obj->AddObserver(vtkCommand::ModifiedEvent, callback4);
>>>
>>>  obj->Modified();
>>>
>>>  callback4->Delete();
>>>  callback3->Delete();
>>>  callback2->Delete();
>>>  callback1->Delete();
>>>
>>>  obj->Delete();
>>>
>>>  return 1;
>>> }
>>>
>>> Output:
>>> modified2
>>> modified3
>>> modified4
>>> modified1
>>>
>>>
>>> Additional Information:
>>> Current code (and suggested fix):
>>>
>>>
>>> //----------------------------------------------------------------------------
>>> unsigned long vtkSubjectHelper::
>>> AddObserver(unsigned long event, vtkCommand *cmd, float p)
>>> {
>>>  vtkObserver *elem;
>>>
>>>  // initialize the new observer element
>>>  elem = new vtkObserver;
>>>  elem->Priority = p;
>>>  elem->Next = NULL;
>>>  elem->Event = event;
>>>  elem->Command = cmd;
>>>  cmd->Register(0);
>>>  elem->Tag = this->Count;
>>>  this->Count++;
>>>
>>>  // now insert into the list
>>>  // if no other elements in the list then this is Start
>>>  if (!this->Start)
>>>    {
>>>    this->Start = elem;
>>>    }
>>>  else
>>>    {
>>>    // insert high priority first
>>>    vtkObserver* prev = 0;
>>>    vtkObserver* pos = this->Start;
>>>    while(pos->Priority>= elem->Priority&&  pos->Next)
>>>      {
>>>      prev = pos;
>>>      pos = pos->Next;
>>>      }
>>>    // pos is Start and elem should not be start
>>>    if(pos->Priority>  elem->Priority)   ////////////////////////// HERE
>>> IT
>>> SHOULD BE: pos->Priority>= elem->Priority
>>>      {
>>>      pos->Next = elem;
>>>      }
>>>    else
>>>      {
>>>      if(prev)
>>>        {
>>>        prev->Next = elem;
>>>        }
>>>      elem->Next = pos;
>>>      // check to see if the new element is the start
>>>      if(pos == this->Start)
>>>        {
>>>        this->Start = elem;
>>>        }
>>>      }
>>>    }
>>>  return elem->Tag;
>>> }
>>> ======================================================================
>>>
>>> Issue History
>>> Date Modified    Username       Field                    Change
>>> ======================================================================
>>> 2011-04-04 14:15 Julien Finet   New Issue
>>> ======================================================================
>>>
>>> _______________________________________________
>>> Powered by www.kitware.com
>>>
>>> Visit other Kitware open-source projects at
>>> http://www.kitware.com/opensource/opensource.html
>>>
>>> Follow this link to subscribe/unsubscribe:
>>> http://www.vtk.org/mailman/listinfo/vtk-developers
>>>
>>>
>>>
>>
>>
>> _______________________________________________
>> Powered by www.kitware.com
>>
>> Visit other Kitware open-source projects at
>> http://www.kitware.com/opensource/opensource.html
>>
>> Follow this link to subscribe/unsubscribe:
>> http://www.vtk.org/mailman/listinfo/vtk-developers
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20110404/9c9b44fe/attachment.html>


More information about the vtk-developers mailing list