-
Notifications
You must be signed in to change notification settings - Fork 21
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
Move scale range calculation from compiler to the renderer. #48
Comments
Hi, @tdhock, @faizan-khan-iit, Please let me know how do I proceed with this issue. |
Hi @lazycipher, regarding this issue, you will first need a little bit of understanding of how we compute the scale ranges to render the axes for different subsets of data. As an overview, we essentially compute the scale ranges that a data subset would fit in for all possible (not really, that's the issue here) data subsets. When a user is viewing a data subset, we select the pre-saved range for that subset and change the axes accordingly <- this provides a zoom in effect where the user sees the data in the whole plot area. Here is an example where we always take the full plot area in the right plot. So every time we select a new country, we use the y-axis scale which was pre-computed in the compiler and render that in the plot. For more details, I highly recommend going through this PR where we implemented the feature. If you do not understand some comments you can just skip them for now or ping me and I will elaborate the important points. The issue has been discussed here: tdhock/animint#158 (comment) Hope this helps! |
there is a javascript function in https://github.com/tdhock/animint2/blob/master/inst/htmljs/animint.js called update_scales which I think needs to be modified. |
Hi @tdhock @faizan-khan-iit , I started to look at the code for computing the domain from last week, As I understand it, the logic for
On the renderer's end (within the JS code) at update_scales(), the renderer would receive the subset data(the .tsv files) from the compiler, with other associated meta-data, like the saved ranges from plot.json. I read through the original PR you mentioned above, it seems you're suggesting shifting the |
right! |
I'm looking into the for loop here to see if we can move all of its variables over to the JS side. I have some problem coming up with the implementation because it looks like not all ggplot info in the environment is making its way to plot.json. For example, Lines 493 to 494 in 12b57c9
compute_domains .
From what I can tell, Lines 331 to 332 in 12b57c9
g.list comes from Geom$export_animint() Lines 368 to 376 in 12b57c9
compute_domains input as well.
I think I can get most of these parameters into plot.json so that we can work with them in JS. But I'm not sure what to do about data that only exists on the compiler side, like layers$data. Right now, animint2 only exports the bare minimum (.tsv files) for rendering the viz. Should we consider exporting the entire dataset? |
I'm not sure I understand what you are asking, can you please clarify? |
Thanks for clarifying how it works! I think I was focusing too much on directly converting the R function into JS, rather than focusing on the functionality itself. You're right, we can definitely calculate scales/domains with the data being exported. I'll go ahead and open a PR to start the refactor. |
Currently, The computations of updating of axes/legends after changing the currently displayed data subset are done in the compiler but there are some limitations, so it will be good to move the computations to the renderer.
We sometimes only have a subset of the whole data while computing the scales in the compiler so we are limited in our implementation.
This can be solved by moving the computation to the renderer so that we have the whole data to compute new scale values.
The text was updated successfully, but these errors were encountered: