Menu

#149 patches for 1647269 and 1659627 (IntervalMarker rendering)

closed-accepted
None
5
2007-03-01
2007-02-28
No

This is a patch for the following issue:
1659627 IntervalMarker with Double.POSITIVE_INFINITY bound
https://sourceforge.net/tracker/index.php?func=detail&aid=1659627&group_id=15494&atid=115494

Implementation notes:

1. I had to code all the clipping myself, because Rectangle2D.Double#createIntersection() did not work well with infinite bounds and produced NaNs while trying to calculate the width/height of the clipped rectangle. Therefore, I compared the bounds first, clipped them and only after that calculated the new width/height.

2. The marker label was not displayed in the correct location. When I previously created a marker with an infinite upper bound and also set label anchor to RectangleAnchor.CENTER, the label was drawn in the infinity, off the screen. Now the label is drawn in the centre of the visible marker area.

3. There was a bug in AbstractXYItemRenderer#drawRangeMarker(), which resulted in outline not being drawn correctly (X and Y coordinates were swapped). This was also fixed.

4. I have successfully tested the patch in all possible combinations of plot and axis orientation, with both linear and logarithmic axes.

5. As correctly pointed out in a discussion of a similar problem:
http://www.jfree.org/phpBB2/viewtopic.php?t=20196
the clipping will affect the gradient paint, particularly when coupled with scaling GradientPaintTransformer. As the drawing area for the marker is clipped to the visible area, the gradient paint will be resized to fit inside the visible area (whereas previously it was stretched to the whole marker area). I do not know if this change in behaviour can be considered critical. One problem I see with the standard GradientPaintTransformer is that it demotes double coordinates to a float range, which results in loss of precision and may still have weird side effects in some situations. In the light of this, clipping will probably do more good than harm.

Discussion

  • Sergei Ivanov

    Sergei Ivanov - 2007-02-28

    Patch against 1.0.4

     
  • Sergei Ivanov

    Sergei Ivanov - 2007-02-28

    Patch against 1.0.4

     
  • Sergei Ivanov

    Sergei Ivanov - 2007-02-28
    • summary: patch for 1659627 (IntervalMarker rendering) --> patches for 1647269 and 1659627 (IntervalMarker rendering)
     
  • Sergei Ivanov

    Sergei Ivanov - 2007-02-28

    Logged In: YES
    user_id=1606022
    Originator: YES

    Applied a similar fix to the category item renderer in order to fix
    [ 1647269 ] IntervalMarker with Double.MAX_VALUE as upper bound
    https://sourceforge.net/tracker/index.php?func=detail&aid=1647269&group_id=15494&atid=115494

    The modified file is attached.
    File Added: AbstractCategoryItemRenderer.java

     
  • Sergei Ivanov

    Sergei Ivanov - 2007-02-28

    Logged In: YES
    user_id=1606022
    Originator: YES

    I suddenly realised that my IDE has automatically stripped all trailing white space. When comparing the files against their reference versions, please ignore the changes in trailing white space. I apologise for the inconvenience caused. I can reapply changes and resubmit files if needed.

     
  • David Gilbert

    David Gilbert - 2007-02-28

    Logged In: YES
    user_id=112975
    Originator: NO

    Hi Sergei,

    I reviewed your AbstractXYItemRenderer.java changes, and the fixes look good. Do you want to commit this to CVS yourself, or would you rather I do it? If you want to do it, I'll need to set you up for CVS write access. Then the procedure is: (1) you need to commit the change along with an entry in the ChangeLog file, and (2) send an e-mail to the jfreechart-patches mailing list with a short note about the change, a copy of the ChangeLog entry, and a CVS diff (cvs diff -uN > diff.txt). That just gives people on the patches mailing list a chance to look more closely at your change if they think it needs reviewing.

    I'll go take a look at the AbstractCategoryItemRenderer.java changes...I'm sure they will also be fine.

    Regards,

    Dave Gilbert
    JFreeChart Project Leader

     
  • David Gilbert

    David Gilbert - 2007-02-28
    • assigned_to: nobody --> mungady
     
  • Sergei Ivanov

    Sergei Ivanov - 2007-02-28

    Logged In: YES
    user_id=1606022
    Originator: YES

    Two extra notes:
    1. Currently drawRangeMarker in XY and Category renderers are totally identical.
    2. drawRangeMarker in XY renderer is very similar to drawDomainMarker. One can be implemented in terms of the other by passing in the correct axis edge and by passing in the inverted orientation.

    Is it possible to extract this functionality into a separate marker renderer class for value axes, so that this massive copy-paste job is avoided?

     
  • Sergei Ivanov

    Sergei Ivanov - 2007-02-28
    • assigned_to: mungady --> nobody
     
  • David Gilbert

    David Gilbert - 2007-02-28
    • assigned_to: nobody --> mungady
     
  • David Gilbert

    David Gilbert - 2007-02-28

    Logged In: YES
    user_id=112975
    Originator: NO

    (1) AbstractCategoryItemRenderer.java : I reviewed the changes, and you are welcome to commit those too (or else I will). Thanks for working on this.

    (2) Trailing white-space : no problem, I removed the white-space and committed to CVS, so now the diffs are easy to read;

    (3) Factoring out the marker drawing is OK by me, as long as it doesn't require breaking the API (which I don't think it does, if I understand correctly what you want to do).

    Regards,

    Dave

     
  • David Gilbert

    David Gilbert - 2007-03-01
    • status: open --> closed-accepted
     
  • David Gilbert

    David Gilbert - 2007-03-01

    Logged In: YES
    user_id=112975
    Originator: NO

    Thanks, these changes are committed to CVS for inclusion in the 1.0.5 release.

     

Log in to post a comment.