toomuchdesign/react-minimal-pie-chart

Tiny slices are not rendered correctly on Chrome

Closed this issue · 8 comments

xumi commented

Do you want to request a feature or report a bug?

This is a rendering bug

What is the current behaviour?

When rendering a lot of really small slices weird artefacts are drawn

Screenshot 2020-03-25 at 19 27 41

What is the expected behaviour?

No artefacts, maybe provide an option to ignore/hide super small slice (below a ceiling % value)

Steps to Reproduce the Problem

<ReactPieChart data={ [{"value":2421,"title":"a","color":"rgb(33, 150, 243)"},{"value":2413,"title":"a","color":"rgb(34, 150, 243)"},{"value":2403,"title":"a","color":"rgb(34, 151, 243)"},{"value":2399,"title":"a","color":"rgb(34, 151, 243)"},{"value":2395,"title":"a","color":"rgb(35, 151, 243)"},{"value":2387,"title":"a","color":"rgb(35, 151, 243)"},{"value":2383,"title":"a","color":"rgb(35, 151, 243)"},{"value":2380,"title":"a","color":"rgb(36, 151, 243)"},{"value":2373,"title":"a","color":"rgb(36, 151, 243)"},{"value":2373,"title":"a","color":"rgb(36, 151, 243)"},{"value":2360,"title":"a","color":"rgb(37, 152, 243)"},{"value":2359,"title":"a","color":"rgb(37, 152, 243)"},{"value":2354,"title":"a","color":"rgb(37, 152, 243)"},{"value":2349,"title":"a","color":"rgb(38, 152, 243)"},{"value":2345,"title":"a","color":"rgb(38, 152, 243)"},{"value":2312,"title":"a","color":"rgb(40, 153, 243)"},{"value":136,"title":"a","color":"rgb(178, 218, 251)"},{"value":136,"title":"a","color":"rgb(178, 218, 251)"},{"value":103,"title":"a","color":"rgb(181, 219, 251)"},{"value":101,"title":"a","color":"rgb(181, 219, 251)"},{"value":101,"title":"a","color":"rgb(181, 219, 251)"},{"value":101,"title":"a","color":"rgb(181, 219, 251)"},{"value":34,"title":"a","color":"rgb(185, 221, 251)"},{"value":34,"title":"a","color":"rgb(185, 221, 251)"},{"value":34,"title":"a","color":"rgb(185, 221, 251)"},{"value":33,"title":"a","color":"rgb(185, 221, 251)"},{"value":33,"title":"a","color":"rgb(185, 221, 251)"},{"value":33,"title":"a","color":"rgb(185, 221, 251)"},{"value":33,"title":"a","color":"rgb(185, 221, 251)"},{"value":33,"title":"a","color":"rgb(185, 221, 251)"},{"value":33,"title":"a","color":"rgb(185, 221, 251)"},{"value":32,"title":"a","color":"rgb(185, 221, 251)"},{"value":32,"title":"a","color":"rgb(185, 221, 251)"},{"value":8,"title":"a","color":"rgb(187, 222, 251)"},{"value":7,"title":"a","color":"rgb(187, 222, 251)"},{"value":7,"title":"a","color":"rgb(187, 222, 251)"},{"value":5,"title":"a","color":"rgb(187, 222, 251)"},{"value":4,"title":"a","color":"rgb(187, 222, 251)"},{"value":2,"title":"a","color":"rgb(187, 222, 251)"},{"value":2,"title":"a","color":"rgb(187, 222, 251)"},{"value":2,"title":"a","color":"rgb(187, 222, 251)"},{"value":2,"title":"a","color":"rgb(187, 222, 251)"},{"value":2,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"},{"value":1,"title":"a","color":"rgb(187, 222, 251)"}] } />

Suggested fix

Rounding the percent seems to be fixing the issue.
Line 266
const valueInPercentage = total === 0 ? 0 : ( dataEntry.value / total * 100 ).toFixed();

Specifications

  • Version: 6.0.1 (latest available
  • Platform: OSX Mojave, Chrome (80.0.3987.132)

Hi @xumi,
thanks for reporting. A quick solution could of course involve filtering the entries in userland code.

On the long term, the solution you suggested about introducing a new prop to provide a minimum degree/percentage threshold for the segments to be rendered could definitely work.

PS.
The bug is the result of a very corner edge implementation issue of SVG paths in browsers. In my tests the unexpected artefact is generated by a single slice when it's percentage is below 0.0X%.

xumi commented

Hey @toomuchdesign, thanks for the quick reply.

Yes, this could definitely be made before sending the data to react-minimal-pie-chart but it would duplicate the logic (computing the total, then the percent per value).
Since I really care about the performance of my application (I'm displaying dozens of pie charts at the same time, with hundreds of values per chart) it would be beneficial to only do it once, in the component, to keep my userland code dry.

Are you open to a pull request if I create one? Thanks again !

PR's are always welcome :)

What kind of API shall we provide? I though that providing a percent (or degree) threshold would be a bit too limited to this use case.

What if we provided a less opinionated optional filterSegments function (the name is totally improvable) receiving dataEntry and index as arguments that would come into play about here?

(dataEntry: ExtendedDataEntry, index: number) => boolean

This is just the idea I had this morning. What do you think?

P.S. If you open a PR don't forget to add relevant tests 🚀

@xumi and I considered the possibility of introducing a filterData prop to avoid iterating twice over data once in user land code to filter out small entries and once in the internal call to extendData.

We found out that this would lead a double iteration over the data set anyway since we would need to call extendData twice: once with the original data set (to evaluate entries' percentage/degrees values) and once afterwards with the filtered data set (to update the previously evaluated percentage/degrees values).

The only benefit would consist in using the library implementation to evaluate extendedData values.

I'll keep the PR open to collect possible further comments/opinions.

Hey @xumi ,
I just found a silly workaround to this weird Chrome "crazy slice" bug.

I noticed that a slight adjustment of startAngle property can prevent the bug from actually happening.

In this case I set startAngle to 0.0003, anything could work, it's just a magic number probably depending on the provided data.

Here is the relative sandbox.

xumi commented

Thanks for the follow up @toomuchdesign! I'll look into it when I'm back on the project! Cheers

Hi @xumi,
does this Chrome bug still occur? I can't reproduce it anymore. Maybe Chrome engine was fixed?

Update: yes, it still occurs :)

Chromium engine seems to have consistently improved SVG rendering. I can't reproduce the issue anymore on Chrome v106, macOS.