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

Add support for high density pixel displays (ex Apple Retina) #562

Conversation

mikeiz404
Copy link
Contributor

@mikeiz404 mikeiz404 commented Jul 21, 2017

On high density displays the html2canvas rendering can look blurry. This fixes that by rendering the page in a higher resolution for those displays.

The root of the problem is that html2canvas was rendering at the reported css pixel window dimensions which is not the actual pixel dimensions of the window for some displays. An html2canvas with scaling support has been added. The variable window.devicePixelRatio is used to set this scale when rendering.

I have only tested this locally on a macbook with a retina display. It would be great if others could test this on both high density and non high-density displays and let me know how that goes.

@mikeiz404 mikeiz404 force-pushed the bugfix/blury-preview-with-retina-displays branch from 4a66a80 to 470362b Compare July 21, 2017 19:45
@deanoemcke
Copy link
Collaborator

deanoemcke commented Aug 22, 2017

Thanks for this.

Firstly, it would be good if you could include a link to the scaling issue you refer to. Is it the one fixed in this html2canvas PR? niklasvh/html2canvas#1087

Unfortunately, the more recent beta version of html2canvas which you are using (changing from 0.4.1 to 0.5.0-beta4) comes with a few other issues which make it less desirable.

From my brief testing, https://github.com pages render using the wrong font, and rendering in general seems slower. I was also not able to sucessfully capture this page: https://material.io/guidelines/style/color.html which seems to be more of an issue with the height cropping (a canvas that is too large subsequently fails to convert during .toDataUrl). None of these are issues in the current 0.4.1 library.

I would love to implement this, but the above issues with the current beta release would need to be resolved first.

On a somewhat related note, there is also this outstanding issue with html2canvas: #503
Where tabs that have been zoomed in/out take a very long time to suspend.

@mikeiz404
Copy link
Contributor Author

mikeiz404 commented Aug 23, 2017

Yup, the issue is the one fixed by the PR you mentioned: niklasvh/html2canvas#1087. That PR is also where this version of html2canvas comes from.

I noticed some rendering issues with this version of html2canvas as well. I'll merge eKoopmans patch into the 0.4.1 version.

What's the best way to go about updating this pull request with the changes? Is a force push fine?

@mikeiz404 mikeiz404 force-pushed the bugfix/blury-preview-with-retina-displays branch from 470362b to 1965aae Compare August 25, 2017 01:08
@mikeiz404
Copy link
Contributor Author

mikeiz404 commented Aug 25, 2017

Ok I have added html2canvas 0.4.1 with scale support along with the other changes.

From what I can tell https://github.com is now rendering with the correct font, and the preview image generated from this branch and the chrome webstore match.

As for https://material.io/guidelines/style/color.html, the page fails to render when rendering to a hd display (scale=2) but renders fine when not rendering to a hd display (scale=1).

My guess is this is due to a url length limit hit by canvas.toDataURL. Increasing the imageTimeout did not help and a canvas element is printed to the console rather quickly. Changing the image quality from .8 to .01 of toDataURL did allow the image url to be generated.

Some possible solutions: 1) Crop the canvas when an empty url is returned and try again. 2) Split the canvas into sections and generate urls from them. The url image data could then be written out to a single image file or displayed on the preview page as multiple images.

@deanoemcke deanoemcke merged commit 1965aae into greatsuspender:master Aug 26, 2017
@deanoemcke
Copy link
Collaborator

@mikeiz404 Thanks so much for your help with this. I briefly tried to build the 0.4.1 version of html2canvas locally but couldn't get it to run. your changes look great.
For the toDataURL issues, I have found that using png rather than webp seems to work for the material.io page. I have added a fallback when webp fails.
These changes also helped me catch another bug when displaying these bad data urls so thanks for that also!

@deanoemcke
Copy link
Collaborator

@mikeiz404 Also, FYI, I ran into another issue that didn't seem to be there in the pure 0.4.1 release. I was getting "Failed to execute 'querySelectorAll' on 'Document' error". The issue is detailed on the html2canvas github page: niklasvh/html2canvas#385

I have patched the library to fix this: 766a78c

Hoping there's not more new issues out there!

@mikeiz404
Copy link
Contributor Author

@deanoemcke Glad it's now working completely. Let me know if you run into any more issues and I'll patch from whichever branch you want.

The current patch is off of the v0.4 tag (https://github.com/niklasvh/html2canvas/tree/v0.4), though the build from that branch is versioned as html2canvas 0.4.1 (https://github.com/niklasvh/html2canvas/blob/v0.4/build/html2canvas.js#L2). I assumed the source was correct, but I guess I was wrong.

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.

2 participants