Skip to content

Conversation

@YawarOsman
Copy link

What’s New?

  • Added borderRadius capability to the PieChartPainter for enhanced customization.
  • Ensured existing styles and functionality remain intact.

Why?

To provide developers with more flexibility in customizing the appearance of pie chart segments.

Testing

  • Verified various borderRadius values.
  • Checked for visual regressions and ensured backward compatibility.

FlBorderData? borderData,
required this.touchData,
}) : borderData = borderData ?? FlBorderData();
BorderRadius? borderRadius,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to have borderRadius property here in our base chart data. (So we don't need to have it for all the charts. And if we do, we should have it around the chart. Not something that only applies for the pie section paths)
So please remove it from here and just add it inside the PieChartSectionData

Rect bounds = sectionPath.getBounds();
Path roundedPath = Path()
..addRRect(
RRect.fromRectAndCorners(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic doesn't seem right to me.
And when I applied borderRadius: const BorderRadius.all(Radius.circular(200)), to our PieChartSample1, I saw this:
image

It only works with small values like 8, 16, ...
So it is not stable I would say

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the better idea is just to edit the shape path generation logic here:

var sectionPath = Path()
..moveTo(startLine.from.dx, startLine.from.dy)
..lineTo(startLine.to.dx, startLine.to.dy)
..arcTo(sectionRadiusRect, startRadians, sweepRadians, false)
..lineTo(endLine.from.dx, endLine.from.dy)
..arcTo(centerRadiusRect, endRadians, -sweepRadians, false)
..moveTo(startLine.from.dx, startLine.from.dy)
..close();

We might be able to use arcTo, conicTo or cubicTo in Path

@YawarOsman
Copy link
Author

You're right, sorry for that inconsistency

@imaNNeo
Copy link
Owner

imaNNeo commented Oct 25, 2025

No worries!
There's a new PR for this topic with a new implementation.
So I'll close this one for now.
Thanks a lot for your contribution and idea!

Please follow it here: #2024

@imaNNeo imaNNeo closed this Oct 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants