Skip to content
This repository has been archived by the owner on Jul 28, 2020. It is now read-only.

Source image dimensions are cropped at 400x300 pixels #1

Closed
wants to merge 3 commits into from

Conversation

adamschroder
Copy link

Since stats.js opens the image directly on a default phantom viewport, the dimensions are clipped at 400x300.

setting phantom viewport to a larger size fixes the issue.

PhantomJS won't fix this ariya/phantomjs#10191, so we need to set viewportSize.

since stats.js opens the image directly on a default phantom viewport, the dimensions are clipped at 400x300.

setting phantom viewport to a larger size fixes the issue.

PhantomJS won't fix this ariya/phantomjs#10191, so we need to set viewportSize.
@netroy
Copy link
Contributor

netroy commented May 6, 2013

👍

@twolfson
Copy link
Owner

twolfson commented May 6, 2013

The issue you have linked to states a problem with the default sizing of screenshots.

stats.js operates by creating an HTML page containing the image, then scraping the image height/width via JS. There is nothing relevant to the viewport at my first glance.

Can you provide a unit test for this case? If it seems there is still a problem, please follow an exponential growth algorithm (doubling while loops for viewportHeight and viewportWidth) similar to that prescribed by ariya in the issue.

@netroy
Copy link
Contributor

netroy commented May 24, 2013

@twolfson unfortunately stats.js open the image directly in the webpage https://github.com/twolfson/phantomjssmith/blob/master/lib/scripts/stats.js#L26
if it was opened through an html page, this probably won't happen.
I'll write a test case.

@adamschroder
Copy link
Author

@twolfson instead of tampering with the viewport size, we override the explicit image resize behaviour of webkit. So any image opened in phantomJS would be unscaled and return the correct resolution.

@twolfson
Copy link
Owner

@adamschroder @netroy A unit test or gist demonstrating the problem would definitely help to clear things up for me.

@twolfson
Copy link
Owner

I am tired of having this open GitHub issue. I have successfully reproduced in a gist and will be pushing a patch shortly.

https://gist.github.com/twolfson/5654659

@twolfson
Copy link
Owner

Tested in [email protected] and patched [email protected]. Thank you for the bug report.

In the future, please make an effort to be more responsive.

@twolfson twolfson closed this May 27, 2013
@netroy
Copy link
Contributor

netroy commented May 27, 2013

@twolfson apologies for not responding in time, will try to be more responsive in the future..
thanks a lot.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants