marcomachadosantos / gwt-chronoscope

Automatically exported from code.google.com/p/gwt-chronoscope
GNU Lesser General Public License v2.1
0 stars 0 forks source link

Improper clipping of leftmost line segment #47

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
To reproduce:
1. Go to timepedia.org
2. Zoom in once
3. Pan chart left and right and observe the angle of the leftmost line segment

The line segment should not change angle, but simply be clipped where it 
intersects the border 
of the plot area.

This may occur because domainAxis.dataToUser() will produce negative values for 
domain values 
less than the domain origin. When this is mapped to a pixel coordinate, it 
might get clipped. 
This doesn't happen on the right hand side because drawing lines to coordinates 
outside the 
layer are appropriately clipped. 

There appears to be two ways to solve this. The first is to change the 
XYRenders to do left-hand 
clipping by checking to see if the point extends outside the domain (x 
coordinate < 0) and 
perform interpolation on the Y value. 

The second is to alter the coordinate system so that negative values work. For 
example, we could 
translate() the origin to chartWidth/2, and change the to/from screenX 
coordinate functions to 
map the [0,1] user values to [-chartWidth/2, chartWidth/2]. The latter solution 
is probably more 
optimal, but it also requires us to check lots of other places where coordinate 
transforms from 
screen/window are used (clicks/hovers/etc, e.g. legend). So perhaps we should 
do the fix in the 
renders for now, although I dislike this solution in the long term because it 
requires people 
writing the renderers to know about this clipping artifact.

Original issue reported on code.google.com by cromwell...@gmail.com on 29 Jul 2008 at 7:26

GoogleCodeExporter commented 8 years ago
I added the linear interpolation in XYLineRenderer (see attached file), which 
fixes the problem.  Before 
checking it in, though, I had a couple of questions:

1) Clipping appears to work properly when the x value falls outside the layer 
to the left (i.e. x is negative).  By 
simply removing the zero-bounding in the ux variable assignment, the rendering 
worked properly in hosted 
mode and in Safari.  Is it the case that other targets won't necessarily handle 
the negative x clipping properly?

2) I was under the impression that the BarChartXYRenderer was not working 
properly or was in some state of 
flux, so do we want to apply any changes to this class at this time?

I agree that solution #2 would be optimal, and also that it would represent a 
much larger refactoring effort.  
So my suggestion is to close this defect issue once it's wrapped up and open a 
new issue for solution #2 with 
Enhancement+Refactoring labels.

Original comment by chadtaka...@gmail.com on 30 Jul 2008 at 3:49

Attachments:

GoogleCodeExporter commented 8 years ago
You mean this line: 
    if (ux - lx >= 0) {

was causing it? if so, perhaps changing it to Math.abs(ux-lx) >= 0. Actually, 
we can probably remove this line as 
I think they fixed this bug in Safari canvas in Safari3.

Original comment by cromwell...@gmail.com on 30 Jul 2008 at 4:41

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Re: my question #1, I was referring to line 97 in the original code (r246):

         double ux = Math.max(0, plot.domainToScreenX(dataX, seriesNum));

If I change it to this:

        double ux = plot.domainToScreenX(dataX, seriesNum);

... then the clipping appears to work properly, just as it does when x values 
fall outside the layer on the right-
hand side.

Original comment by chadtaka...@gmail.com on 30 Jul 2008 at 6:09

GoogleCodeExporter commented 8 years ago
Well gosh darn, I totally missed that line. I've been looking at this codebase 
too much that my eyes tend to glaze 
over. I don't remember why that max is there, I believe early on in the 
development of Chronoscope, there were 
crashes on Safari 2 and/or Firefox, and I may have stuck that in there to 
prevent them. 

I'd say let's remove it, and check if it works in Safari3, IE6, Firefox2/3, and 
Opera, and if so, then commit that as 
the fix.  You can check if it works in IE/Flash by adding ?_force_flash to the 
URL.

Original comment by cromwell...@gmail.com on 30 Jul 2008 at 7:10

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Resolved in r250.  Tested in Safari3, Firefox2/3, Opera, and IE.

Original comment by chadtaka...@gmail.com on 30 Jul 2008 at 9:36