Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get specs passing on displays using certain device scale factors #7145

Merged
merged 6 commits into from Sep 9, 2016

Conversation

kevinsawicki
Copy link
Contributor

@kevinsawicki kevinsawicki commented Sep 8, 2016

Previously there were several BrowserWindow specs that failed when run on displays using a non-integer device scale factor such as 1.5 or an odd number value like 3.

This was due to the rounding of pixel values that occurs when converting content and window size and position.

This pull request switches these specs to use a assertBoundsEqual helper method that allows the expected value to be off by 1 when run on non-integer scale factor screen.

An alternative to this would be calling app.commandLine.appendSwitch('force-device-scale-factor', '2') in the specs themselves but then the runner window would be a possibly annoying scale factor that might make it hard to use for certain people.

/cc @haacked

@haacked
Copy link
Contributor

haacked commented Sep 8, 2016

I'll give this a shot.

@haacked
Copy link
Contributor

haacked commented Sep 8, 2016

I tried this out and a couple of tests still fail.

BrowserWindow.setMinimum/MaximumSize(width, height)

sets the maximum and minimum size of the window

AssertionError: [ 100, 100 ] deepEqual [ 100, 101 ]
    at assertBoundsEqual (C:\dev\work\electron\spec\api-browser-window-spec.js:1359:12)
    at Context.<anonymous> (C:\dev\work\electron\spec\api-browser-window-spec.js:289:7)
    at callFn (C:\dev\work\electron\spec\node_modules\mocha\lib\runnable.js:251:21)
    at Test.Runnable.run (C:\dev\work\electron\spec\node_modules\mocha\lib\runnable.js:244:7)
    at Runner.runTest (C:\dev\work\electron\spec\node_modules\mocha\lib\runner.js:374:10)
    at C:\dev\work\electron\spec\node_modules\mocha\lib\runner.js:452:12
    at next (C:\dev\work\electron\spec\node_modules\mocha\lib\runner.js:299:14)
    at C:\dev\work\electron\spec\node_modules\mocha\lib\runner.js:309:7
    at next (C:\dev\work\electron\spec\node_modules\mocha\lib\runner.js:248:23)
    at Immediate.<anonymous> (C:\dev\work\electron\spec\node_modules\mocha\lib\runner.js:276:5)

BrowserWindow.setContentBounds(bounds)

sets the content size and position

AssertionError: { x: 10, y: 10, width: 250, height: 250 } deepEqual { x: 10, y: 9, width: 250, height: 250 }
    at assertBoundsEqual (C:\dev\work\electron\spec\api-browser-window-spec.js:1359:12)
    at C:\dev\work\electron\spec\api-browser-window-spec.js:353:9
    at CallbacksRegistry.apply (C:\dev\work\electron\out\D\resources\electron.asar\common\api\callbacks-registry.js:54:39)
    at EventEmitter.<anonymous> (C:\dev\work\electron\out\D\resources\electron.asar\renderer\api\remote.js:274:21)
    at emitThree (events.js:116:13)
    at EventEmitter.emit (events.js:194:7)

Notice that deepEqual is still being called which doesn't allow for the margin of error.

Get specs passing on displays using non-integer device scale factor

So the thing is, on my machine I do have an integer scale factor. I've scaled things to 300% which is a scale factor of 3. Not sure why that would create the off by one errors though.

@kevinsawicki
Copy link
Contributor Author

So the thing is, on my machine I do have an integer scale factor. I've scaled things to 300% which is a scale factor of 3. Not sure why that would create the off by one errors though.

Yeah, that is interesting, could you run process.argv from the Console tab of the devtools and include the value here of the device-scale-factor element?

@haacked
Copy link
Contributor

haacked commented Sep 8, 2016

"--device-scale-factor=3"

@kevinsawicki
Copy link
Contributor Author

"--device-scale-factor=3"

Okay, thanks, yeah, perhaps because it is an odd number scaling up and down can cause some rounding.

@kevinsawicki
Copy link
Contributor Author

@haacked can you try again on this branch? I adjusted the delta logic case to include odd number scale factors above 2.

@kevinsawicki kevinsawicki changed the title Get specs passing on displays using non-integer device scale factor Get specs passing on displays using certain device scale factors Sep 8, 2016
@haacked
Copy link
Contributor

haacked commented Sep 8, 2016

That fixed it!

@zcbenz
Copy link
Member

zcbenz commented Sep 9, 2016

👍

@haacked haacked merged commit cadbd7b into master Sep 9, 2016
@haacked haacked deleted the device-scale-factor-failures branch September 9, 2016 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants