XYPlot rendering logic in jfreechart-fse 1.0-SNAPSHOT

[dbundy]

Hello David,

I realize there have been a number of changes/improvements in the last builds of jfreechart-fse 1.0-SNAPSHOT version.
Most of them are very good. Thank you. Besides, the graphics seems more pretty.

Nevertheless, i have a problem with one of those.

The XYPlot seems to have fully changed about its rendering logic.
Previously, (in older versions of jfreechart-fse 1.0-SNAPSHOT, about end of 2013), we could use any index for datasets and renderers.
Which allows us to create datasets and renders groups by using a range for example (1-100 -> some data; 101-200-> other data; etc...)
In this way, we can handle in a separate way each group and realize some grouped actions (delete, highlight, save, etc...) .
With the new implementation, It seems that it 's not possible any longer because all rendering handling use the size of the renders and datasets Map to iterate on them from zero to their size.

So, i wonder for for which reason have you proceed like that ?

[david.gilbert]

There was a bug with the ObjectList class and while I was fixing that I decided that ObjectList is more or less just a Map so I replaced it with a standard Map. You should still be able to use any index for your dataset, although when I look at the getDataset(int) method, it looks like it may be implemented incorrectly:

Code: Select all

    public XYDataset getDataset(int index) {
        XYDataset result = null;
        if (this.datasets.size() > index) {
            result = this.datasets.get(index);
        }
        return result;
    }

...as there is no reason here why the index should be less than the number of entries in the map. I'll have to review this.

[dbundy]

David,

Thank your for your come back.
I understand. I agree with you. It's always better to use native object when it's enough.

I have noticed that this way of iterating on dataset is the same for renders and at many multiples places in XYPlot.
Not exhaustive, for example :

Here :

Code: Select all

  public XYItemRenderer getRenderer(int index) {
        XYItemRenderer result = null;
        if (this.renderers.size() > index) {
            result = this.renderers.get(index);
        }
        return result;
 }

Here :

Code: Select all

public XYItemRenderer getRendererForDataset(XYDataset dataset) {
        XYItemRenderer result = null;
        for (int i = 0; i < this.datasets.size(); i++) {
            if (this.datasets.get(i) == dataset) {
                result = this.renderers.get(i);
                if (result == null) {
                    result = getRenderer();
                }
                break;
            }
        }
        return result;
    }

Here too :

Code: Select all

    public void draw(Graphics2D g2, Rectangle2D area, Point2D anchor,
            PlotState parentState, PlotRenderingInfo info) {
   .... 
   for (int i = 0; i < this.renderers.size(); i++) {
            drawDomainMarkers(g2, dataArea, i, Layer.BACKGROUND);
        }
        for (int i = 0; i < this.renderers.size(); i++) {
            drawRangeMarkers(g2, dataArea, i, Layer.BACKGROUND);
        }

I have modified the source and rebuild it and it seems OK.
XYPlotTest.java passes too.
The modified version is in the next message :

[dbundy]

The java code owns too much characters to be posted. Anyway, i can share by another way if you wish.
Thank you for your work.

[david.gilbert]

I'm testing a patch for both XYPlot and CategoryPlot at the moment. It's along the lines of what you described, but there are a few other fields affected as well (the axes, axis locations, datasets, markers and renderers) and the rendering order code needed fixing as well.

[david.gilbert]

It is pushed to the repo at GitHub now. Quite a few changes, so there are bound to be some issues…let me know if you see anything that needs fixing.

[dbundy]

Hi,

Sorry, I haven't tried yet the new version and I could not for the moment because it will miss a way to iterate through renders or datasets, outside the XyPlot instances.
That, because in the logic used, the only way to iterator through them was the assumed fact that the indexes increment from 1 to 1 and begin to zero.
In this way, outside the XyPlot insance, we could do :

Code: Select all

for (int i=0; i < myXyPlot.getDatasetCount(); i++){
  XYDataset currentDataset =  myXyPlot.getDataset(i);
}

To bypass this lack, in my local version, I had added a public method in XYPlot in order to iterate :

Code: Select all

 public Set<Integer> getDatasetIndexes() {
	return datasets.keySet();
}

So in client, I can access in this way :

Code: Select all

for (Integer datasetIndex : getXYPlot().getDatasetIndexes()) {
    XYDataset dataset = getXYPlot().getDataset(datasetIndex );
    XYRender render = getXYPlot().getRender(datasetIndex );
}

The same method for retrieve renders indexes could be useful too.

[david.gilbert]

Yes, you are right. In the patch I committed, there are methods to fetch the dataset indices in order, like this:

Code: Select all

private List<Integer> getDatasetIndices(DatasetRenderingOrder order)

I should probably make those methods public. I'm also tempted to change the keys from Integer to Comparable, so that it is clear that the datasets are using keys rather than indices.

[dbundy]

I agree with you. Integer as key is too restricted indeed and finally misleading.
Nevertheless, why do you think about the Comparable Interface precisely ?
If in XYPlot, you don't use the natural order of the keys to iterate through datasets or renders, I think that Object is simpler and less confusing.

[david.gilbert]

I like Comparable because it defines a natural ordering and more or less guarantees that equals() is implemented in an obvious way. And the common key types that people are likely to use (String and Integer) already implement this interface.

[olbrich]

Thanks a lot,

thats exacly my problem today. Extending XYPlot and extending CategoryPlot and iteratings through renderers, dataSets ans axes.

In which version is any new protected or public method availiable?

regards markus

[paradoxoff]

I saw this thread just today and had a look at the FSE on github.
IMHO, the new map-based way in which datasets, renderers and all the other objects are accessed is much less intuitive than with the current, standard JFreeChart.
While it is certainly possible to replace an List<AnObject> with a Map<Integer, AnObject>, a couple of drawbacks are already mentioned in this thread. Going from Integer to Comparable or even Object looks very counterintuitive to me. The call

Code: Select all

xyplot.setDataset("FooBar", theDataset)

just looks weird.
I only see the benefit of a Map<Integer, AnObject> is the range of Integers is large and non-coherent, in the worst case containing two value with indices 0 and Integer.MAX_VALUE. it certainly doesn´t make sense to allocate billions of null objects in a list.
Is there any benefit of using a map that I might have overseen?

[paradoxoff]

I just had a look at the source of XYPlot in the newest version (1.0.19). This version is already using a Map-based approach to store datasets, renderers, etc.
I haven´t tried the new version (I'm using a heavily modified custom version based on JFreeChart 1.0.13), but I believe that the new version will break my existing JFreeChart based application.
My most inportant concerns are:
- As mentioned above, the loop

Code: Select all

for(int i = 0; i < plot.getDatasetCount(); i++){
	XYDataset d = plot.getDataset(i);
}

is not guaranteed to work as their might be datasets with an integer key that is larger than the size of the dataset map.
- Using negative values as int parameter in XYPlot.setDataset(int index, XYDataset dataset) is forbidden according to the API documentation, but appears to be possible without throwing an exception. Datasets with a negative index are not accessible by the loop above. If a dataset is not present in the dataset map, XYPlot.indexOf(XYDataset dataset) returns -1. Since this value is still possible as a normal dataset index (if one ignores the API doc), this may lead to erroneous behaviour.
Are their any plans to return to a List-based approach to store the elements of an XYPlot, or is this new Map based approach expected to persist?

[paradoxoff]

David, if you can find some time, could you please briefly outline your future plans regarding this topic?

[paradoxoff]

*Push*
Just want to know whether the new map-based approach will be used from now on.