Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix data selection following zooming with mouse #48

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mosherubin
Copy link

The Problem

Data selection works fine when the chart is unzoomed, with an app's selectionChanged() callback being invoked following data point selection. When the chart is zoomed, however, the selectionChanged() callback is not invoked.

Playing with zooming shows that data selection always fails following zooming using the mouse (i.e., mouse button down + drag). When zooming using the mouse wheel, data selection works for the first few zoom-in mouse wheel works. Once the chart has been enlarged a few times, selection fails.

Reproducing the Problem

The problem can be reproduced easily with a minimally modified SelectionDemo1.java file.

The Technical Explanation

Examining the FSE code shows that the reason for SelectionChanged() not being called following zooming is because the hash value of the DatasetSelectionExtension<XYCursor> object registered in function createDemoPanel() is modified because of zooming. This causes the statement "registeredExtensions.get(dataset)" in DatasetExtensionManager.getExtension() to return null:

public <T extends DatasetExtension> T getExtension(Dataset dataset, 
            Class<T> interfaceClass) {        
        if (interfaceClass.isAssignableFrom(dataset.getClass())) {
            //the dataset supports the interface
            return interfaceClass.cast(dataset);
        } else {
           List<DatasetExtension> extensionList 
                    = registeredExtensions.get(dataset);
            if (extensionList != null) {
                for (DatasetExtension extension : extensionList) {
                    if (interfaceClass.isAssignableFrom(extension.getClass())) {
                        //the dataset does not support the extension but
                        //a matching helper object is registered for the dataset
                        return interfaceClass.cast(extension);
                    }
                }
            }
        }
                
        return null;        
    }

Drilling down further shows that the Calendar member variable workingCalendar has some of its member variables changed due to zooming.

The hash returned by the Calendar class takes these into account, causing the HashMap get() to fail and return null, preventing selectionChanged() from being invoked.

For an explanation complete with screen captures, see the following PDF file.

The Proposed Fix

I believe workCalendar is unsafe to include in the hash of a TimesSeriesCollection object and should be removed. I would like to propose removing it from TimeSeriesCollection.hashCode(). When I made the fix locally and rebuilt FSE, data selection works perfectly on both zoomed and non-zoomed charts.

##The Problem##

Data selection works fine when the chart is unzoomed, with an app's ```selectionChanged()``` callback being invoked following data point selection.  When the chart is zoomed, however, the ```selectionChanged()``` callback is not invoked.

Playing with zooming shows that data selection always fails following zooming using the mouse (i.e., mouse button down + drag).  When zooming using the mouse wheel, data selection works for the first few zoom-in mouse wheel works.  Once the chart has been enlarged a few times, selection fails.

##Reproducing the Problem##

The problem can be reproduced easily  with a [minimally modified ```SelectionDemo1.java``` file](https://drive.google.com/open?id=0B09i2ZH5SsTHMmoxa3ZuMWd4WVU).

##The Technical Explanation##

Examining the FSE code shows that the reason for ```SelectionChanged()``` not being called following zooming is because the hash value of the ```DatasetSelectionExtension<XYCursor>``` object registered in function ```createDemoPanel()``` is modified because of zooming.  This causes the statement "registeredExtensions.get(dataset)" in ```DatasetExtensionManager.getExtension()``` to return null:

```java
public <T extends DatasetExtension> T getExtension(Dataset dataset, 
            Class<T> interfaceClass) {        
        if (interfaceClass.isAssignableFrom(dataset.getClass())) {
            //the dataset supports the interface
            return interfaceClass.cast(dataset);
        } else {
           List<DatasetExtension> extensionList 
                    = registeredExtensions.get(dataset);
            if (extensionList != null) {
                for (DatasetExtension extension : extensionList) {
                    if (interfaceClass.isAssignableFrom(extension.getClass())) {
                        //the dataset does not support the extension but
                        //a matching helper object is registered for the dataset
                        return interfaceClass.cast(extension);
                    }
                }
            }
        }
                
        return null;        
    }
```

Drilling down further shows that the ```Calendar``` member variable ```workingCalendar``` has some of its member variables changed due to zooming.

The hash returned by the ```Calendar``` class takes these into account, causing the ```HashMap get()``` to fail and return null, preventing ```selectionChanged()``` from being invoked.

For an explanation complete with screen captures, [see the following PDF file](https://drive.google.com/open?id=0B09i2ZH5SsTHZ3dpTmViWW5hRm8).

##The Proposed Fix##

I believe ```workCalendar``` is unsafe to include in the hash of a ```TimesSeriesCollection``` object and should be removed.  I would like to propose removing it from ```TimeSeriesCollection.hashCode()```.  When I made the fix locally and rebuilt FSE, data selection works perfectly on both zoomed and non-zoomed charts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant