maskRegions not supported for multi-page pdf

[moreginger]

Firstly, thanks so much for doing this, it's so close to solving my problem!

The remaining issue is that I need to mask a region containing the time the pdf was generated. However, maskRegions is not applied when there are multiple pages in the pdf (it would be good to at least document this).

It would be amazing to have some way to use maskRegions... either page at a time diff as per #1, or different masks per page, or a way to force the same mask on every page.

Meanwhile I'll try to tune the tolerance for now.

[moshensky]

Thanks for reporting it. I will have a look into it. At the very least will make sure to update the docs, but will try to make it work first.

[moshensky]

@moreginger unfortunately it is not possible to define mask per page. Right now multi page pdf is being converted to a single image containing all the pages one after another. Without implementing issue that you have mentioned, your only option is to add an array with masks with coordinates for each specific page. If your pages are same height and you want to mask same region everywhere, then you could simply generate that array by updating the y coordinate with a constant.

[moreginger]

@moshensky - passing a mask per page sounds fine to me. It looks like the masks are applied before the images are concatenated, so I don't think you'd need to offset each page? Currently, if there is more than one image (page) then it doesn't apply the mask at all: https://github.com/moshensky/pdf-visual-diff/blob/master/src/compare-pdf-to-snapshot.ts#L34

I can try to propose a PR if you'd like. The tricky bit is deciding the API for maskRegions. Should it require an MaskRegions per page, or should RectangleMask have an optional page property (otherwise applying it to every page), or something else? Changing RectangleMask to have an optional page property might be a good way to avoid breaking your current API.

[moshensky]

@moreginger thank you for pointing me to the root cause. I took the liberty to break the interface and update the code right away.

I have released a new version 0.8.0 please have a look and see whether it will work for you. You can see in the tests a sample usage.

Don't take my change as an unwillingness to have more contributors to the code base and feel welcomed to contribute to the project!

[moreginger]

Thanks for fixing! I didn't think of using a callback, and it looks good. I just updated it and got my test passing 👌

One thing that I didn't realise, it may or may not be worth thinking about. I'm calling the compare function in a cypress (https://www.cypress.io/) "task". These are functions the cypress process can run for you outside of the browser, which is necessary to be able to test the downloaded pdf here. It turns out that you can't pass function arguments to a cypress task - which makes sense as they are going across a process boundary from browser to cypress backend - so I had to pass maskRegions as an array from my test class, and then create the function argument inside the task implementation. Easy enough to do, of course, but it's something to consider.

Thanks for the quick update. I'm always very happy for someone else to do the work :D

[moshensky]

Unless it becomes a big pain point for someone I don't consider doing any changes right now due to some other priorities.