arrayfire / forge

High Performance Visualization
226 stars 48 forks source link

[WIP] Pie chart #204

Open tusharpm opened 5 years ago

tusharpm commented 5 years ago

What kind of change does this PR introduce? Feature.

What is the current behavior?

68 lists pie chart as an option for statistical plots.

What is the new behavior? Add Pie chart.

Does this PR ensure that key events handling across different window toolkits is identical if it has changes related to window toolkits? N/A.

Does this PR introduce a breaking change? No. Pie API is purely additive.

Please check if the PR fulfills these requirements

Other information: WIP: The branch includes a CPU example - need to write the same for CUDA/OpenCL. Need to find a formal way to disable grid and ticks for the chart object. The API has the doc comments (similar to histogram). Need to update README.md to reflect Pie chart support.

9prady9 commented 5 years ago

@tusharpm Looks good, yeah, right now there is no way to disable rendering of ticks and grid lines. But I can add those, shouldn't be hard - just want to make sure it is done the right way.

9prady9 commented 5 years ago

I will comment on changes once I look at the shader etc.

tusharpm commented 5 years ago

For the pie %, I'm not sure about a GLSL way of rendering text. (I'll try looking that up). Alternatively, it might need copying the vertex buffer contents locally to process in pie_impl.cpp.

9prady9 commented 5 years ago

perhaps something like examples/cpu/plotting.cpp does - put legend for each sector in respective color. I will check what are our options there. If you have ideas, please do share.

9prady9 commented 5 years ago

Percentage That is what I have in mind at the moment, anything more like the following Percentage_Ideally

would take more time but makes %'s and labels conspicuous. I think there is disadvantage to the second approach in some cases where larger number of labels and screen real estate becomes cluttered. In this case, the first approach also has an issue of how many labels we can fit in vertical direction while having a legible font size.

9prady9 commented 5 years ago

I have created the https://github.com/arrayfire/forge/pull/205 with necessary changes

You can apply the following diff on your PR to rendering pie without axes.

diff --git a/src/api/c/chart.cpp b/src/api/c/chart.cpp
index e63af51..d21ca75 100644
--- a/src/api/c/chart.cpp
+++ b/src/api/c/chart.cpp
@@ -180,6 +180,7 @@ fg_err fg_append_pie_to_chart(fg_chart pChart, fg_pie pPie)
         ARG_ASSERT(1, (pPie!=0));

         getChart(pChart)->addRenderable(getPie(pPie)->impl());
+        getChart(pChart)->setAxesVisibility(false);
     }
     CATCHALL

@@ -279,6 +280,7 @@ fg_err fg_add_pie_to_chart(fg_pie* pPie, fg_chart pChart,

         common::Pie* pie = new common::Pie(pNSectors, (forge::dtype)pType);
         chrt->addRenderable(pie->impl());
+        chrt->setAxesVisibility(false);
         *pPie = getHandle(pie);
     }
     CATCHALL
9prady9 commented 5 years ago

@tusharpm Great, I see that you have disabled axes rendering now. Sorry, I didn't get notification for that push, so I was under the impression you are still working on it.

What are your thoughts on legends ?

tusharpm commented 5 years ago

I had actually missed the notification for the PR you merged earlier. I'm sorry for the confusion.

I haven't looked into legends, yet. The first image from your comment seems closer to the legends used in plotting.cpp. Calculating and rendering the list elements would require reading data from the vertex and color buffers. Can you share your opinion/suggestions for that approach?

9prady9 commented 5 years ago

@tusharpm Sorry about the delay, I will think about possible ideas, but as of now, I can only think of it to get the lists of texts(legends) and percentages from user.

The legends from plotting are slightly different because each legend is set individually by each item of 2d chat. However, in the case of pie, we need to push all these into a single item, thus would involve a vector of strings or something like that.

Percentages will be a tricky one for sure. Let me take a look this PR changes once again. I will update here if I have something new.

You can also ping me on slack - https://arrayfire-org.slack.com/messages/C6QTLM3M0 for a real time discussion.

Thank you.

9prady9 commented 4 years ago

I have rebased it from latest master and added remove API for pie renderables too.

9prady9 commented 6 months ago

I will work on this some time in 2024 to revive this feature. So will make this into draft up until the point.