Skip to content
This repository has been archived by the owner on Oct 25, 2023. It is now read-only.

Fix find element wrong coordinate #306

Merged
merged 10 commits into from
Mar 1, 2019
94 changes: 66 additions & 28 deletions lib/basedriver/commands/find.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ helpers.findByImage = async function (b64Template, {
}) {
const {
imageMatchThreshold: threshold,
fixImageTemplateSize
fixImageTemplateSize,
fixImageTemplateScale
} = this.settings.getSettings();

log.info(`Finding image element with match threshold ${threshold}`);
Expand All @@ -222,7 +223,12 @@ helpers.findByImage = async function (b64Template, {
let rect = null;
const condition = async () => {
try {
let b64Screenshot = await this.getScreenshotForImageFind(screenWidth, screenHeight);
const {b64Screenshot, scale} = await this.getScreenshotForImageFind(screenWidth, screenHeight);

if (fixImageTemplateScale) {
b64Template = await this.fixImageTemplateScale(b64Template, scale);
}

rect = (await this.compareImages(MATCH_TEMPLATE_MODE, b64Screenshot,
b64Template, {threshold})).rect;
return true;
Expand Down Expand Up @@ -288,24 +294,36 @@ helpers.ensureTemplateSize = async function (b64Template, screenWidth, screenHei
let imgObj = await imageUtil.getJimpImage(b64Template);
let {width: tplWidth, height: tplHeight} = imgObj.bitmap;

log.info(`Template image is ${tplWidth}x${tplHeight}. Screen size is ${screenWidth}x${screenHeight}`);
// if the template fits inside the screen dimensions, we're good
if (tplWidth <= screenWidth && tplHeight <= screenHeight) {
return b64Template;
}

log.info(`Scaling template image from ${tplWidth}x${tplHeight} to match ` +
`screen at ${screenWidth}x${screenHeight}`);
// otherwise, scale it to fit inside the screen dimensions
imgObj = imgObj.scaleToFit(screenWidth, screenHeight);
return (await imgObj.getBuffer(imageUtil.MIME_PNG)).toString('base64');
};

/**
* @typedef {Object} Screenshot
* @property {string} b64Screenshot - base64 based screenshot string
*/
/**
* @typedef {Object} ScreenshotScale
* @property {float} xScale - Scale ratio for width
* @property {float} yScale - Scale ratio for height
*/
/**
* Get the screenshot image that will be used for find by element, potentially
* altering it in various ways based on user-requested settings
*
* @param {int} screenWidth - width of screen
* @param {int} screenHeight - height of screen
*
* @returns {string} base64-encoded screenshot
* @returns {Screenshot, ?ScreenshotScale} base64-encoded screenshot and ScreenshotScale
*/
helpers.getScreenshotForImageFind = async function (screenWidth, screenHeight) {
if (!this.getScreenshot) {
Expand All @@ -318,7 +336,7 @@ helpers.getScreenshotForImageFind = async function (screenWidth, screenHeight) {
// between the screenshot and the screen, just return the screenshot now
if (!this.settings.getSettings().fixImageFindScreenshotDims) {
log.info(`Not verifying screenshot dimensions match screen`);
return b64Screenshot;
return {b64Screenshot};
}

// otherwise, do some verification on the screenshot to make sure it matches
Expand All @@ -332,43 +350,63 @@ helpers.getScreenshotForImageFind = async function (screenWidth, screenHeight) {
// the height and width of the screenshot and the device screen match, which
// means we should be safe when doing template matches
log.info('Screenshot size matched screen size');
return b64Screenshot;
return {b64Screenshot};
}

// otherwise, if they don't match, it could spell problems for the accuracy
// of coordinates returned by the image match algorithm, since we match based
// on the screenshot coordinates not the device coordinates themselves. There
// are two potential types of mismatch: aspect ratio mismatch and scale
// mismatch. we need to detect and fix both

const screenAR = screenWidth / screenHeight;
const shotAR = shotWidth / shotHeight;

if (screenAR === shotAR) {
log.info('Screenshot aspect ratio matched screen aspect ratio');
} else {
log.warn(`When trying to find an element, determined that the screen ` +
`aspect ratio and screenshot aspect ratio are different. Screen ` +
`is ${screenWidth}x${screenHeight} whereas screenshot is ` +
`${shotWidth}x${shotHeight}.`);
shotWidth = shotWidth / (shotAR / screenAR);
log.warn(`Resizing screenshot to ${shotWidth}x${shotHeight} to match ` +
`screen aspect ratio so that image element coordinates have a ` +
`greater chance of being correct.`);
imgObj = imgObj.resize(shotWidth, shotHeight);
}

// now we know the aspect ratios match, but there might still be a scale
// mismatch, so just resize based on the screen dimensions
if (screenWidth !== shotWidth) {
// mismatch.

// Resize based on the screen dimensions only if both width and height are mismatched
// since except for that, it might be a situation which is different window rect and
// screenshot size like `@driver.window_rect #=>x=0, y=0, width=1080, height=1794` and
// `"deviceScreenSize"=>"1080x1920"`
let scale;
if (screenWidth !== shotWidth && screenHeight !== shotHeight) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will consider different ratio case more (and create a PR)

log.info(`Scaling screenshot from ${shotWidth}x${shotHeight} to match ` +
`screen at ${screenWidth}x${screenHeight}`);
imgObj = imgObj.resize(screenWidth, screenHeight);

scale = {xScale: (1.0 * screenWidth) / shotWidth, yScale: (1.0 * screenHeight) / shotHeight};
}

return (await imgObj.getBuffer(imageUtil.MIME_PNG)).toString('base64');
b64Screenshot = (await imgObj.getBuffer(imageUtil.MIME_PNG)).toString('base64');
return {b64Screenshot, scale};
};

/**
* Get a image that will be used for template maching.
* Returns scaled image if scale ratio is provided.
*
* @param {string} b64Template - base64-encoded image used as a template to be
* matched in the screenshot
* @param {ScreenshotScale} scale - scale of screen. Default to `{}`
*
* @returns {string} base64-encoded scaled template screenshot
*/
helpers.fixImageTemplateScale = async function (b64Template, scale = {}) {
if (!scale) {
return b64Template;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scale might be destructed here

Copy link
Contributor

@mykola-mokhnach mykola-mokhnach Feb 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean const {xScale, yScale} = scale, so you don't have to always add scele. prefix


const {xScale, yScale} = scale;
if (!xScale || !yScale) {
return b64Template;
}

let imgTempObj = await imageUtil.getJimpImage(b64Template);
let {width: baseTempWidth, height: baseTempHeigh} = imgTempObj.bitmap;

const scaledWidth = baseTempWidth * xScale;
const scaledHeight = baseTempHeigh * yScale;
log.info(`Scaling template image from ${baseTempWidth}x${baseTempHeigh}` +
` to ${scaledWidth}x${scaledHeight}`);
log.info(`The ratio is ${xScale} and ${yScale}`);
imgTempObj = await imgTempObj.resize(scaledWidth, scaledHeight);
return (await imgTempObj.getBuffer(imageUtil.MIME_PNG)).toString('base64');
};

Object.assign(extensions, commands, helpers);
export { commands, helpers, IMAGE_STRATEGY, CUSTOM_STRATEGY };
Expand Down
8 changes: 8 additions & 0 deletions lib/basedriver/device-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ const GLOBAL_DEFAULT_SETTINGS = {
// complain
fixImageTemplateSize: false,

// whether Appium should ensure that an image template sent in during image
// element find should have its scale adjusted to display size so the match
// algorithm will not complain.
// e.g. iOS has `width=375, height=667` window rect, but its screenshot is
// `width=750 × height=1334` pixels. This setting help to adjust the scale
// if a user use `width=750 × height=1334` pixels's base template image.
fixImageTemplateScale: false,

// whether Appium should re-check that an image element can be matched
// against the current screenshot before clicking it
checkForImageElementStaleness: true,
Expand Down
76 changes: 61 additions & 15 deletions test/basedriver/commands/find-specs.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@ import path from 'path';
import chaiAsPromised from 'chai-as-promised';
import sinon from 'sinon';
import { BaseDriver, ImageElement } from '../../..';
import { IMAGE_STRATEGY, CUSTOM_STRATEGY } from '../../../lib/basedriver/commands/find';
import { IMAGE_STRATEGY, CUSTOM_STRATEGY, helpers } from '../../../lib/basedriver/commands/find';
import { imageUtil } from 'appium-support';


chai.should();
const should = chai.should();
chai.use(chaiAsPromised);


class TestDriver extends BaseDriver {
async getWindowSize () {}
async getScreenshot () {}
Expand Down Expand Up @@ -92,6 +93,46 @@ describe('finding elements by image', function () {
imgEl.template.should.eql(newTemplate);
compareStub.args[0][2].should.eql(newTemplate);
});

it('should fix template size scale if requested', async function () {
const d = new TestDriver();
const newTemplate = 'iVBORbaz';
const {compareStub} = basicStub(d);
await d.settings.update({fixImageTemplateScale: true});
sinon.stub(d, 'fixImageTemplateScale').returns(newTemplate);
const imgElProto = await d.findByImage(template, {multiple: false});
const imgEl = basicImgElVerify(imgElProto, d);
imgEl.template.should.eql(newTemplate);
compareStub.args[0][2].should.eql(newTemplate);
});
it('should not fix template size scale if it is not requested', async function () {
const d = new TestDriver();
const newTemplate = 'iVBORbaz';
basicStub(d);
await d.settings.update({});
sinon.stub(d, 'fixImageTemplateScale').returns(newTemplate);
d.fixImageTemplateScale.callCount.should.eql(0);
});
it('should not fix template size scale if no scale value', async function () {
const newTemplate = 'iVBORbaz';
await helpers.fixImageTemplateScale(newTemplate).should.eventually.eql(newTemplate);
});
it('should not fix template size scale if it is null', async function () {
const newTemplate = 'iVBORbaz';
await helpers.fixImageTemplateScale(newTemplate, null)
.should.eventually.eql(newTemplate);
});
it('should not fix template size scale if it is not number', async function () {
const newTemplate = 'iVBORbaz';
await helpers.fixImageTemplateScale(newTemplate, 'wrong-scale')
.should.eventually.eql(newTemplate);
});
it('should fix template size scale', async function () {
const actual = 'iVBORw0KGgoAAAANSUhEUgAAAAYAAAAGCAYAAADgzO9IAAAAWElEQVR4AU3BQRWAQAhAwa/PGBsEgrC16AFBKEIPXW7OXO+Rmey9iQjMjHFzrLUwM7qbqmLcHKpKRFBVuDvj4agq3B1VRUQYT2bS3QwRQVUZF/CaGRHB3wc1vSZbHO5+BgAAAABJRU5ErkJggg==';
await helpers.fixImageTemplateScale(TINY_PNG, { xScale: 1.5, yScale: 1.5 })
.should.eventually.eql(actual);
});

it('should throw an error if template match fails', async function () {
const d = new TestDriver();
const {compareStub} = basicStub(d);
Expand Down Expand Up @@ -156,44 +197,49 @@ describe('finding elements by image', function () {
sinon.stub(d, 'getScreenshot').returns(TINY_PNG);
d.settings.update({fixImageFindScreenshotDims: false});
const screen = TINY_PNG_DIMS.map(n => n + 1);
await d.getScreenshotForImageFind(...screen)
.should.eventually.eql(TINY_PNG);
const {b64Screenshot, scale} = await d.getScreenshotForImageFind(...screen);
b64Screenshot.should.eql(TINY_PNG);
should.equal(scale, undefined);
});
it('should return screenshot without adjustment if it matches screen size', async function () {
const d = new TestDriver();
sinon.stub(d, 'getScreenshot').returns(TINY_PNG);
await d.getScreenshotForImageFind(...TINY_PNG_DIMS)
.should.eventually.eql(TINY_PNG);
const {b64Screenshot, scale} = await d.getScreenshotForImageFind(...TINY_PNG_DIMS);
b64Screenshot.should.eql(TINY_PNG);
should.equal(scale, undefined);
});
it('should return scaled screenshot with same aspect ratio if matching screen aspect ratio', async function () {
const d = new TestDriver();
sinon.stub(d, 'getScreenshot').returns(TINY_PNG);
const screen = TINY_PNG_DIMS.map(n => n * 1.5);
const newScreenshot = await d.getScreenshotForImageFind(...screen);
newScreenshot.should.not.eql(TINY_PNG);
const screenshotObj = await imageUtil.getJimpImage(newScreenshot);
const {b64Screenshot, scale} = await d.getScreenshotForImageFind(...screen);
b64Screenshot.should.not.eql(TINY_PNG);
const screenshotObj = await imageUtil.getJimpImage(b64Screenshot);
screenshotObj.bitmap.width.should.eql(screen[0]);
screenshotObj.bitmap.height.should.eql(screen[1]);
scale.should.eql({ xScale: 1.5, yScale: 1.5 });
});
it('should return scaled screenshot with different aspect ratio if not matching screen aspect ratio', async function () {
const d = new TestDriver();
sinon.stub(d, 'getScreenshot').returns(TINY_PNG);

// try first with portrait screen
let screen = [TINY_PNG_DIMS[0] * 2, TINY_PNG_DIMS[1] * 3];
let newScreenshot = await d.getScreenshotForImageFind(...screen);
newScreenshot.should.not.eql(TINY_PNG);
let screenshotObj = await imageUtil.getJimpImage(newScreenshot);
const {b64Screenshot, scale} = await d.getScreenshotForImageFind(...screen);
b64Screenshot.should.not.eql(TINY_PNG);
let screenshotObj = await imageUtil.getJimpImage(b64Screenshot);
screenshotObj.bitmap.width.should.eql(screen[0]);
screenshotObj.bitmap.height.should.eql(screen[1]);
scale.should.eql({ xScale: 2, yScale: 3 });

// then with landscape screen
screen = [TINY_PNG_DIMS[0] * 3, TINY_PNG_DIMS[1] * 2];
newScreenshot = await d.getScreenshotForImageFind(...screen);
newScreenshot.should.not.eql(TINY_PNG);
screenshotObj = await imageUtil.getJimpImage(newScreenshot);
const {b64Screenshot: newScreen, scale: newScale} = await d.getScreenshotForImageFind(...screen);
newScreen.should.not.eql(TINY_PNG);
screenshotObj = await imageUtil.getJimpImage(newScreen);
screenshotObj.bitmap.width.should.eql(screen[0]);
screenshotObj.bitmap.height.should.eql(screen[1]);
newScale.should.eql({ xScale: 3, yScale: 2 });
});
});
});
Expand Down