Summary: | [chromium] When computing surface content scale, use top level device scale | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adrienne Walker <enne> | ||||||||||
Component: | New Bugs | Assignee: | Adrienne Walker <enne> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | cc-bugs, danakj, enne, jamesr, kbr, senorblanco, vollick, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 91815 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Adrienne Walker
2012-07-25 19:07:21 PDT
Created attachment 154523 [details]
Patch
Comment on attachment 154523 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154523&action=review > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:-569 > - float contentsScale = layer->contentsScale(); can we remove CCLayerImpl::contentsScale then? (In reply to comment #2) > (From update of attachment 154523 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=154523&action=review > > > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:-569 > > - float contentsScale = layer->contentsScale(); > > can we remove CCLayerImpl::contentsScale then? Maybe we shouldn't have layer->contentsScale at all, and setContentsScale should setContentBounds, where appropriate? Comment on attachment 154523 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154523&action=review >>> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:-569 >>> - float contentsScale = layer->contentsScale(); >> >> can we remove CCLayerImpl::contentsScale then? > > Maybe we shouldn't have layer->contentsScale at all, and setContentsScale should setContentBounds, where appropriate? I mean, contentsScale on LayerChromium makes sense as a hint to help create the contentBounds (tho maybe we should push that down to GraphicsLayerChromium)? But I think we should remove it from CCLayerImpl again if we can now, we plumbed it thru just for this code which is now going away here. (In reply to comment #4) > (From update of attachment 154523 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=154523&action=review > > >>> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:-569 > >>> - float contentsScale = layer->contentsScale(); > >> > >> can we remove CCLayerImpl::contentsScale then? > > > > Maybe we shouldn't have layer->contentsScale at all, and setContentsScale should setContentBounds, where appropriate? > > I mean, contentsScale on LayerChromium makes sense as a hint to help create the contentBounds (tho maybe we should push that down to GraphicsLayerChromium)? But I think we should remove it from CCLayerImpl again if we can now, we plumbed it thru just for this code which is now going away here. Or.. ya I get what you mean now.. that sounds like it could be very nice. Created attachment 154529 [details]
Remove CCLayerImpl contents scale
(In reply to comment #6) > Created an attachment (id=154529) [details] > Remove CCLayerImpl contents scale Yeah, removing it from the LayerChromium side *sounds* nice but is way more complicated. Punt! Comment on attachment 154529 [details]
Remove CCLayerImpl contents scale
yup! LGTM!
Created attachment 154695 [details]
Rebased
prpl :( Created attachment 154722 [details]
Rebased
I love this patch. LGTM, too. Yay green, still lgtm :) kbr or senorblanco for review, as jamesr is still in catch-up mode? Comment on attachment 154722 [details]
Rebased
rs=me
Comment on attachment 154722 [details]
Rebased
Thanks, kbr. :)
Comment on attachment 154722 [details] Rebased Clearing flags on attachment: 154722 Committed r123814: <http://trac.webkit.org/changeset/123814> All reviewed patches have been landed. Closing bug. |