Use device scale factor instead of physical screen DPI for screen DPI
Created attachment 173322 [details] Patch
Thanks Adam. I'll land this once all the prerequisites are in place.
Comment on attachment 173322 [details] Patch Attachment 173322 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14790157
Comment on attachment 173322 [details] Patch Attachment 173322 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14785335
Comment on attachment 173322 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=173322&action=review Nice cleanup - looks good! > Source/WebCore/page/Screen.cpp:NaN > unsigned Screen::verticalDPI() const Screen::horizontalDPI and Screen::verticalDPI are now redundant. Could we remove one of them, and rename the other to Screen::cssDPI (to emphasize that it computes the CSS dpi unit, not the true physical dpi of the screen)? > Source/WebCore/platform/blackberry/PlatformScreenBlackBerry.cpp:-32 > - FloatSize currentPPI = BlackBerry::Platform::Graphics::Screen::primaryScreen()->pixelsPerInch(-1); Seems like blackberry implemented this too. Might want to ping them on the review.
(In reply to comment #5) > Screen::horizontalDPI and Screen::verticalDPI are now redundant. Could we remove one of them, and rename the other to Screen::cssDPI (to emphasize that it computes the CSS dpi unit, not the true physical dpi of the screen)? Good idea. Let me try to see how that works out. > > Source/WebCore/platform/blackberry/PlatformScreenBlackBerry.cpp:-32 > > - FloatSize currentPPI = BlackBerry::Platform::Graphics::Screen::primaryScreen()->pixelsPerInch(-1); > > Seems like blackberry implemented this too. Might want to ping them on the review. Antonio, any thoughts about this one?
(In reply to comment #6) > (In reply to comment #5) > > > Source/WebCore/platform/blackberry/PlatformScreenBlackBerry.cpp:-32 > > > - FloatSize currentPPI = BlackBerry::Platform::Graphics::Screen::primaryScreen()->pixelsPerInch(-1); > > > > Seems like blackberry implemented this too. Might want to ping them on the review. > > Antonio, any thoughts about this one? Looks good to me.
Comment on attachment 173322 [details] Patch Rejecting attachment 173322 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: j.target/webkit/Source/WebKit/chromium/src/DatabaseObserver.o Source/WebKit/chromium/src/ChromeClientImpl.cpp: In member function 'virtual void WebKit::ChromeClientImpl::dispatchViewportPropertiesDidChange(const WebCore::ViewportArguments&) const': Source/WebKit/chromium/src/ChromeClientImpl.cpp:625: error: 'screenHorizontalDPI' was not declared in this scope make: *** [out/Release/obj.target/webkit/Source/WebKit/chromium/src/ChromeClientImpl.o] Error 1 make: *** Waiting for unfinished jobs.... Full output: http://queues.webkit.org/results/14872336
Comment on attachment 173322 [details] Patch Clearing flags on attachment: 173322 Committed r135165: <http://trac.webkit.org/changeset/135165>
All reviewed patches have been landed. Closing bug.