Bug 64658

Summary: Add code to attempt to align compositing layers to pixel boundaries when page scale changes
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: mitz, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch mitz: review+

Simon Fraser (smfr)
Reported 2011-07-16 15:30:24 PDT
Add code to attempt to align compositing layers to pixel boundaries when page scale changes
Attachments
Patch (37.50 KB, patch)
2011-07-16 15:48 PDT, Simon Fraser (smfr)
no flags
Patch (37.71 KB, patch)
2011-07-16 17:59 PDT, Simon Fraser (smfr)
no flags
Patch (38.19 KB, patch)
2011-07-16 18:18 PDT, Simon Fraser (smfr)
mitz: review+
Simon Fraser (smfr)
Comment 1 2011-07-16 15:48:04 PDT
mitz
Comment 2 2011-07-16 17:57:58 PDT
Comment on attachment 101102 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=101102&action=review > Source/WebCore/platform/graphics/GraphicsLayer.h:354 > + virtual bool keepPixelAligned() const { return m_keepPixelAligned; } This name makes it sounds as if calling this function keeps the GraphicsLayer pixel-aligned. Note how the name differs from “appliesPageScale” in this regard. I suggest “maintainsPixelAlignment” or “snapsToDevicePixels” and renaming the setter and member variable to match. > Source/WebCore/platform/graphics/GraphicsLayerClient.h:67 > // Multiplier for backing store size, related to high DPI. LOL “related to high DPI”. > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:802 > + if (currLayer->client()) > + pageScale = currLayer->client()->pageScaleFactor(); Would it make sense to add GraphicsLayer::pageScaleFactor() that does this? > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:819 > + float pageScaleFactor = 1; No need to initialize this. > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:830 > + pageScaleFactor = m_client ? m_client->pageScaleFactor() : 1; Would it make sense to add GraphicsLayer::pageScaleFactor() that does this? > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2439 > + FloatRect alignedBounds = enclosingIntRect(scaledBounds); Why always expand? I am afraid that this will turn abutting layers into overlapping layers.
Simon Fraser (smfr)
Comment 3 2011-07-16 17:59:24 PDT
Simon Fraser (smfr)
Comment 4 2011-07-16 18:01:13 PDT
(In reply to comment #2) > (From update of attachment 101102 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=101102&action=review > > > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2439 > > + FloatRect alignedBounds = enclosingIntRect(scaledBounds); > > Why always expand? I am afraid that this will turn abutting layers into overlapping layers. The added parts of the layers will always be transparent, so new overlap should not matter. New patch ensures that we only set the 'applies scale' bit for the RenderView in main frame.
Simon Fraser (smfr)
Comment 5 2011-07-16 18:18:16 PDT
Simon Fraser (smfr)
Comment 6 2011-07-16 19:34:40 PDT
Note You need to log in before you can comment on or make changes to this bug.