DIRTYFLAGS_DATA

Helper

On 04/12/2017 at 23:43, xxxxxxxx wrote:

Andreas, thanks for bringing up the BaseSelect::GetDirty(), as this has been something I had overlooked.
However, I do get inconsistent results from this.
Sorry to keep bringing up these weird situations.

In the same test MessageData plugin, I thus check on the dirty value of the baseselect.
It changes each time one or more polygons are selected/deselected.
The increment of the dirty value seems to depend on the amount of polygons that get selected/deselected. Not really that important for now, but if there is some logic behind it, it might be useful for me to understand.

Now, when I perform an undo or redo, the dirty value increments with a value of one. Again, the increment value is not that important. But the dirty value changes, which is important to know.
When you use loop or ring selection, the dirty value increments as expected, except if you perform undo or redo. Then suddenly the dirty value DOES NOT CHANGE.
Still the selection is different before and after an undo.

As such, there is again an inconsistency between live selection and loop/ring selection. Which means checking the dirty value is again not a common solution.

Helper

On 05/12/2017 at 06:20, xxxxxxxx wrote:

I already mentioned (and explained the reasons) in the other thread (https://developers.maxon.net/forum/topic/10483/13933_notification-on-tag-delete-or-rename), IsDirty() shouldn't be used in this context. Instead use GetDirty(). Also one should not try to interpret any meaning into the changes of the dirty counts. Only compare them to the last value on (in-)equality.

I'm terribly sorry, you are encountering one issue after another. You seem to have a run there...

I can reproduce the behavior of the BaseSelect dirty count. In my tests a combination of the host object's dirty count and the BaseSelect's dirty count seem to work. But I'm sure you will find another issue (but please don't stop, every issue we are aware of helps in the end) and I will also add this topic to my discussion with development.

Helper

On 05/12/2017 at 07:11, xxxxxxxx wrote:

Yes, read the IsDirty() explanation from the other thread.

Combination of object's dirty and BaseSelect's dirty ... humm, where did I see that before.

In a former life this is what I did:

  
  UInt32 objectDirty = object->GetHDirty(HDIRTYFLAGS_OBJECT);  
  UInt32 selectionDirty = object->GetDirty(DIRTYFLAGS_SELECT);  
  Bool selectionHasChanged = (mLastSelectionChecksum != selectionDirty);  
  // When points, edges or polygons are selected via loop or ring selection,  
  // performing an undo will not change the dirty checksum for DIRTYFLAGS_SELECT,  
  // this dirty checksum is changed when undoing selections done via live, path, ... selection  
  // In order to still detect selection changes we then look at the dirty flag on object  
  // (which is changed by the undo/redo, but also most of all other user action, we thus need to filter)  
  if (!selectionHasChanged && (mLastObjectChecksum != objectDirty))  
  {  
      mLastObjectChecksum = objectDirty;  
      // check if the current selected items are still the same as previously,  
      // user pressing undo will not trigger a change in DIRTYFLAGS_SELECT  
      // when items were selected by ring selection or loop selection  
      // This is time consuming for all the cases where object is dirty and selection not,  
      // but is the only solution to detect undo/redo for loop and ring selection  
...

Not exactly the same as checking object and baseselect, but close.
Now, I wanted to get rid of this "dirty" implementation (pun intended), but it seems I'll have to embrace it once again I am afraid.

Looking forward to developers' feedback on both this and the other topic.

By the way, is there a reason why BaseObject::GetDirty(...) returns an UInt32 while BaseSelect::GetDirty() returns an Int32 ?

Helper

On 05/12/2017 at 07:21, xxxxxxxx wrote:

Regarding signed'ness of the different dirty counts... really? If I had to guess, these were implemented by two different developers and somebody failed to synchronize them. On the other hand, I'm sure, there are profound reasons, I just don't know them.

For your checks simply adding the dirty counts should do the trick:

const Int32 dirtyCount = obj->GetDirty(DIRTYFLAGS_ALL) + selection->GetDirty();
if (dirtyCount == mLastDirtyCount)
	return; // nothing to do
mLastDirtyCount = dirtyCount;
// ...

Helper

On 05/12/2017 at 07:50, xxxxxxxx wrote:

Nah, the signed/unsigned question was just a joke.
I guessed the same thing as you did, but then again, never know if the difference was deliberate.

Helper

On 06/12/2017 at 03:00, xxxxxxxx wrote:

There, I edited the post and moved the part about the signed/unsigned so as not to assume too much about it.

Additionally, I have now filtered out the BaseSelect changes as a result of undo/redo, and can thus (in theory) skip the check of object dirtiness.
Which doesn't mean I am not interested in feedback from developers about the inconsistent dirty count change. But for now, and this particular scenario, I seem to have a workaround.

Moving focus to the other issue of finding a proper way of detecting geometry change.

Helper

On 06/12/2017 at 05:50, xxxxxxxx wrote:

The behavior of the BaseSelect's dirty count after undo (when loop or ring selection are undone, dirty count staying constant) is a bug.
As a workaround please stay with the suggestion above. I'll try to notify here, when it is fixed.

Helper

On 06/12/2017 at 06:03, xxxxxxxx wrote:

Sad to hear it's a bug.

Would this bug also be involved in the other inconsistent behaviour of the ring/loop selection reported in https://developers.maxon.net/forum/topic/736/13898_detect-polygon-selection-change-revisited
Where performing an undo would result in a crash, while it doesn't crash when undoing a live-selection action

Excerpt of the problem below:

  
    
    
                // selection change detected, undo the selection change  
              doc->DoUndo();  
      
              // we will now create our own undo stack,  
              // make the selection change and our own change  
              Bool ret = doc->StartUndo();  
              ret = doc->AddUndo(UNDOTYPE_CHANGE, object);    // <- crashes when loop selection  
      
              // step 1. redo the selection change for the new undo stack  
              BaseSelect* polyS = ToPoly(object)->GetPolygonS();  
              for (const auto& item : added)  
                  polyS->Select(item);  
              for (const auto& item : removed)  
                  polyS->Deselect(item);  
      
              // step 2. perform the actual action

Helper

On 06/12/2017 at 06:07, xxxxxxxx wrote:

And I thought you'd be happy to hear it's a bug. This way in future a more consistent behavior is to be expected.

For the other question: I can't tell, yet. Please be patient, I will get back to you in the other thread as soon as I have something new.

Helper

On 06/12/2017 at 06:28, xxxxxxxx wrote:

Oh no, I'm not at all happy that it's a bug.

First, it means that we'll need to wait for it to get fixed.
If it wasn't a bug but a user-error, I might be told to implement it this or that way. And a solution might be available sooner.
Secondly, when the bug gets fixed that means in order to support different Cinema releases I will have to provide two versions of the implementation: a pre and a post bug-fix

So, I would prefer no bugs get fixed
(and now I am going to duck and cover, as I hear things being thrown at me ...)
Of course I am looking forward to a bug-fix and more consistent behaviour.

I think this thread can be closed as solved. I'll follow the progress of the remaining two issues in their appropriate threads.
Thanks Andreas.