ronantakizawa/codereview-bench huggingface dataset:
Repository: pixijs/pixijs
File: tests/mixin-get-global-position/getGlobalPosition.tests.ts (TypeScript)

Code before:
import { Container } from '../../src/scene/container/Container';

describe('DisplayObject#getGlobalPosition', () =>
{
    it('should exist', () =>
    {
        const obj = new Container();

        expect(obj.getGlobalPosition).toBeDefined();
        expect(obj.getGlobalPosition).toBeInstanceOf(Function);
    });

    it.skip('should return correct global coordinates of a displayObject, without depending on its pivot', () =>
    {
        const parent = new Container();
        const container = new Container();

        parent.addChild(container);

        parent.position.set(100, 100);
        parent.rotation = Math.PI;
        parent.scale.set(2, 2);
        container.position.set(10, -30);
        container.pivot.set(1000, 1000);

        let globalPoint = container.getGlobalPosition();

        expect(globalPoint.x).toEqual(80);
        expect(globalPoint.y).toEqual(160);

        // check but skipUpdate

        parent.position.set(200, 200);
        globalPoint = container.getGlobalPosition(globalPoint, true);

        expect(globalPoint.x).toEqual(80); // currently returning x=10, is this a bug?
        expect(globalPoint.y).toEqual(160); // currently returning y=-30, is this a bug?
    });

Review comment (suggestion):
```suggestion
    it.skip('should return correct global coordinates of a container, without depending on its pivot', () =>
```

Code after:
import { Container } from '../../src/scene/container/Container';

describe('Container#getGlobalPosition', () =>
{
    it('should exist', () =>
    {
        const obj = new Container();

        expect(obj.getGlobalPosition).toBeDefined();
        expect(obj.getGlobalPosition).toBeInstanceOf(Function);
    });

    it('should return correct global coordinates of a container, without depending on its pivot', () =>
    {
        const parent = new Container();
        const container = new Container();

        parent.addChild(container);

        parent.position.set(100, 100);
        parent.rotation = Math.PI;
        parent.scale.set(2, 2);
        container.position.set(10, -30);
        container.pivot.set(1000, 1000);

        let globalPoint = container.getGlobalPosition();

        expect(globalPoint.x).toEqual(80);
        expect(globalPoint.y).toEqual(160);

        // check but skipUpdate

        parent.position.set(200, 200);
        // not strictly skipUpdate, more like use scene graph values...
        globalPoint = container.getGlobalPosition(globalPoint, true);

        expect(globalPoint.x).toEqual(10); // this is the containers values from above unchanged
        expect(globalPoint.y).toEqual(-30); // this is the containers values from above unchanged
Repository: pixijs/pixijs
File: tests/mixin-get-global-position/getGlobalPosition.tests.ts (TypeScript)

Code before:
import { Container } from '../../src/scene/container/Container';

describe('Container#getGlobalPosition', () =>
{
    it('should exist', () =>
    {
        const obj = new Container();

        expect(obj.getGlobalPosition).toBeDefined();
        expect(obj.getGlobalPosition).toBeInstanceOf(Function);
    });

    it('should return correct global coordinates of a displayObject, without depending on its pivot', () =>
    {
        const parent = new Container();
        const container = new Container();

        parent.addChild(container);

        parent.position.set(100, 100);
        parent.rotation = Math.PI;
        parent.scale.set(2, 2);
        container.position.set(10, -30);
        container.pivot.set(1000, 1000);

        let globalPoint = container.getGlobalPosition();

        expect(globalPoint.x).toEqual(80);
        expect(globalPoint.y).toEqual(160);

        // check but skipUpdate

        parent.position.set(200, 200);
        // not strictly skipUpdate, more like use scene graph values...
        globalPoint = container.getGlobalPosition(globalPoint, true);

        expect(globalPoint.x).toEqual(10); // this is the containers values from above unchanged
        expect(globalPoint.y).toEqual(-30); // this is the containers values from above unchanged

Review comment (suggestion):
```suggestion
    it('should return correct global coordinates of a container, without depending on its pivot', () =>
```

Code after:
import { Container } from '../../src/scene/container/Container';

describe('Container#getGlobalPosition', () =>
{
    it('should exist', () =>
    {
        const obj = new Container();

        expect(obj.getGlobalPosition).toBeDefined();
        expect(obj.getGlobalPosition).toBeInstanceOf(Function);
    });

    it('should return correct global coordinates of a container, without depending on its pivot', () =>
    {
        const parent = new Container();
        const container = new Container();

        parent.addChild(container);

        parent.position.set(100, 100);
        parent.rotation = Math.PI;
        parent.scale.set(2, 2);
        container.position.set(10, -30);
        container.pivot.set(1000, 1000);

        let globalPoint = container.getGlobalPosition();

        expect(globalPoint.x).toEqual(80);
        expect(globalPoint.y).toEqual(160);

        // check but skipUpdate

        parent.position.set(200, 200);
        // not strictly skipUpdate, more like use scene graph values...
        globalPoint = container.getGlobalPosition(globalPoint, true);

        expect(globalPoint.x).toEqual(10); // this is the containers values from above unchanged
        expect(globalPoint.y).toEqual(-30); // this is the containers values from above unchanged
Repository: pixijs/pixijs
File: src/rendering/renderers/shared/buffer/Buffer.ts (TypeScript)

Code before:
            usage,
            mappedAtCreation,
            label,
        };

        this.shrinkToFit = shrinkToFit ?? true;
    }

    /** @todo */
    get data()
    {
        return this._data;
    }

    set data(value: TypedArray)
    {
        this.setData(value, value.length);
    }

    /**
     * Sets the data in the buffer to the given value. This will immediately update the buffer on the GPU.
     * If you only want to update a subset of the buffer, you can pass in the size of the data.
     * @param value - the data to set
     * @param size - the size of the data in bytes
     */
    public setData(value: TypedArray, size?: number)
    {
        // Increment update ID
        this._updateID++;

        // Calculate size in bytes
        const sizeInBytes = size ? (size * value.BYTES_PER_ELEMENT) : value.byteLength;

        this._updateSize = sizeInBytes;

        // If the data hasn't changed, early return after emitting 'update'
        if (this._data === value)
        {
            this.emit('update', this);

            return;
        }

        // Cache old data and update to new value
        const oldData = this._data;

        this._data = value;

        // Event handling
        if (oldData.length !== value.length)
        {

Review comment (suggestion):
Seems weird to have 2 api's for the same thing but with an optional arg

```js
buffer.data = typedArray
buffer.setData(typedArray)
```

we should either remove the setter of `buffer.data`, or change the name of `setData` to something like `setDataWithSize()` and not make the `size` arg optional


Code after:
            usage,
            mappedAtCreation,
            label,
        };

        this.shrinkToFit = shrinkToFit ?? true;
    }

    /** @todo */
    get data()
    {
        return this._data;
    }

    set data(value: TypedArray)
    {
        this.setDataWithSize(value, value.length);
    }

    /**
     * Sets the data in the buffer to the given value. This will immediately update the buffer on the GPU.
     * If you only want to update a subset of the buffer, you can pass in the size of the data.
     * @param value - the data to set
     * @param size - the size of the data in bytes
     */
    public setDataWithSize(value: TypedArray, size: number)
    {
        // Increment update ID
        this._updateID++;

        this._updateSize = (size * value.BYTES_PER_ELEMENT);

        // If the data hasn't changed, early return after emitting 'update'
        if (this._data === value)
        {
            this.emit('update', this);

            return;
        }

        // Cache old data and update to new value
        const oldData = this._data;

        this._data = value;

        // Event handling
        if (oldData.length !== value.length)
        {
            if (!this.shrinkToFit && value.byteLength < oldData.byteLength)
            {
                this.emit('update', this);
Repository: pixijs/pixijs
File: src/rendering/renderers/shared/buffer/Buffer.ts (TypeScript)

Code before:
     */
    public _touched = 0;

    /**
     * a description of the buffer and how it should be set up on the GPU
     * @internal
     * @ignore
     */
    public readonly descriptor: BufferDescriptor;

    /**
     * @internal
     * @ignore
     */
    public _updateID = 1;

    /**
     * @internal
     * @ignore
     */
    public _updateSize: number;

    private _data: TypedArray;

    /**
     * should the GPU buffer bew shrunk when the data becomes smaller?
     * changing this will cause the buffer to be destroyed and a new one created on the GPU
     * this can be expensive, especially if the buffer is already big enough!
     * setting ts to false will prevent the buffer from being shrunk. This will yield better performance
     * if you are constantly setting data that is changing size often.
     * @default true
     */
    public shrinkToFit = true;

    /**
     * Creates a new Buffer with the given options
     * @param options - the options for the buffer
     */
    constructor(options: BufferOptions)
    {
        let { data, size } = options;
        const { usage, label, shrinkToFit } = options;

        super();

        if (data instanceof Array)
        {
            data = new Float32Array(data as number[]);
        }

        this._data = data as TypedArray;

Review comment (suggestion):
```suggestion
     * should the GPU buffer be shrunk when the data becomes smaller?
```

Code after:
     */
    public _touched = 0;

    /**
     * a description of the buffer and how it should be set up on the GPU
     * @internal
     * @ignore
     */
    public readonly descriptor: BufferDescriptor;

    /**
     * @internal
     * @ignore
     */
    public _updateID = 1;

    /**
     * @internal
     * @ignore
     */
    public _updateSize: number;

    private _data: TypedArray;

    /**
     * should the GPU buffer be shrunk when the data becomes smaller?
     * changing this will cause the buffer to be destroyed and a new one created on the GPU
     * this can be expensive, especially if the buffer is already big enough!
     * setting this to false will prevent the buffer from being shrunk. This will yield better performance
     * if you are constantly setting data that is changing size often.
     * @default true
     */
    public shrinkToFit = true;

    /**
     * Creates a new Buffer with the given options
     * @param options - the options for the buffer
     */
    constructor(options: BufferOptions)
    {
        let { data, size } = options;
        const { usage, label, shrinkToFit } = options;

        super();

        if (data instanceof Array)
        {
            data = new Float32Array(data as number[]);
        }

        this._data = data as TypedArray;
Repository: pixijs/pixijs
File: src/rendering/renderers/shared/buffer/Buffer.ts (TypeScript)

Code before:
    /**
     * a description of the buffer and how it should be set up on the GPU
     * @internal
     * @ignore
     */
    public readonly descriptor: BufferDescriptor;

    /**
     * @internal
     * @ignore
     */
    public _updateID = 1;

    /**
     * @internal
     * @ignore
     */
    public _updateSize: number;

    private _data: TypedArray;

    /**
     * should the GPU buffer bew shrunk when the data becomes smaller?
     * changing this will cause the buffer to be destroyed and a new one created on the GPU
     * this can be expensive, especially if the buffer is already big enough!
     * setting ts to false will prevent the buffer from being shrunk. This will yield better performance
     * if you are constantly setting data that is changing size often.
     * @default true
     */
    public shrinkToFit = true;

    /**
     * Creates a new Buffer with the given options
     * @param options - the options for the buffer
     */
    constructor(options: BufferOptions)
    {
        let { data, size } = options;
        const { usage, label, shrinkToFit } = options;

        super();

        if (data instanceof Array)
        {
            data = new Float32Array(data as number[]);
        }

        this._data = data as TypedArray;

        size = size ?? (data as TypedArray)?.byteLength;


Review comment (performance):
```suggestion
     * setting this to false will prevent the buffer from being shrunk. This will yield better performance
```

Code after:
    /**
     * a description of the buffer and how it should be set up on the GPU
     * @internal
     * @ignore
     */
    public readonly descriptor: BufferDescriptor;

    /**
     * @internal
     * @ignore
     */
    public _updateID = 1;

    /**
     * @internal
     * @ignore
     */
    public _updateSize: number;

    private _data: TypedArray;

    /**
     * should the GPU buffer be shrunk when the data becomes smaller?
     * changing this will cause the buffer to be destroyed and a new one created on the GPU
     * this can be expensive, especially if the buffer is already big enough!
     * setting this to false will prevent the buffer from being shrunk. This will yield better performance
     * if you are constantly setting data that is changing size often.
     * @default true
     */
    public shrinkToFit = true;

    /**
     * Creates a new Buffer with the given options
     * @param options - the options for the buffer
     */
    constructor(options: BufferOptions)
    {
        let { data, size } = options;
        const { usage, label, shrinkToFit } = options;

        super();

        if (data instanceof Array)
        {
            data = new Float32Array(data as number[]);
        }

        this._data = data as TypedArray;

        size = size ?? (data as TypedArray)?.byteLength;
Repository: pixijs/pixijs
File: src/utils/logging/logDebugTexture.ts (TypeScript)

Code before:
import type { Texture } from '../../rendering/renderers/shared/texture/Texture';
import type { Renderer } from '../../rendering/renderers/types';

/**
 * Logs a texture to the console as a base64 image.
 * This can be very useful for debugging issues with rendering.
 * @param texture - The texture to log
 * @param renderer - The renderer to use
 * @param size - The size of the texture to log in the console
 * @memberof utils
 */
export async function logDebugTexture(texture: Texture, renderer: Renderer, size = 200)
{
    const base64 = await renderer.extract.base64(texture);

    await renderer.encoder.commandFinished;

    const width = size;

    // eslint-disable-next-line no-console
    console.log(`logging texture ${texture.source.width}px ${texture.source.height}px`);

    const style = [
        'font-size: 1px;',
        `padding: ${width}px ${300}px;`,
        `background: url(${base64}) no-repeat;`,
        'background-size: contain;',
    ].join(' ');

    // eslint-disable-next-line no-console
    console.log('%c ', style);
}


Review comment (refactor):
we can ignore this function as it actually is used when I need to log a texture mid render (which no one should be doing). 

Users can use `renderer.extract.log(thing)` for most use cases

Code after:
import type { Texture } from '../../rendering/renderers/shared/texture/Texture';
import type { Renderer } from '../../rendering/renderers/types';

/**
 * Logs a texture to the console as a base64 image.
 * This can be very useful for debugging issues with rendering.
 * @param texture - The texture to log
 * @param renderer - The renderer to use
 * @param size - The size of the texture to log in the console
 * @ignore
 */
export async function logDebugTexture(texture: Texture, renderer: Renderer, size = 200)
{
    const base64 = await renderer.extract.base64(texture);

    await renderer.encoder.commandFinished;

    const width = size;

    // eslint-disable-next-line no-console
    console.log(`logging texture ${texture.source.width}px ${texture.source.height}px`);

    const style = [
        'font-size: 1px;',
        `padding: ${width}px ${300}px;`,
        `background: url(${base64}) no-repeat;`,
        'background-size: contain;',
    ].join(' ');

    // eslint-disable-next-line no-console
    console.log('%c ', style);
}
Repository: pixijs/pixijs
File: src/utils/misc/Transform.ts (TypeScript)

Code before:
import { Matrix } from '../../maths/matrix/Matrix';
import { ObservablePoint } from '../../maths/point/ObservablePoint';

import type { Observer } from '../../maths/point/ObservablePoint';

/**
 * Options for the {@link utils.Transform} constructor.
 * @memberof utils
 */
export interface TransformOptions
{
    /** The matrix to use. */
    matrix?: Matrix;
    /** The observer to use. */
    observer?: {onUpdate: (transform: Transform) => void}
}

/**
 * Transform that takes care about its versions.
 * @memberof utils
 */
export class Transform
{
    /**
     * The local transformation matrix.
     * @internal
     * @private
     */
    public _matrix: Matrix;

    /** The coordinate of the object relative to the local coordinates of the parent. */
    public position: ObservablePoint;

    /** The scale factor of the object. */
    public scale: ObservablePoint;

    /** The pivot point of the container that it rotates around. */
    public pivot: ObservablePoint;

    /** The skew amount, on the x and y axis. */
    public skew: ObservablePoint;

    /** The rotation amount. */
    protected _rotation: number;

Review comment (suggestion):
```suggestion
 * The Transform class facilitates the manipulation of a 2D transformation matrix through 
 * user-friendly properties: position, scale, rotation, skew, and pivot. 
```

Code after:
import { Matrix } from '../../maths/matrix/Matrix';
import { ObservablePoint } from '../../maths/point/ObservablePoint';

import type { Observer } from '../../maths/point/ObservablePoint';

/**
 * Options for the {@link utils.Transform} constructor.
 * @memberof utils
 */
export interface TransformOptions
{
    /** The matrix to use. */
    matrix?: Matrix;
    /** The observer to use. */
    observer?: {onUpdate: (transform: Transform) => void}
}

/**
 * The Transform class facilitates the manipulation of a 2D transformation matrix through
 * user-friendly properties: position, scale, rotation, skew, and pivot.
 * @memberof utils
 */
export class Transform
{
    /**
     * The local transformation matrix.
     * @internal
     * @private
     */
    public _matrix: Matrix;

    /** The coordinate of the object relative to the local coordinates of the parent. */
    public position: ObservablePoint;

    /** The scale factor of the object. */
    public scale: ObservablePoint;

    /** The pivot point of the container that it rotates around. */
    public pivot: ObservablePoint;

    /** The skew amount, on the x and y axis. */
    public skew: ObservablePoint;

    /** The rotation amount. */
Repository: pixijs/pixijs
File: src/utils/misc/Transform.ts (TypeScript)

Code before:
    protected observer: Observer<Transform>;

    /**
     * @param param0 - Options for the transform.
     * @param param0.matrix - The matrix to use.
     * @param param0.observer - The observer to use.
     */
    constructor({ matrix, observer }: TransformOptions = {})
    {
        this._matrix = matrix ?? new Matrix();
        this.observer = observer;

        this.position = new ObservablePoint(this, 0, 0);
        this.scale = new ObservablePoint(this, 1, 1);
        this.pivot = new ObservablePoint(this, 0, 0);
        this.skew = new ObservablePoint(this, 0, 0);

        this._rotation = 0;
        this._cx = 1;
        this._sx = 0;
        this._cy = 0;
        this._sy = 1;
    }

    /**
     * The local matrix.
     * @readonly
     */
    get matrix(): Matrix
    {
        const lt = this._matrix;

        if (!this.dirty) return lt;

        lt.a = this._cx * this.scale.x;
        lt.b = this._sx * this.scale.x;
        lt.c = this._cy * this.scale.y;
        lt.d = this._sy * this.scale.y;

        lt.tx = this.position.x - ((this.pivot.x * lt.a) + (this.pivot.y * lt.c));
        lt.ty = this.position.y - ((this.pivot.x * lt.b) + (this.pivot.y * lt.d));

        this.dirty = false;

        return lt;
    }
    /**
     * Called when a value changes.
     * @param point
     * @internal
     * @private

Review comment (suggestion):
```suggestion
     * This matrix is computed by combining this Transforms position, scale, rotation, skew, and pivot
     * properties into a single matrix.
```

Code after:
    protected dirty = true;
    protected observer: Observer<Transform>;

    /**
     * @param options - Options for the transform.
     * @param options.matrix - The matrix to use.
     * @param options.observer - The observer to use.
     */
    constructor({ matrix, observer }: TransformOptions = {})
    {
        this._matrix = matrix ?? new Matrix();
        this.observer = observer;

        this.position = new ObservablePoint(this, 0, 0);
        this.scale = new ObservablePoint(this, 1, 1);
        this.pivot = new ObservablePoint(this, 0, 0);
        this.skew = new ObservablePoint(this, 0, 0);

        this._rotation = 0;
        this._cx = 1;
        this._sx = 0;
        this._cy = 0;
        this._sy = 1;
    }

    /**
     * This matrix is computed by combining this Transforms position, scale, rotation, skew, and pivot
     * properties into a single matrix.
     * @readonly
     */
    get matrix(): Matrix
    {
        const lt = this._matrix;

        if (!this.dirty) return lt;

        lt.a = this._cx * this.scale.x;
        lt.b = this._sx * this.scale.x;
        lt.c = this._cy * this.scale.y;
        lt.d = this._sy * this.scale.y;

        lt.tx = this.position.x - ((this.pivot.x * lt.a) + (this.pivot.y * lt.c));
        lt.ty = this.position.y - ((this.pivot.x * lt.b) + (this.pivot.y * lt.d));

        this.dirty = false;

        return lt;
    }
    /**
     * Called when a value changes.
     * @param point
Repository: pixijs/pixijs
File: src/utils/pool/Pool.ts (TypeScript)

Code before:
/**
 * A generic class for managing a pool of items.
 * @template T The type of items in the pool. Must extend {@link utils.PoolItem}.
 * @memberof utils
 */
export class Pool<T extends PoolItem>
{
    public readonly _classType: PoolItemConstructor<T>;
    private readonly _pool: T[] = [];
    private _count = 0;
    private _index = 0;

    /**
     * Constructs a new Pool.
     * @param ClassType - The constructor of the items in the pool.
     * @param {number} [initialSize] - The initial size of the pool.
     */
    constructor(ClassType: PoolItemConstructor<T>, initialSize?: number)
    {
        this._classType = ClassType;

        if (initialSize)
        {
            this.prepopulate(initialSize);
        }
    }

    /**

Review comment (suggestion):
```suggestion
 * @template T The type of items in the pool. Must implement {@link utils.PoolItem}.
```

Code after:
/**
 * A generic class for managing a pool of items.
 * @template T The type of items in the pool. Must implement {@link utils.PoolItem}.
 * @memberof utils
 */
export class Pool<T extends PoolItem>
{
    public readonly _classType: PoolItemConstructor<T>;
    private readonly _pool: T[] = [];
    private _count = 0;
    private _index = 0;

    /**
     * Constructs a new Pool.
     * @param ClassType - The constructor of the items in the pool.
     * @param {number} [initialSize] - The initial size of the pool.
     */
    constructor(ClassType: PoolItemConstructor<T>, initialSize?: number)
    {
        this._classType = ClassType;

        if (initialSize)
        {
            this.prepopulate(initialSize);
        }
    }

    /**