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

BUG #1811: Show point information for plots using geographic projecti… #1878

Merged
merged 2 commits into from Mar 21, 2016

Conversation

@danlipsa
Copy link
Contributor

@danlipsa danlipsa commented Mar 15, 2016

…ons.

@danlipsa
Copy link
Contributor Author

@danlipsa danlipsa commented Mar 15, 2016

@danlipsa
Copy link
Contributor Author

@danlipsa danlipsa commented Mar 15, 2016

@aashish24 @doutriaux1 Please review. Picking works for boxfill and meshfill. We can make it work for all plots after #1881

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Mar 17, 2016

@danlipsa why would this baseline changed? test_vcs_settings_color_name_rgba_meshfill.png

I = d.array[0].getAxis(-1).mapInterval((X, X, 'cob'))[0]
# for non-linear projection or for meshfill. Meshfill is wrapped at
# VTK level, so vcs calculations do not work.
if gm.projection != "linear" or gm.g_name == 'Gfm':
Copy link
Contributor

@aashish24 aashish24 Mar 17, 2016

@danlipsa why not use the same code (under if) for both cases?

Copy link
Contributor Author

@danlipsa danlipsa Mar 17, 2016

Because this code does not work now for isofill, isoline and vectors. When we fix dataset creation we can get rid of the vcs code.

Copy link
Contributor

@aashish24 aashish24 Mar 17, 2016

@danlipsa thanks. And I guess we have an issue for it?

Copy link
Contributor Author

@danlipsa danlipsa Mar 17, 2016

Yes we do:
#1881

Copy link
Contributor

@aashish24 aashish24 Mar 17, 2016

thanks @danlipsa

@danlipsa
Copy link
Contributor Author

@danlipsa danlipsa commented Mar 17, 2016

@aashish24 I think has to do with the fact that now I use one renderer for all mappers. Meshfill and boxfill use several mappers one for each range of scalar values. I assume using only one renderer changed the order in which those mappers are rendered. We can file an issue fix this many mappers problem.

@@ -507,6 +507,10 @@ def genGrid(data1, data2, gm, deep=True, grid=None, geo=None):
vg.SetPoints(geopts)
else:
vg = grid
# Add a GlobalIds array to keep track of cell ids throughout the pipeline
globalIds = numpy_to_vtk_wrapper(numpy.arange(0, vg.GetNumberOfCells()), deep=True)
Copy link
Contributor

@aashish24 aashish24 Mar 17, 2016

@danlipsa why we have to do this? Meaning why we have to explicitly append the global ids?

Copy link
Contributor Author

@danlipsa danlipsa Mar 17, 2016

Because the hardware selector renders and gets the cell ids of the geometry which are not usually the same as the cell ids of the dataset.

Copy link
Contributor

@aashish24 aashish24 Mar 17, 2016

Because the hardware selector renders and gets the cell ids of the geometry which are not usually the same as the cell ids of the dataset.

this is interesting. But by default should they be same?

Copy link
Contributor Author

@danlipsa danlipsa Mar 17, 2016

What do you mean 'by default'? In the uvcdat case we have threshold + different mapper for different scalar ranges so the ids are not the dataset ids. If you have only the dataset in the pipeline I think only the polydata will not change ids. Maybe there are others but I would not bet on it :-)

Copy link
Contributor

@aashish24 aashish24 Mar 17, 2016

If you have only the dataset in the pipeline I think only the polydata will not change ids. Maybe there are others but I would not bet on it :-)

I see. yeah, I won't either.

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Mar 17, 2016

@aashish24 I think has to do with the fact that now I use one renderer for all mappers. Meshfill and boxfill use several mappers one for each range of scalar values. I assume using only one renderer changed the order in which those mappers are rendered. We can file an issue fix this many mappers problem.

Sure, if you can create an issue for it (if it does not exists already), that would be great.

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Mar 17, 2016

@danlipsa other than some of the questions I have, this branch looks good to me. I did manually compiled the branch and use the GUI for testing. Looks good to me 👍

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Mar 17, 2016

@danlipsa the only thing I am not clear about is point data scalars other than that I think you covered it pretty well.

@danlipsa
Copy link
Contributor Author

@danlipsa danlipsa commented Mar 17, 2016

So, for point data, the hardware selector renders the points using pointids instead of color. So we can get the pointid and from it we can get the point scalar. The only question I have is how close do you have to click on the point to get the point id and how user friendly is that. We might be able to set some sort of point size/range so that you don't have to click exactly on the point to get its value.

@danlipsa
Copy link
Contributor Author

@danlipsa danlipsa commented Mar 17, 2016

Sure, if you can create an issue for it (if it does not exists already), that would be great.
See:
#1884

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Mar 17, 2016

Sure, if you can create an issue for it (if it does not exists already), that would be great.
See:
#1884

thanks @danlipsa

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Mar 18, 2016

So, for point data, the hardware selector renders the points using pointids instead of color. So we can get the pointid and from it we can get the point scalar. The only question I have is how close do you have to click on the point to get the point id and how user friendly is that. We might be able to set some sort of point size/range so that you don't have to click exactly on the point to get its value.

Sure. I guess we should create an issue for it as well.

@danlipsa
Copy link
Contributor Author

@danlipsa danlipsa commented Mar 18, 2016

@doutriaux1 I think this is ready to go. :-)

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Mar 21, 2016

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Mar 21, 2016

@doutriaux1 please approve.

@doutriaux1
Copy link
Contributor

@doutriaux1 doutriaux1 commented Mar 21, 2016

just finished building. Testing the click and approving

@doutriaux1
Copy link
Contributor

@doutriaux1 doutriaux1 commented Mar 21, 2016

@danlipsa so it does not work on isofill. Correct

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Mar 21, 2016

@doutriaux1 right, it does not workon isofill (#1885)

doutriaux1 added a commit that referenced this issue Mar 21, 2016
BUG #1811: Show point information for plots using geographic projecti…
@doutriaux1 doutriaux1 merged commit eb22e0b into master Mar 21, 2016
3 checks passed
@doutriaux1
Copy link
Contributor

@doutriaux1 doutriaux1 commented Mar 21, 2016

Ok approving this for now then.

@doutriaux1 doutriaux1 deleted the fix_projected_click branch Mar 21, 2016
@danlipsa
Copy link
Contributor Author

@danlipsa danlipsa commented Mar 21, 2016

@doutriaux1 Make sure you merge the data branch as well: CDAT/uvcdat-testdata#117

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants