Modify

Opened 12 months ago

Closed 12 months ago

Last modified 3 months ago

#15670 closed defect (fixed)

[patch] filtering the dataset does not generate any events, despite of the fact that the DataSet has actually changed

Reported by: cmuelle8 Owned by: team
Priority: normal Milestone: 17.12
Component: Core Version: latest
Keywords: filtering dataset DataChangedEvent filterdialog Cc:

Description

Code and some plugins listen for DataChangedEvents.

Filtering modifies isDisabled() and isHidden() attributes of OsmPrimitives.
Hence the dataSet may indeed have changed after applying or removing a filter with the FilterDialog.

Symptom:
When a filter is made active, no DataChangedEvent is fired.

Please find attached patch that fires a new DataChangedEvent if the current dataset
has been modified after all current filters executed. It is up to the receiver how
or if this event will be consumed.

Change History (5)

comment:1 in reply to:  description Changed 12 months ago by Don-vip

Replying to cmuelle8:

Filtering modifies isDisabled() and isHidden() attributes of OsmPrimitives.

Then don't we have a PrimitiveFlagsChangedEvent fired?

comment:2 Changed 12 months ago by cmuelle8

This is the call chain, none of these methods fires an event, so it seems we do not.
Since filters usually modify a rather large part of the dataset, I guess it won't
replace the necessity to generate and fire a DataChangedEvent.

In the vast majority of cases a filter action will modify more than 30 primitives,
30 being the current maximum in DataSet.java to fire cachedEvents individually.

This means we should not pressure the event queue or the consumer
with one event per primitive each using

but instead fire one for the whole collection changed. This will need a class
like PrimitivesFlagsChangedAdded, that constructs with a collection rather
than a single primitive. Firing with a collection will be consistent to how
added and removed primitives are handled, see firePrimitivesAdded()
and firePrimitivesRemoved().


So instead of (-I-)

fireEvent(new DataChangedEvent(this));

we would talk about extending it to (-II-)

// flagsChanged is of type Collection<? extends OsmPrimitive>
fireEvent(new DataChangedEvent(this, new PrimitivesFlagsChangedAdded(flagsChanged));

If more than 1000 (MAX_EVENTS) individual firePrimitiveFlagsChanged() were
generated instead, all that is fired is a single new DataChangedEvent(this).
This means each consumer must provide for the case to scan the whole dataset at all times.

Firing individual events means the consumer would have to listen for and process three different cases:

  • individual events
  • DataChangedEvent that wraps events (process wrapped)
  • DataChangedEvent without payload (process/scan the whole dataset)

Firing one event (either without or with flagsChanged collection payload) the consumer logic
simplifies to processing a single event, which is another reason to avoid using
firePrimitiveFlagsChanged() adding to the one already discussed above.


So from the perspective the current code offers, an argument to implement (-II-) instead of
(-I-) may solely be that lots of filter operations modify flags of less than 1000 elements
(in relation to those modifying more).

Bloating event consumer logic for the very rare case that a filter modifies <30 elements,
seems unnecessary / over-elaborate / cumbersome.

Also note that (-I-) paves the way to (-II-), so having (-I-) first, does not hinder
implementation of (-II-) at a later time.

Last edited 12 months ago by cmuelle8 (previous) (diff)

comment:3 Changed 12 months ago by Don-vip

Resolution: fixed
Status: newclosed

In 13208/josm:

fix #15670 - fire a data set change event when applying filters (patch by cmuelle8)

comment:4 Changed 3 months ago by Don-vip

In 14206/josm:

fix #16698, see #15670 - make sure filters are executed (costly operation) only when necessary:

  • data changes imply execution of filters only when at least a filter is enabled
  • filter changes imply execution of filters even is no filter is enabled
  • filter dataset change events should not trigger a new filter execution!

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.