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

Issue 10548026: mac: Make dynamic DPI changes work. (Closed)

Created:
8 years, 6 months ago by Nico
Modified:
8 years, 6 months ago
Reviewers:
Avi (use Gerrit)
CC:
chromium-reviews, yusukes+watch_chromium.org, jochen+watch-content_chromium.org, jam, penghuang+watch_chromium.org, joi+watch-content_chromium.org, darin-cc_chromium.org, James Su, oshima, rjkroege, pkotwicz, sadrul
Visibility:
Public.

Description

mac: Make dynamic DPI changes work. This fixes the software case completely, and makes everything look good on the hardware path after a few frames. In the hardware path, the IOSurfaces aren't tagged with their own DPI, so they are rendered at the wrong scale for a few frames, until the "DPI changed" message made it from the browser to the renderer. This CL adds three things that happen in response to DPI changes: 1.) It reallocates the browser-side BackingStoreMac at the new DPI. 2.) It lets the renderer do a full repaint after DPI changed. 3.) It passes the new DPI on to WebKit (in render_view_impl.cc) (3) won't have an effect for hardware pages until https://bugs.webkit.org/show_bug.cgi?id=88916 lands. BUG=128267 TEST=change from lodpi and hidpi and back, check that software- and hardware-rendered pages look good after the change in both foreground and background tabs. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=142099

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : weird #

Patch Set 4 : rebase #

Patch Set 5 : works! #

Patch Set 6 : rebase #

Patch Set 7 : simpler #

Patch Set 8 : simpler #

Patch Set 9 : more complicated #

Patch Set 10 : bg tabs work; repaints #

Patch Set 11 : . #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+71 lines, -9 lines) Patch
M content/browser/renderer_host/backing_store_mac.h View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M content/browser/renderer_host/backing_store_mac.mm View 1 2 3 4 5 6 7 8 9 10 1 chunk +22 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_mac.mm View 1 2 3 4 5 6 7 8 9 10 5 chunks +29 lines, -7 lines 2 comments Download
M content/renderer/render_view_impl.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M content/renderer/render_view_impl.cc View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download
M content/renderer/render_widget.h View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/render_widget.cc View 1 2 3 4 5 6 7 8 9 1 chunk +10 lines, -1 line 0 comments Download

Messages

Total messages: 5 (0 generated)
Nico
8 years, 6 months ago (2012-06-14 04:40:26 UTC) #1
Avi (use Gerrit)
lgtm https://chromiumcodereview.appspot.com/10548026/diff/12003/content/browser/renderer_host/render_widget_host_view_mac.mm File content/browser/renderer_host/render_widget_host_view_mac.mm (right): https://chromiumcodereview.appspot.com/10548026/diff/12003/content/browser/renderer_host/render_widget_host_view_mac.mm#newcode1920 content/browser/renderer_host/render_widget_host_view_mac.mm:1920: // active, in DidBecomeSelected(). Not for this CL, ...
8 years, 6 months ago (2012-06-14 04:51:20 UTC) #2
Nico
Thanks! https://chromiumcodereview.appspot.com/10548026/diff/12003/content/browser/renderer_host/render_widget_host_view_mac.mm File content/browser/renderer_host/render_widget_host_view_mac.mm (right): https://chromiumcodereview.appspot.com/10548026/diff/12003/content/browser/renderer_host/render_widget_host_view_mac.mm#newcode1920 content/browser/renderer_host/render_widget_host_view_mac.mm:1920: // active, in DidBecomeSelected(). On 2012/06/14 04:51:20, Avi ...
8 years, 6 months ago (2012-06-14 04:57:45 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/thakis@chromium.org/10548026/12003
8 years, 6 months ago (2012-06-14 04:58:01 UTC) #4
Avi (use Gerrit)
8 years, 6 months ago (2012-06-14 05:08:35 UTC) #5
I just renamed Browser::GetSelectedWebContents to GetActiveWebContents, so some
clarification might be useful here (what does "selected" mean?)
DidBecomeSelected() really means more like "DidBecomeShown"...

Powered by Google App Engine
This is Rietveld 408576698