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

Change minimum width and height to 0 #32

Closed
wants to merge 2 commits into from

Conversation

perkee
Copy link

@perkee perkee commented Jan 28, 2013

Hi Paul,

Like everyone before me, I first want to thank you for an amazing program. Huge help.
I've changed the option parsing code to set the width and height to any value greater than 0, but keep the default of 800x600. I needed this feature to test responsive CSS, and I think it might help out some other cats too.

If there is some hard minimum that the WebView shouldn't go below, please let me know. I've tested it down to -W 300 and gotten the desired results.

Again, thanks for the awesome work,
Perkee

previously w/h would only change if specified w/h was greater than
default value. Now change if greater than 0, but keep default value at
800x600
It previously said 'width must be…"
@finferflu
Copy link

Thank you, that's exactly what I was looking for! I don't understand why the minimum size is set to 600.

@perkee
Copy link
Author

perkee commented Jun 27, 2013

Yay! Glad you guys like it! Kind of wish the pull request were accepted, but I'm sure there are good reasons for it.

@paulhammond
Copy link
Owner

There's no good reason that I haven't looked at this yet - I've just been really bad at keeping on top of webkit2png pull requests. I'm sorry.

The minimum size is currently set to 800px to make sure that the clipped images are always exactly the requested size (which is 200px wide by default, at a 25% ratio, giving you 800px). If you run the code from this pull request on, say, alistapart.com at 300px wide then you get a 75 wide clipped image, not a 200 wide image as requested:

webkit2png(pr/32=)$ ./webkit2png -W 300 http://alistapart.com/
Fetching http://alistapart.com/ ...
 ... done
webkit2png(pr/32=)$ file alistapartcom-clipped.png
alistapartcom-clipped.png: PNG image data, 75 x 150, 8-bit/color RGBA, non-interlaced

One way around that would be to request a smaller clipped image. For example, on master right now you can add a --clipwidth=50 argument and get a 300px wide image:

webkit2png(master=)$ ./webkit2png -W 300 --clipwidth=50 http://alistapart.com/
Fetching http://alistapart.com/ ...
 ... done
webkit2png(master=)$ file alistapartcom-full.png 
alistapartcom-full.png: PNG image data, 300 x 3856, 8-bit/color RGBA, non-interlaced

There's no reason why this restriction should apply when no clipped image is requested, but with the code currently in master it does:

webkit2png(master=)$ ./webkit2png -W 300 -FT http://alistapart.com/
Fetching http://alistapart.com/ ...
 ... done
webkit2png(master=)$ file alistapartcom-full.png 
alistapartcom-full.png: PNG image data, 800 x 3050, 8-bit/color RGBA, non-interlaced

I don't want to merge this as it is because it changes what the clipped image means. I've been meaning to rethink the distinction between clipped, thumbnails and full size images for some time (most people need just one, generating all three by default is wasteful behavior) and I think this pull request might be what tips me over into making that change.

@paulhammond
Copy link
Owner

I just realized this is the same issue as #12, so I'm going to close this pull request and continue the work over there.

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.

4 participants