Graphics: adaptive calculation of quadratic curve and arc subdivision

[icosaeder]

Evaluating PIXI (great engine indeed!) I found that the curves have constant subdivision ratio. This produces jagged edges if the curves are long enough:
https://www.dropbox.com/s/y9hxkkhjbtpkgsm/pixi_curves_old.png?dl=0
So I suggest calculating the subdivision based on the curves' length:
https://www.dropbox.com/s/h1dx5smaxp7inzl/pixi_curves_new.png?dl=0

[ivanpopelyshev]

Adaptive rasterizing is good, but that change breaks previous behaviour. I approve it either other approve either if something like PIXI.settings.graphicsAdaptCurve is set.

[ivanpopelyshev]

Also it has max segments lengths constant. Lets see if @GoodBoyDigital and @bigtimebuddy agree to changes in current form.

[cursedcoder]

this can be tagged by @see http://usejsdoc.org/tags-see.html

[cursedcoder]

Big +1

As for BC I'm not sure it applies here, so we should be able to drop it for next patch/minor version.

6. Patch version Z (x.y.Z | x > 0) MUST be incremented if only backwards compatible bug fixes are introduced. A bug fix is defined as an internal change that fixes incorrect behavior.

7. Minor version Y (x.Y.z | x > 0) MUST be incremented if new, backwards compatible functionality is introduced to the public API. It MUST be incremented if any public API functionality is marked as deprecated. It MAY be incremented if substantial new functionality or improvements are introduced within the private code. It MAY include patch level changes. Patch version MUST be reset to 0 when minor version is incremented.

See https://semver.org/#spec-item-6

[bigtimebuddy]

Could you explain more about these magic numbers 10 and clamping to 8 and 2048? What's the rationale? Are there scenarios where you'd want to alter these?

[ivanpopelyshev]

I say just put those numbers and behaviour switch in constant and document it properly , and then merge. Set behaviour by default in next major version.

[icosaeder]

These constants were selected experimentally. 10 is the length of a single segment. It seems to give good looking results on the curves of average length. 8 seems to be enough for very short curves. 2048 is just some "bug number" limiting the maximal number of segments. But you right, there could be some corner cases, when these constants won't be adequate. So I agree, the setting for this is the best choice. I fix it soon.

[ivanpopelyshev]

Btw, currently, all vector things in pixi work for scale=1. Transform isnt taken into account in rasterizing, and for now its ok.

[OSUblake]

@icosaeder Don't forget about the regular bezierCurveTo method. It also uses a fixed number of segments.

Another way to get number of segments using hyperbola.
http://ciechanowski.me/blog/2014/02/18/drawing-bezier-curves/

function tesselationSegmentsForLength(length) {
  const segs = length / segmentLength;
  return Math.ceil(Math.sqrt(segs * segs * 0.6 + minSegmentNumber * minSegmentNumber));
}

[GoodBoyDigital]

I like it! Nice work @icosaeder 👍

[GoodBoyDigital]

@OSUblake makes a good point! @icosaeder, Thanks for your efforts so far, think you could round this PR off by adding similar functionality to bezierCurveTo?

[icosaeder]

Thank you! :)
Sure, I take a look at bezierCurveTo.

[bigtimebuddy]

hey @icosaeder, any progress on this with bezierCurveTo?

[icosaeder]

Hi! I'm very sorry for the delay, the last couple of weeks I had some urgent things to do. But now I pushed the bezierCurveTo improvement.

[bigtimebuddy]

I moved the settings to Graphics.CURVES object instead of creating new setting properties. I also tweaked the property names slightly for brevity and clarity.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.