[REGRESSION] DevTools: Screenshots functionality in DevTools is broken in many ways |
|||||||||||||||||||||||||||||||
Issue descriptionThe "Capture Screenshot" functionality in Device Mode produces a variety of bugs. In the attached screenshots, you can observe the following: 1) Nexus 6P: Device Art is horizontally stretched, viewport is not correctly positioned at all 2) Nexus 5X: Same issue, but slightly more subtle – viewport is too small to fit.
,
Oct 5 2016
,
Oct 5 2016
Let's try to fix and merge into M54.
,
Oct 5 2016
,
Oct 6 2016
Able to reproduce this issue in Win10, MAC10.12 and ubuntu14.04 with latest stable version 53.0.2785.143. Observed that "show device frame " is implemented from 52.0.2737.0 version. From this version itself, the screenshot functionality is not working correctly. Attached the screenshot with their file names as version for reference and removing bisect label for now. Please feel free to add back if it is required.
,
Oct 6 2016
Thank you for the bisect. It turns out that our current Nexus 5X/6P device outline images had a different aspect ratio from our calculated outlineRect. DeviceModeView tried to rectify the former by setting the element's width, height in CSS using applyRect(). However, the SVG image's intrinsic size isn't overridden by this. In order for screenshots to appear correctly, all SVGs need to have the same aspect ratio as (inset.top + screen.height + inset.bottom) / (inset.left + screen.width + inset.right), where inset comes from values in the emulated_devices/module.json. Ideally, we would be able to either hard code correct insets and not worry about the SVG dimensions, or hard code a correct SVG and compute insets at runtime.
,
Oct 7 2016
A friendly reminder that M53 Stable is launching VERY soon! Your bug is labelled as Stable Release Block Stable, please make sure to land the fix and get it merged into the release branch ASAP (before 5:00 PM PT, Monday) so we can take it for next weeks LAST Beta release for Desktop. Thank you!
,
Oct 7 2016
Thanks for letting us know the deadline. The CL is in CQ, and I'll be sure to request merge once it's on Canary. https://codereview.chromium.org/2387353005/
,
Oct 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ecbadddd6fdc69baeca94183e6805ba10eb587e2 commit ecbadddd6fdc69baeca94183e6805ba10eb587e2 Author: luoe <luoe@chromium.org> Date: Fri Oct 07 23:00:32 2016 DevTools: disable smoothing and correct nexus device frame dimensions for screenshots For Nexus5X and Nexus6P presets, the existing SVGs were not well sized. That is: height of device outline !== insets.top + screenSize.height + insets.bottom This produced a noticeable gap in device mode screenshots. This CL corrects the SVGs and insets to match measured dimensions. Additionally, canvas drawn images look blurry in a device mode screenshots since canvas contexts' imageSmoothingEnabled property is true by default. This CL disables unnecessary smoothing. BUG= 652938 Review-Url: https://codereview.chromium.org/2387353005 Cr-Commit-Position: refs/heads/master@{#424001} [modify] https://crrev.com/ecbadddd6fdc69baeca94183e6805ba10eb587e2/third_party/WebKit/Source/devtools/front_end/emulated_devices/Nexus5X-landscape.svg [modify] https://crrev.com/ecbadddd6fdc69baeca94183e6805ba10eb587e2/third_party/WebKit/Source/devtools/front_end/emulated_devices/Nexus5X-portrait.svg [modify] https://crrev.com/ecbadddd6fdc69baeca94183e6805ba10eb587e2/third_party/WebKit/Source/devtools/front_end/emulated_devices/Nexus6P-landscape.svg [modify] https://crrev.com/ecbadddd6fdc69baeca94183e6805ba10eb587e2/third_party/WebKit/Source/devtools/front_end/emulated_devices/Nexus6P-portrait.svg [modify] https://crrev.com/ecbadddd6fdc69baeca94183e6805ba10eb587e2/third_party/WebKit/Source/devtools/front_end/emulated_devices/module.json [modify] https://crrev.com/ecbadddd6fdc69baeca94183e6805ba10eb587e2/third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeView.js
,
Oct 8 2016
I tested that screenshots look correct on compiled Chromium on Linux, Windows 10/Canary 56.0.2884.0, and Mac/56.0.2884.0.
,
Oct 8 2016
Your change meets the bar and is auto-approved for M54 (branch: 2840)
,
Oct 10 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/451e4a3e46ba8c942386fec3901d2196e33dad25 commit 451e4a3e46ba8c942386fec3901d2196e33dad25 Author: luoe <luoe@chromium.org> Date: Mon Oct 10 17:38:59 2016 DevTools: disable smoothing and correct nexus device frame dimensions for screenshots For Nexus5X and Nexus6P presets, the existing SVGs were not well sized. That is: height of device outline !== insets.top + screenSize.height + insets.bottom This produced a noticeable gap in device mode screenshots. This CL corrects the SVGs and insets to match measured dimensions. Additionally, canvas drawn images look blurry in a device mode screenshots since canvas contexts' imageSmoothingEnabled property is true by default. This CL disables unnecessary smoothing. BUG= 652938 Review-Url: https://codereview.chromium.org/2387353005 Cr-Commit-Position: refs/heads/master@{#424001} (cherry picked from commit ecbadddd6fdc69baeca94183e6805ba10eb587e2) NOTRY=true NOPRESUBMIT=true Review-Url: https://codereview.chromium.org/2408603002 Cr-Commit-Position: refs/branch-heads/2840@{#699} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/451e4a3e46ba8c942386fec3901d2196e33dad25/third_party/WebKit/Source/devtools/front_end/emulated_devices/Nexus5X-landscape.svg [modify] https://crrev.com/451e4a3e46ba8c942386fec3901d2196e33dad25/third_party/WebKit/Source/devtools/front_end/emulated_devices/Nexus5X-portrait.svg [modify] https://crrev.com/451e4a3e46ba8c942386fec3901d2196e33dad25/third_party/WebKit/Source/devtools/front_end/emulated_devices/Nexus6P-landscape.svg [modify] https://crrev.com/451e4a3e46ba8c942386fec3901d2196e33dad25/third_party/WebKit/Source/devtools/front_end/emulated_devices/Nexus6P-portrait.svg [modify] https://crrev.com/451e4a3e46ba8c942386fec3901d2196e33dad25/third_party/WebKit/Source/devtools/front_end/emulated_devices/module.json [modify] https://crrev.com/451e4a3e46ba8c942386fec3901d2196e33dad25/third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeView.js
,
Oct 10 2016
,
Oct 12 2016
Tested in Chrome beta version 54.0.2840.59 in Windows 10, ubuntu 14.04 and mac 10.12. In all these OS platforms, the issue is fixed. Please check the attached screenshots.
,
Oct 13 2016
Thank you, those screenshots look good to me!
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/451e4a3e46ba8c942386fec3901d2196e33dad25 commit 451e4a3e46ba8c942386fec3901d2196e33dad25 Author: luoe <luoe@chromium.org> Date: Mon Oct 10 17:38:59 2016 DevTools: disable smoothing and correct nexus device frame dimensions for screenshots For Nexus5X and Nexus6P presets, the existing SVGs were not well sized. That is: height of device outline !== insets.top + screenSize.height + insets.bottom This produced a noticeable gap in device mode screenshots. This CL corrects the SVGs and insets to match measured dimensions. Additionally, canvas drawn images look blurry in a device mode screenshots since canvas contexts' imageSmoothingEnabled property is true by default. This CL disables unnecessary smoothing. BUG= 652938 Review-Url: https://codereview.chromium.org/2387353005 Cr-Commit-Position: refs/heads/master@{#424001} (cherry picked from commit ecbadddd6fdc69baeca94183e6805ba10eb587e2) NOTRY=true NOPRESUBMIT=true Review-Url: https://codereview.chromium.org/2408603002 Cr-Commit-Position: refs/branch-heads/2840@{#699} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/451e4a3e46ba8c942386fec3901d2196e33dad25/third_party/WebKit/Source/devtools/front_end/emulated_devices/Nexus5X-landscape.svg [modify] https://crrev.com/451e4a3e46ba8c942386fec3901d2196e33dad25/third_party/WebKit/Source/devtools/front_end/emulated_devices/Nexus5X-portrait.svg [modify] https://crrev.com/451e4a3e46ba8c942386fec3901d2196e33dad25/third_party/WebKit/Source/devtools/front_end/emulated_devices/Nexus6P-landscape.svg [modify] https://crrev.com/451e4a3e46ba8c942386fec3901d2196e33dad25/third_party/WebKit/Source/devtools/front_end/emulated_devices/Nexus6P-portrait.svg [modify] https://crrev.com/451e4a3e46ba8c942386fec3901d2196e33dad25/third_party/WebKit/Source/devtools/front_end/emulated_devices/module.json [modify] https://crrev.com/451e4a3e46ba8c942386fec3901d2196e33dad25/third_party/WebKit/Source/devtools/front_end/emulation/DeviceModeView.js |
|||||||||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||||||||
Comment 1 by l...@chromium.org
, Oct 5 2016Owner: l...@chromium.org
Status: Assigned (was: Unconfirmed)