search

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

wip: fix: resize with interpolationType nearest #453

Closed tpoisseau closed 3 months ago

tpoisseau commented 3 months ago

Closes: https://github.com/image-js/image-js-typescript/issues/452

targos commented 3 months ago

@stropitek wdyt? Maybe we should remove the custom code and use transform with a scaling matrix?

stropitek commented 3 months ago

I think it would make sense to use transform to have a consistent implementation of methods like resize, cropRectangle, rotate which all just iterate over a target image, calculate a position on the source image and then interpolate the value.

tpoisseau commented 3 months ago

I would like to try to use transform matrix instead but I don't know how it work

targos commented 3 months ago

https://en.wikipedia.org/wiki/Scaling_(geometry)

With a 2x3 matrix, it should be 


[
  [ factor   0    0 ]
  [   0    factor 0 ]
]
tpoisseau commented 3 months ago

It's the best I can do. Nearest is working. Bilinear is very close from the opencv result.

I let you review before resetting snapshots to let the tests pass.

tpoisseau commented 3 months ago

Bilinear:

PR OpenCV main
result resize_bilinear testResizeBilinear resize_bilinear
substract resize_bilinear_substraction resize_bilinear_substraction

Nearest:

PR OpenCV main
result resize_nearest testResizeNearest resize_nearest
substract resize_nearest_substraction resize_nearest_substraction
targos commented 3 months ago

Do we still need custom implementation of the interpolation functions?

targos commented 3 months ago

Bilinear is known to give different result compared to OpenCV. Solution is to add a margin of error with comment like here: https://github.com/image-js/image-js-typescript/blob/8d81c2b4094c25522b9c40aec4b6469837123352/src/geometry/__tests__/transform.test.ts#L110-L113

Use the smallest possible error margin

tpoisseau commented 3 months ago

Do we still need custom implementation of the interpolation functions?

I removed It because I do not use it anymore.

Bilinear is known to give different result compared to OpenCV.

Ok I thought we should be one to one with OpenCV (And I found his less precise algorithm more smoother).

If you agree with theses code changes I think we can reset snapshots and merge

targos commented 3 months ago

I found his less precise algorithm more smoother

Really? The difference is barely visible in the other tests (you have to quickly change from one image to the other to see that the colors change, but they are basically the same).

tpoisseau commented 3 months ago

The difference is barely visible in the other tests

I did not check other tests, but take a lot of time to tweak trying to get result most similar result with opencv. Comparing result with substraction, zoom-in and pixel grid ^^'

tpoisseau commented 3 months ago

After snaptshots reset, some tests did not pass yet

targos commented 3 months ago

There are still substantial changes to interpolateNeighbourBilinear which may explain it.

tpoisseau commented 3 months ago

Yes, I'll adapt tests data

targos commented 3 months ago

But are you sure about the change to the algorithm? It worked fine until now.

tpoisseau commented 3 months ago

I'm not sure. I found bilinear for the small image better and more similar to opencv. but for largeur image, black and white (like letters with rotation and matching) it seems more blurry. rotation seems to produce some color shifting with triangles.

Maybe I should rollback the bilinear but keep nearest.

tpoisseau commented 3 months ago

This PR was in a dead end I preferred to restart from scratch and only fix nearest resize. https://github.com/image-js/image-js-typescript/pull/454

This PR will be usefull for future as exploration of this topic.