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

RangedTwoDimensionalFilter is backwards on the y-dimension #1476

Closed
braaannigan opened this issue Aug 21, 2018 · 3 comments
Closed

RangedTwoDimensionalFilter is backwards on the y-dimension #1476

braaannigan opened this issue Aug 21, 2018 · 3 comments

Comments

@braaannigan
Copy link

Hi - thanks for all your great work on this project!

This is a bug in dc 3.0.6

When using the RangedTwoDimensionalFilter on scatter plots the coordinates of the brush values should be increasing from the bottom left to top right i.e. [ [x1,y1], [x2,y2] ] where x1 < x2 and y1 < y2. However, the y-values are the wrong way around with y1 larger than y2.

This doesn't affect the scatter-brushing because the current code does a max/min call on each pair:

fromBottomLeft = [
            [Math.min(filter[0][0], filter[1][0]), Math.min(filter[0][1], filter[1][1])],
            [Math.max(filter[0][0], filter[1][0]), Math.max(filter[0][1], filter[1][1])]
        ]

so the reversal of the y order isn't apparent. If the arrays were in the right order then the fromBottomLeft array could simply be assigned without min() and max() - perhaps the .min() and .max() calls were introduced because of the reversal of the y-values?

I've been able to trace back the problem to around line 11099 in dc.js:

if (brushSelection) {
            brushSelection = brushSelection.map(function (point) {
                return point.map(function (coord, i) {
                    var scale = i === 0 ? _chart.x() : _chart.y();
                    return scale.invert(coord);
                });
            });

At this point the relative values given by scale.invert(coord) is reversed for the x and y coordinates. I'm not sure where the invert method originates from, I've got a bit lost in the d3 side. Any suggestions?

I came upon this because I was adding a custom filterHandler onto a scatterplot. In this case you need to access the values in the 2d ranged filter. I'd be happy to open a PR on this if anyone has any guidance.

Thanks for any advice you can offer.

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented Sep 17, 2018

Hi @braaannigan, it looks like d3.brush provides coordinates which always have y1 <= y2, but when we use scale.invert to convert from screen coordinates to data coordinates, that reverses the Y values.

In general, SVG always uses screen coordinates where 0,0 is the upper left, but charts almost always have 0,0 at the lower left.

I think the code is behaving as documented, since the RangedToDimensionFilter documentation states that

It takes two two-dimensional points in the form [[x1,y1],[x2,y2]], and normalizes them so that x1 <= x2 and y1 <= y2.

However, you're right that this is pretty confusing behavior. I'd be amenable to a fix. You would have to swap brushSelection[0][1] and brushSelection[1][1] in src/scatter-plot.js after the map/invert calls you show above.

@braaannigan
Copy link
Author

Hi @gordonwoodhull thanks for your reply. Life has intervened quite dramatically since I posted the issue, so I'm not going to have time in the near future to do a PR. Given that the code works and the issue is documented here, suggest we just close the issue?

@gordonwoodhull
Copy link
Contributor

Hi @braaannigan. I agree, the behavior is a little confusing but it also works as documented, so let's close this. Thanks for the report!

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

No branches or pull requests

2 participants