Skip to content

Commit

Permalink
Curves: Clean up.
Browse files Browse the repository at this point in the history
  • Loading branch information
@mrdoob
mrdoob committed Feb 16, 2021
1 parent a36141f commit 330b186
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 12 deletions.
3 changes: 3 additions & 0 deletions src/extras/curves/CatmullRomCurve3.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ class CatmullRomCurve3 extends Curve {
return point;

}

copy( source ) {

super.copy( source );
Expand All @@ -202,6 +203,7 @@ class CatmullRomCurve3 extends Curve {
return this;

}

toJSON() {

const data = super.toJSON();
Expand All @@ -222,6 +224,7 @@ class CatmullRomCurve3 extends Curve {
return data;

}

fromJSON( json ) {

super.fromJSON( json );
Expand Down
3 changes: 3 additions & 0 deletions src/extras/curves/CubicBezierCurve.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class CubicBezierCurve extends Curve {
return point;

}

copy( source ) {

super.copy( source );
Expand All @@ -44,6 +45,7 @@ class CubicBezierCurve extends Curve {
return this;

}

toJSON() {

const data = super.toJSON();
Expand All @@ -56,6 +58,7 @@ class CubicBezierCurve extends Curve {
return data;

}

fromJSON( json ) {

super.fromJSON( json );
Expand Down
4 changes: 4 additions & 0 deletions src/extras/curves/CubicBezierCurve3.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class CubicBezierCurve3 extends Curve {
this.v3 = v3;

}

getPoint( t, optionalTarget = new Vector3() ) {

const point = optionalTarget;
Expand All @@ -32,6 +33,7 @@ class CubicBezierCurve3 extends Curve {
return point;

}

copy( source ) {

super.copy( source );
Expand All @@ -44,6 +46,7 @@ class CubicBezierCurve3 extends Curve {
return this;

}

toJSON() {

const data = super.toJSON();
Expand All @@ -56,6 +59,7 @@ class CubicBezierCurve3 extends Curve {
return data;

}

fromJSON( json ) {

super.fromJSON( json );
Expand Down
22 changes: 13 additions & 9 deletions src/extras/curves/EllipseCurve.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,28 @@ import { Vector2 } from '../../math/Vector2.js';

class EllipseCurve extends Curve {

constructor( aX, aY, xRadius, yRadius, aStartAngle, aEndAngle, aClockwise, aRotation ) {
constructor( aX = 0, aY = 0, xRadius = 1, yRadius = 1, aStartAngle = 0, aEndAngle = Math.PI * 2, aClockwise = false, aRotation = 0 ) {

super();

this.type = 'EllipseCurve';
this.isEllipseCurve = true;

this.aX = aX || 0;
this.aY = aY || 0;

This comment has been minimized.

Sign in to view

Copy link
@Mugen87

Mugen87 Mar 15, 2021 •
edited
Loading

Collaborator

This actually introduced a regression. Or more precisely revealed an issue in SVGLoader.

When NaN was passed to the ctor, NaN || 0 evaluated to 0. Now the code actually uses NaN. This breaks the Defs and Defs2 examples in webgl_loader_svg.

This comment has been minimized.

Sign in to view

Copy link
@Mugen87

Mugen87 Mar 15, 2021 •
edited
Loading

Collaborator

I guess we have to make sure that SVGLoader uses a default value (e.g. 0) when getAttribute() returns null. In this way NaN values can be avoided.

This comment has been minimized.

Sign in to view

Copy link
@yomboprime

yomboprime Mar 16, 2021

Collaborator

Should it be sufficient to put a warning in SVGLoader.parseFloatWithUnits() in case the parsed value is NaN?

This comment has been minimized.

Sign in to view

Copy link
@Mugen87

Mugen87 Mar 16, 2021

Collaborator

see #21469
this.aX = aX;
this.aY = aY;

this.xRadius = xRadius || 1;
this.yRadius = yRadius || 1;
this.xRadius = xRadius;
this.yRadius = yRadius;

this.aStartAngle = aStartAngle || 0;
this.aEndAngle = aEndAngle || 2 * Math.PI;
this.aStartAngle = aStartAngle;
this.aEndAngle = aEndAngle;

this.aClockwise = aClockwise || false;
this.aClockwise = aClockwise;

this.aRotation = aRotation || 0;
this.aRotation = aRotation;

}

getPoint( t, optionalTarget ) {

const point = optionalTarget || new Vector2();
Expand Down Expand Up @@ -85,6 +86,7 @@ class EllipseCurve extends Curve {
return point.set( x, y );

}

copy( source ) {

super.copy( source );
Expand All @@ -105,6 +107,7 @@ class EllipseCurve extends Curve {
return this;

}

toJSON() {

const data = super.toJSON();
Expand All @@ -125,6 +128,7 @@ class EllipseCurve extends Curve {
return data;

}

fromJSON( json ) {

super.fromJSON( json );
Expand Down
6 changes: 6 additions & 0 deletions src/extras/curves/LineCurve.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class LineCurve extends Curve {
this.v2 = v2;

}

getPoint( t, optionalTarget = new Vector2() ) {

const point = optionalTarget;
Expand All @@ -32,12 +33,14 @@ class LineCurve extends Curve {
return point;

}

// Line curve is linear, so we can overwrite default getPointAt
getPointAt( u, optionalTarget ) {

return this.getPoint( u, optionalTarget );

}

getTangent( t, optionalTarget ) {

const tangent = optionalTarget || new Vector2();
Expand All @@ -47,6 +50,7 @@ class LineCurve extends Curve {
return tangent;

}

copy( source ) {

super.copy( source );
Expand All @@ -57,6 +61,7 @@ class LineCurve extends Curve {
return this;

}

toJSON() {

const data = super.toJSON();
Expand All @@ -67,6 +72,7 @@ class LineCurve extends Curve {
return data;

}

fromJSON( json ) {

super.fromJSON( json );
Expand Down
4 changes: 4 additions & 0 deletions src/extras/curves/QuadraticBezierCurve.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class QuadraticBezierCurve extends Curve {
this.v2 = v2;

}

getPoint( t, optionalTarget = new Vector2() ) {

const point = optionalTarget;
Expand All @@ -30,6 +31,7 @@ class QuadraticBezierCurve extends Curve {
return point;

}

copy( source ) {

super.copy( source );
Expand All @@ -41,6 +43,7 @@ class QuadraticBezierCurve extends Curve {
return this;

}

toJSON() {

const data = super.toJSON();
Expand All @@ -52,6 +55,7 @@ class QuadraticBezierCurve extends Curve {
return data;

}

fromJSON( json ) {

super.fromJSON( json );
Expand Down
4 changes: 4 additions & 0 deletions src/extras/curves/QuadraticBezierCurve3.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class QuadraticBezierCurve3 extends Curve {
this.v2 = v2;

}

getPoint( t, optionalTarget = new Vector3() ) {

const point = optionalTarget;
Expand All @@ -31,6 +32,7 @@ class QuadraticBezierCurve3 extends Curve {
return point;

}

copy( source ) {

super.copy( source );
Expand All @@ -42,6 +44,7 @@ class QuadraticBezierCurve3 extends Curve {
return this;

}

toJSON() {

const data = super.toJSON();
Expand All @@ -53,6 +56,7 @@ class QuadraticBezierCurve3 extends Curve {
return data;

}

fromJSON( json ) {

super.fromJSON( json );
Expand Down
10 changes: 7 additions & 3 deletions src/extras/curves/SplineCurve.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class SplineCurve extends Curve {
this.points = points;

}

getPoint( t, optionalTarget = new Vector2() ) {

const point = optionalTarget;
Expand All @@ -37,9 +38,10 @@ class SplineCurve extends Curve {
return point;

}

copy( source ) {

Curve.prototype.copy.call( this, source );
super.copy( source );

this.points = [];

Expand All @@ -54,9 +56,10 @@ class SplineCurve extends Curve {
return this;

}

toJSON() {

const data = Curve.prototype.toJSON.call( this );
const data = super.toJSON();

data.points = [];

Expand All @@ -70,9 +73,10 @@ class SplineCurve extends Curve {
return data;

}

fromJSON( json ) {

Curve.prototype.fromJSON.call( this, json );
super.fromJSON( json );

this.points = [];

Expand Down

0 comments on commit 330b186

Please sign in to comment.