Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(25)

Issue 2387353005: DevTools: disable smoothing and correct nexus device frame dimensions for screenshots (Closed)

Created:
4 years, 2 months ago by luoe
Modified:
4 years, 2 months ago
Reviewers:
dgozman
CC:
chromium-reviews, caseq+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, blink-reviews, pfeldman, kozyatinskiy+blink_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

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 Committed: https://crrev.com/ecbadddd6fdc69baeca94183e6805ba10eb587e2 Cr-Commit-Position: refs/heads/master@{#424001}

Patch Set 1 #

Patch Set 2 : Use integer numbers #

Patch Set 3 : 1px adjust #

Patch Set 4 : Use optimized svg format #

Messages

Total messages: 18 (10 generated)
luoe
ptal!
4 years, 2 months ago (2016-10-05 21:10:54 UTC) #3
luoe
Converted numbers to integers. See the bug for an explanation as to why this bug ...
4 years, 2 months ago (2016-10-06 21:18:16 UTC) #4
dgozman
The screenshots are still blurry for me, and Nexus 5X screens seems off by one ...
4 years, 2 months ago (2016-10-06 23:06:44 UTC) #5
luoe
That is correct, pixel adjusted after visual verification.
4 years, 2 months ago (2016-10-07 00:53:37 UTC) #6
dgozman
lgtm
4 years, 2 months ago (2016-10-07 20:45:22 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2387353005/60001
4 years, 2 months ago (2016-10-07 21:37:59 UTC) #14
commit-bot: I haz the power
Committed patchset #4 (id:60001)
4 years, 2 months ago (2016-10-07 23:00:56 UTC) #16
commit-bot: I haz the power
4 years, 2 months ago (2016-10-07 23:03:48 UTC) #18
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/ecbadddd6fdc69baeca94183e6805ba10eb587e2
Cr-Commit-Position: refs/heads/master@{#424001}

Powered by Google App Engine
This is Rietveld 408576698