Fix offset on doughnut charts

[Zivangu9]

Fix #10465

Adding another half offset can fix the cropped graphics error.

Reproducible sample

I use the examples that doc-code-hub gave in the issue
https://stackblitz.com/edit/web-platform-dr6rsv?file=js%2Fdata.js,index.html

[etimberg]

How was 1.5 determined here?

[Zivangu9]

Each arc is offset from center 1/4 offset

Chart.js/src/elements/element.arc.js

Line 324 in 7432b60

const offset = (options.offset || 0) / 2;

Chart.js/src/elements/element.arc.js

Line 337 in 7432b60

radiusOffset = offset / 2;

Chart.js/src/elements/element.arc.js

Line 339 in 7432b60

ctx.translate(Math.cos(halfAngle) * radiusOffset, Math.sin(halfAngle) * radiusOffset);

Except when the arc measures more than 180 degrees, in that case is 1/2 offset
And this is the cause of the problem, if there is an arc greater than 180 the offset is increased by an extra 1/2

Chart.js/src/elements/element.arc.js

Lines 340 to 342 in 7432b60

	if (this.circumference >= PI) {
	radiusOffset = offset;
	}

In addition to the separation from the center to the radius of the arc, the radiusOffset is increased again

Chart.js/src/elements/element.arc.js

Line 348 in 7432b60

const endAngle = drawArc(ctx, this, radiusOffset, spacing);

I just saw that if there are 2 arcs of 180 the offset would be doubled

[Zivangu9]

Do you think the solution is correct, or should I look for a different one?

[etimberg]

@Zivangu9 sorry, I didn't have internet yesterday

I think I understand where this is coming from. Would it be possible to add a test for the 2 arcs that are each 180 degrees? I'd be curious to see how this performs

[Zivangu9]

I was testing with two 180 degree arcs and now I understand a little better.
The center offset is always 1/4, but the radius increment is 1/2 for 180 degree arcs, so when it's 2 180 arcs it's just 1.5 offset. When it is only one arc greater than 180 it becomes 1.25. And if there are no arcs greater than 180, the increment is 1 offset.
https://stackblitz.com/edit/web-platform-dr6rsv?file=js%2Fdata.js,index.html

[etimberg]

It almost looks like the angle changes should only apply to arcs that are not 180 degrees exactly.

[kurkle]

There is a jump at the end of the animation, when the arcs reach 180 degree circumference. That is another problem.

I think a proper fix would be getting rid of this kind of condition in the arc element.

[kurkle]

I experimented a little with it.
Replaced

Chart.js/src/elements/element.arc.js

Lines 340 to 342 in 7432b60

	if (this.circumference >= PI) {
	radiusOffset = offset;
	}

with

      const fix = 1 + Math.sin(-Math.min(PI, circumference));
      radiusOffset = offset / 2 * fix;

to make the animations smooth for large arcs. @etimberg any thoughts?

https://stackblitz.com/edit/web-platform-ayr13m?file=js%2Fdata.js

[etimberg]

I experimented a little with it. Replaced

Chart.js/src/elements/element.arc.js

Lines 340 to 342 in 7432b60

	if (this.circumference >= PI) {
	radiusOffset = offset;
	}

with

      const fix = 1 + Math.sin(-Math.min(PI, circumference));
      radiusOffset = offset / 2 * fix;

to make the animations smooth for large arcs. @etimberg any thoughts?

https://stackblitz.com/edit/web-platform-ayr13m?file=js%2Fdata.js

The animation in that version looks really good 👍

[Zivangu9]

That seems like a better solution to the problem, do I make the changes in the PR?

[stockiNail]

sorry if I'm jumping in the thread. Issue #10220 was addressing the issue about hover offset (similar of the offset one).
I didn't check the PR in deep, but I was thinking if make sense to address hoverOffset option and not only the offset one.

[kurkle]

@stockiNail probably the same issue.

[LeeLenaleee]

Since this can drastically change how some charts are shown I am not sure if this should be considered a bug fix or a breaking change

[etimberg]

Hmm, not sure if this is breaking or not. The previous behaviour was pretty buggy.

[LeeLenaleee]

That is true, but since we want to start working on V4 after 3.9 I think it will be the safer option to postpone it till V4

[etimberg]

Fair, if we aim to get a 3.9 out quickly this won't have to sit around for a while