react-minimal-pie-chart icon indicating copy to clipboard operation
react-minimal-pie-chart copied to clipboard

Tiny slices are not rendered correctly on Chrome

Open xumi opened this issue 4 years ago • 7 comments

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)

xumi avatar Mar 25 '20 18:03 xumi

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%.

toomuchdesign avatar Mar 25 '20 21:03 toomuchdesign

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 !

xumi avatar Mar 26 '20 09:03 xumi

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 🚀

toomuchdesign avatar Mar 26 '20 10:03 toomuchdesign

@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.

toomuchdesign avatar Mar 30 '20 16:03 toomuchdesign

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.

toomuchdesign avatar May 19 '20 08:05 toomuchdesign

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

xumi avatar May 25 '20 08:05 xumi

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

Update: yes, it still occurs :)

toomuchdesign avatar Nov 05 '20 13:11 toomuchdesign

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

toomuchdesign avatar Oct 30 '22 17:10 toomuchdesign