Skip to content

Commit

Permalink
move to a kite-specific issue, phetsims/dot#120 phetsims/tasks#1129
Browse files Browse the repository at this point in the history
  • Loading branch information
@zepumph
zepumph committed Sep 15, 2023
1 parent 26615b5 commit 265c1ff
Show file tree
Hide file tree
Showing 11 changed files with 74 additions and 74 deletions.
28 changes: 14 additions & 14 deletions js/Shape.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* Notes for elliptical arcs: http://www.w3.org/TR/SVG/implnote.html#PathElementImplementationNotes
* Notes for painting strokes: https://svgwg.org/svg2-draft/painting.html
*
* TODO: add nonzero / evenodd support when browsers support it https://github.com/phetsims/tasks/issues/1129
* TODO: add nonzero / evenodd support when browsers support it https://github.com/phetsims/kite/issues/102
* TODO: docs
*
* @author Jonathan Olson <jonathan.olson@colorado.edu>
Expand Down Expand Up @@ -432,7 +432,7 @@ class Shape implements CanApplyParsedSVG {
* Adds a quadratic curve to this shape. The quadratic curves passes through the x and y coordinate.
* The shape should join smoothly with the previous subpaths
*
* TODO: consider a rename to put 'smooth' farther back? https://github.com/phetsims/tasks/issues/1129
* TODO: consider a rename to put 'smooth' farther back? https://github.com/phetsims/kite/issues/102
*
* @param x - final x position of the quadratic curve
* @param y - final y position of the quadratic curve
Expand Down Expand Up @@ -470,7 +470,7 @@ class Shape implements CanApplyParsedSVG {
this.getLastSubpath().addPoint( point );
const nondegenerateSegments = quadratic.getNondegenerateSegments();
_.each( nondegenerateSegments, segment => {
// TODO: optimization https://github.com/phetsims/tasks/issues/1129
// TODO: optimization https://github.com/phetsims/kite/issues/102
this.addSegmentAndBounds( segment );
} );
this.setQuadraticControlPoint( controlPoint );
Expand Down Expand Up @@ -868,8 +868,8 @@ class Shape implements CanApplyParsedSVG {
public ellipse( center: Vector2, radiusX: number, radiusY: number, rotation: number ): this;
public ellipse( centerX: number, centerY: number, radiusX: number, radiusY: number, rotation: number ): this;
public ellipse( centerX: Vector2 | number, centerY: number, radiusX: number, radiusY: number, rotation?: number ): this {
// TODO: separate into ellipse() and ellipsePoint()? https://github.com/phetsims/tasks/issues/1129
// TODO: Ellipse/EllipticalArc has a mess of parameters. Consider parameter object, or double-check parameter handling https://github.com/phetsims/tasks/issues/1129
// TODO: separate into ellipse() and ellipsePoint()? https://github.com/phetsims/kite/issues/102
// TODO: Ellipse/EllipticalArc has a mess of parameters. Consider parameter object, or double-check parameter handling https://github.com/phetsims/kite/issues/102
if ( typeof centerX === 'object' ) {
// ellipse( center, radiusX, radiusY, rotation )
const center = centerX;
Expand Down Expand Up @@ -1093,7 +1093,7 @@ class Shape implements CanApplyParsedSVG {
* Returns a new Shape that is transformed by the associated matrix
*/
public transformed( matrix: Matrix3 ): Shape {
// TODO: allocation reduction https://github.com/phetsims/tasks/issues/1129
// TODO: allocation reduction https://github.com/phetsims/kite/issues/102
const subpaths = _.map( this.subpaths, subpath => subpath.transformed( matrix ) );
const bounds = _.reduce( subpaths, ( bounds, subpath ) => bounds.union( subpath.bounds ), Bounds2.NOTHING );
return new Shape( subpaths, bounds );
Expand All @@ -1111,7 +1111,7 @@ class Shape implements CanApplyParsedSVG {
curveEpsilon: ( providedOptions && providedOptions.includeCurvature ) ? 0.002 : null
}, providedOptions );

// TODO: allocation reduction https://github.com/phetsims/tasks/issues/1129
// TODO: allocation reduction https://github.com/phetsims/kite/issues/102
const subpaths = _.map( this.subpaths, subpath => subpath.nonlinearTransformed( options ) );
const bounds = _.reduce( subpaths, ( bounds, subpath ) => bounds.union( subpath.bounds ), Bounds2.NOTHING );
return new Shape( subpaths, bounds );
Expand Down Expand Up @@ -1236,7 +1236,7 @@ class Shape implements CanApplyParsedSVG {
return true;
}

// TODO: if an issue, we can reduce this allocation to a scratch variable local in the Shape.js scope. https://github.com/phetsims/tasks/issues/1129
// TODO: if an issue, we can reduce this allocation to a scratch variable local in the Shape.js scope. https://github.com/phetsims/kite/issues/102
const delta = endPoint.minus( startPoint );
const length = delta.magnitude;

Expand Down Expand Up @@ -1306,7 +1306,7 @@ class Shape implements CanApplyParsedSVG {

let hitPoint;
let i;
// TODO: could optimize to intersect differently so we bail sooner https://github.com/phetsims/tasks/issues/1129
// TODO: could optimize to intersect differently so we bail sooner https://github.com/phetsims/kite/issues/102
const horizontalRayIntersections = this.intersection( minHorizontalRay ).concat( this.intersection( maxHorizontalRay ) );
for ( i = 0; i < horizontalRayIntersections.length; i++ ) {
hitPoint = horizontalRayIntersections[ i ].point;
Expand All @@ -1331,7 +1331,7 @@ class Shape implements CanApplyParsedSVG {
* Returns a new Shape that is an outline of the stroked path of this current Shape. currently not intended to be
* nested (doesn't do intersection computations yet)
*
* TODO: rename stroked( lineStyles )? https://github.com/phetsims/tasks/issues/1129
* TODO: rename stroked( lineStyles )? https://github.com/phetsims/kite/issues/102
*/
public getStrokedShape( lineStyles: LineStyles ): Shape {
let subpaths: Subpath[] = [];
Expand All @@ -1353,7 +1353,7 @@ class Shape implements CanApplyParsedSVG {
* Gets a shape offset by a certain amount.
*/
public getOffsetShape( distance: number ): Shape {
// TODO: abstract away this type of behavior https://github.com/phetsims/tasks/issues/1129
// TODO: abstract away this type of behavior https://github.com/phetsims/kite/issues/102
const subpaths = [];
const bounds = Bounds2.NOTHING.copy();
let subLen = this.subpaths.length;
Expand Down Expand Up @@ -1570,7 +1570,7 @@ class Shape implements CanApplyParsedSVG {
}

public toString(): string {
// TODO: consider a more verbose but safer way? https://github.com/phetsims/tasks/issues/1129
// TODO: consider a more verbose but safer way? https://github.com/phetsims/kite/issues/102
return `new phet.kite.Shape( '${this.getSVGPath()}' )`;
}

Expand Down Expand Up @@ -1739,7 +1739,7 @@ class Shape implements CanApplyParsedSVG {
/**
* Returns a new shape that only contains portions of segments that are within the passed-in shape's area.
*
* // TODO: convert Graph to TS and get the types from there https://github.com/phetsims/tasks/issues/1129
* // TODO: convert Graph to TS and get the types from there https://github.com/phetsims/kite/issues/102
*/
public shapeClip( shape: Shape, options?: { includeExterior?: boolean; includeBoundary: boolean; includeInterior: boolean } ): Shape {
return Graph.clipShape( shape, this, options );
Expand Down Expand Up @@ -1982,7 +1982,7 @@ class Shape implements CanApplyParsedSVG {
public static ellipse( center: Vector2, radiusX: number, radiusY: number, rotation: number ): Shape;
public static ellipse( radiusX: number, radiusY: number, rotation: number ): Shape;
public static ellipse( a: Vector2 | number, b: number, c: number, d?: number, e?: number ): Shape {
// TODO: Ellipse/EllipticalArc has a mess of parameters. Consider parameter object, or double-check parameter handling https://github.com/phetsims/tasks/issues/1129
// TODO: Ellipse/EllipticalArc has a mess of parameters. Consider parameter object, or double-check parameter handling https://github.com/phetsims/kite/issues/102
if ( d === undefined ) {
// ellipse( radiusX, radiusY ), center = 0,0
return new Shape().ellipse( 0, 0, a as number, b, c );
Expand Down
2 changes: 1 addition & 1 deletion js/ops/BoundsIntersection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ export default class BoundsIntersection {
}

// Subdivide continuously
// TODO: is 50 the proper number of iterations? https://github.com/phetsims/tasks/issues/1129
// TODO: is 50 the proper number of iterations? https://github.com/phetsims/kite/issues/102
for ( let i = 0; i < 50; i++ ) {
const newIntersections: BoundsIntersection[] = [];
for ( let j = intersections.length - 1; j >= 0; j-- ) {
Expand Down
26 changes: 13 additions & 13 deletions js/ops/Graph.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*
* See Graph.binaryResult for the general procedure for CAG.
*
* TODO: Use https://github.com/mauriciosantos/Buckets-JS for priority queue, implement simple sweep line https://github.com/phetsims/tasks/issues/1129
* TODO: Use https://github.com/mauriciosantos/Buckets-JS for priority queue, implement simple sweep line https://github.com/phetsims/kite/issues/102
* with "enters" and "leaves" entries in the queue. When edge removed, remove "leave" from queue.
* and add any replacement edges. Applies to overlap and intersection handling.
* NOTE: This should impact performance a lot, as we are currently over-scanning and re-scanning a lot.
Expand Down Expand Up @@ -550,7 +550,7 @@ class Graph {

assert && assert( this.loops.length === 0 );

// TODO: Can we avoid this in the inner loop? https://github.com/phetsims/tasks/issues/1129
// TODO: Can we avoid this in the inner loop? https://github.com/phetsims/kite/issues/102
if ( aEdge.startVertex === vertex ) {
aSegment = aSegment.reversed();
}
Expand Down Expand Up @@ -607,7 +607,7 @@ class Graph {
const addToQueue = edge => {
const bounds = edge.segment.bounds;

// TODO: see if object allocations are slow here https://github.com/phetsims/tasks/issues/1129
// TODO: see if object allocations are slow here https://github.com/phetsims/kite/issues/102
queue.push( { start: true, edge: edge }, bounds.minY - epsilon );
queue.push( { start: false, edge: edge }, bounds.maxY + epsilon );
};
Expand Down Expand Up @@ -643,7 +643,7 @@ class Graph {
let overlappedEdge;
let addedEdges;

// TODO: Is this closure killing performance? https://github.com/phetsims/tasks/issues/1129
// TODO: Is this closure killing performance? https://github.com/phetsims/kite/issues/102
segmentTree.query( edge, otherEdge => {
const overlaps = edge.segment.getOverlaps( otherEdge.segment );

Expand Down Expand Up @@ -833,7 +833,7 @@ class Graph {
const segment = edge.segment;

if ( segment instanceof Cubic ) {
// TODO: This might not properly handle when it only one endpoint is on the curve https://github.com/phetsims/tasks/issues/1129
// TODO: This might not properly handle when it only one endpoint is on the curve https://github.com/phetsims/kite/issues/102
const selfIntersection = segment.getSelfIntersection();

if ( selfIntersection ) {
Expand Down Expand Up @@ -889,7 +889,7 @@ class Graph {
const addToQueue = edge => {
const bounds = edge.segment.bounds;

// TODO: see if object allocations are slow here https://github.com/phetsims/tasks/issues/1129
// TODO: see if object allocations are slow here https://github.com/phetsims/kite/issues/102
queue.push( { start: true, edge: edge }, bounds.minY - epsilon );
queue.push( { start: false, edge: edge }, bounds.maxY + epsilon );
};
Expand Down Expand Up @@ -926,7 +926,7 @@ class Graph {
let addedEdges;
let removedEdges;

// TODO: Is this closure killing performance? https://github.com/phetsims/tasks/issues/1129
// TODO: Is this closure killing performance? https://github.com/phetsims/kite/issues/102
segmentTree.query( edge, otherEdge => {

const aSegment = edge.segment;
Expand All @@ -942,7 +942,7 @@ class Graph {
} );
if ( intersections.length ) {

// TODO: In the future, handle multiple intersections (instead of re-running) https://github.com/phetsims/tasks/issues/1129
// TODO: In the future, handle multiple intersections (instead of re-running) https://github.com/phetsims/kite/issues/102
const intersection = intersections[ 0 ];

const result = this.simpleSplit( edge, otherEdge, intersection.aT, intersection.bT, intersection.point );
Expand Down Expand Up @@ -1102,7 +1102,7 @@ class Graph {

// Adds an vertex to the queue
const addToQueue = vertex => {
// TODO: see if object allocations are slow here https://github.com/phetsims/tasks/issues/1129
// TODO: see if object allocations are slow here https://github.com/phetsims/kite/issues/102
queue.push( { start: true, vertex: vertex }, vertex.point.y - epsilon );
queue.push( { start: false, vertex: vertex }, vertex.point.y + epsilon );
};
Expand Down Expand Up @@ -1138,7 +1138,7 @@ class Graph {
let overlappedVertex;
let addedVertices;

// TODO: Is this closure killing performance? https://github.com/phetsims/tasks/issues/1129
// TODO: Is this closure killing performance? https://github.com/phetsims/kite/issues/102
segmentTree.query( vertex, otherVertex => {
const distance = vertex.point.distance( otherVertex.point );
if ( distance < VERTEX_COLLAPSE_THRESHOLD_DISTANCE ) {
Expand Down Expand Up @@ -1377,7 +1377,7 @@ class Graph {
* This information is stored in the childBoundaries array of Boundary, and is then read out to set up faces.
*/
computeBoundaryTree() {
// TODO: detect "indeterminate" for robustness (and try new angles?) https://github.com/phetsims/tasks/issues/1129
// TODO: detect "indeterminate" for robustness (and try new angles?) https://github.com/phetsims/kite/issues/102
const unboundedHoles = []; // {Array.<Boundary>}

// We'll want to compute a ray for each outer boundary that starts at an extreme point for that direction and
Expand Down Expand Up @@ -1563,7 +1563,7 @@ class Graph {
* Returns the boundary that contains the specified half-edge.
* @private
*
* TODO: find a better way, this is crazy inefficient https://github.com/phetsims/tasks/issues/1129
* TODO: find a better way, this is crazy inefficient https://github.com/phetsims/kite/issues/102
*
* @param {HalfEdge} halfEdge
* @returns {Boundary}
Expand Down Expand Up @@ -1735,7 +1735,7 @@ class Graph {
* Returns the xor of an array of shapes.
* @public
*
* TODO: reduce code duplication? https://github.com/phetsims/tasks/issues/1129
* TODO: reduce code duplication? https://github.com/phetsims/kite/issues/102
*
* @param {Array.<Shape>} shapes
* @returns {Shape}
Expand Down
14 changes: 7 additions & 7 deletions js/segments/Arc.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import Utils from '../../../dot/js/Utils.js';
import Vector2 from '../../../dot/js/Vector2.js';
import { EllipticalArc, kite, Line, Overlap, RayIntersection, Segment, SegmentIntersection, svgNumber } from '../imports.js';

// TODO: See if we should use this more https://github.com/phetsims/tasks/issues/1129
// TODO: See if we should use this more https://github.com/phetsims/kite/issues/102
const TWO_PI = Math.PI * 2;

export type SerializedArc = {
Expand Down Expand Up @@ -260,7 +260,7 @@ export default class Arc extends Segment {
return [ this ];
}

// TODO: verify that we don't need to switch anticlockwise here, or subtract 2pi off any angles https://github.com/phetsims/tasks/issues/1129
// TODO: verify that we don't need to switch anticlockwise here, or subtract 2pi off any angles https://github.com/phetsims/kite/issues/102
const angle0 = this.angleAt( 0 );
const angleT = this.angleAt( t );
const angle1 = this.angleAt( 1 );
Expand Down Expand Up @@ -484,7 +484,7 @@ export default class Arc extends Segment {
* Returns the angle for the parametrized t value. The t value should range from 0 to 1 (inclusive).
*/
public angleAt( t: number ): number {
//TODO: add asserts https://github.com/phetsims/tasks/issues/1129
//TODO: add asserts https://github.com/phetsims/kite/issues/102
return this._startAngle + ( this.getActualEndAngle() - this._startAngle ) * t;
}

Expand All @@ -511,7 +511,7 @@ export default class Arc extends Segment {
*/
public containsAngle( angle: number ): boolean {
// transform the angle into the appropriate coordinate form
// TODO: check anticlockwise version! https://github.com/phetsims/tasks/issues/1129
// TODO: check anticlockwise version! https://github.com/phetsims/kite/issues/102
const normalizedAngle = this._anticlockwise ? angle - this._endAngle : angle - this._startAngle;

// get the angle between 0 and 2pi
Expand Down Expand Up @@ -591,7 +591,7 @@ export default class Arc extends Segment {
_.each( [ 0, Math.PI / 2, Math.PI, 3 * Math.PI / 2 ], angle => {
if ( this.containsAngle( angle ) ) {
const t = this.tAtAngle( angle );
const epsilon = 0.0000000001; // TODO: general kite epsilon?, also do 1e-Number format https://github.com/phetsims/tasks/issues/1129
const epsilon = 0.0000000001; // TODO: general kite epsilon?, also do 1e-Number format https://github.com/phetsims/kite/issues/102
if ( t > epsilon && t < 1 - epsilon ) {
result.push( t );
}
Expand Down Expand Up @@ -682,7 +682,7 @@ export default class Arc extends Segment {
/**
* Returns a new copy of this arc, transformed by the given matrix.
*
* TODO: test various transform types, especially rotations, scaling, shears, etc. https://github.com/phetsims/tasks/issues/1129
* TODO: test various transform types, especially rotations, scaling, shears, etc. https://github.com/phetsims/kite/issues/102
*/
public transformed( matrix: Matrix3 ): Arc | EllipticalArc {
// so we can handle reflections in the transform, we do the general case handling for start/end angles
Expand Down Expand Up @@ -1058,7 +1058,7 @@ export default class Arc extends Segment {

const radius = ( startDiff.magnitude + middleDiff.magnitude + endDiff.magnitude ) / 3;

// Try anticlockwise first. TODO: Don't require creation of extra Arcs https://github.com/phetsims/tasks/issues/1129
// Try anticlockwise first. TODO: Don't require creation of extra Arcs https://github.com/phetsims/kite/issues/102
const arc = new Arc( center, radius, startAngle, endAngle, false );
if ( arc.containsAngle( middleAngle ) ) {
return arc;
Expand Down
Loading

1 comment on commit 265c1ff

@zepumph
Copy link
Member Author

Choose a reason for hiding this comment

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

#102

Please sign in to comment.