Scale retina images down

zinserjan / wdio-screenshot

A WebdriverIO plugin. Additional commands for taking screenshots with WebdriverIO.

MIT License

109 stars 43 forks source link

Scale retina images down #55

Closed zinserjan closed 7 years ago

zinserjan commented 7 years ago

At the moment all screenshots commands emitting a screenshot that respects the display size. This means for devices with a pixel ratio of 2, the resulting screenshot is twice as big like on other devices with a pixel ratio of 1.

Currently I see some reasons to start scaling retina images down:

WebdriverCSS scales the images also down
it may allows us to compare screenshots better (Retina Mac vs normal PC)?
it may resolve the grey-line issue in #4 in the stitching process

Any opinions about this change?

texttechne commented 7 years ago

Right now the behaviour is an impressive demonstration of the meaning of pixel ratio :-)

However, I would argue that you neither want nor need the different scaling. The difference in pixel ratio is intended to give the user a better image quality without him knowing the technique behind that; so from the point of view of the user (or the customer for whom we took the screenshot) the image size is always the same. From the point of view of the tester the taken screenshots are usually not analyzed with retina displays. Even if, it wouldn't really make sense since it's all about testing automation not view experience (which would be lousy for all screenshots of devices with ratio 1).

So I'm all for scaling down.

mehdivk commented 7 years ago

I'm all for scaling down the images as well but I would be probably a breaking change. Not sure what's the approach here but we may want to decide about it using a configuration key? Ignoring the approach to go for, I think it will be very useful

zinserjan commented 7 years ago

From the point of view of the tester the taken screenshots are usually not analyzed with retina displays.

Yep, this is also my opinion.

As far as I know there aren't any cloud services with retina displays and mobile devices aren't supported yet. So this should only affect users that are testing locally with their own browsers.

would be probably a breaking change

Right, but that isn't a probem, cause it will be released with a minor version bump. Minor bumps below 1.0.0 acts like major bumps, so everything is fine.

using a configuration key

This makes the code and test cases just more complicated and that isn't worth it.

mehdivk commented 7 years ago

This is great! Thanks @zinserjan for this feature.