image-js / image-js-typescript

Temporary repository to work on the migration of image-js to TypeScript
https://image-js.github.io/image-js-typescript/
MIT License
5 stars 5 forks source link

Unexpected behavior when using decimals while drawing a rectangle #381

Closed josoriom closed 1 year ago

josoriom commented 1 year ago

Code:

import { writeFileSync } from 'fs';

import { Image, encode } from 'image-js';

const rectangles = [
  { width: 100, height: 50, origin: { row: 369, column: 152 } },
  { width: 34, height: 15, origin: { row: 193, column: 343 } },
  { width: 730, height: 84.00001, origin: { row: 479, column: 137 } },
];

const image = new Image(1000, 800).fill(255);
image.drawRectangle({ out: image });
image.drawCircle({ column: 500, row: 400 }, 400, { out: image });

for (const rectangle of rectangles) {
  image.drawRectangle({...rectangle, out: image});
}

writeFileSync(new URL('./test.png', import.meta.url), encode(image));

Result:

image

lpatiny commented 1 year ago

We apply already rounding in some 'draw' methods like:

https://github.com/image-js/image-js-typescript/blob/24a6b6622ead70c2264322501d9cf95279733554/src/draw/drawLineOnImage.ts#L53-L61

So I think all the draw methods should allow non integer numbers and we should use Math.round. Seems this should also be replaced by Math.round:

https://github.com/image-js/image-js-typescript/blob/24a6b6622ead70c2264322501d9cf95279733554/src/draw/drawMarker.ts#L91-L94

@targos @stropitek If you agree Javier could make a PR with test cases to allow all the draw methods to accept non integer numbers.

stropitek commented 1 year ago

SGTM

targos commented 1 year ago

Yes, we already decided to Math.round in "draw" functions: https://github.com/image-js/image-js-typescript/issues/353#issue-1735752261