Summary: | [Win] Tiled drawing is rendering more times than it should | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> | ||||||||||||
Component: | Layout and Rendering | Assignee: | Brent Fulgham <bfulgham> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | bfulgham, simon.fraser, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | PC | ||||||||||||||
OS: | All | ||||||||||||||
Attachments: |
|
Description
Brent Fulgham
2015-09-14 17:44:39 PDT
Created attachment 261166 [details]
WIP Patch (Not for review)
Created attachment 261171 [details]
Patch v2
Created attachment 261201 [details]
Patch
Created attachment 261207 [details]
Patch (Correct deprecation warning)
Comment on attachment 261207 [details] Patch (Correct deprecation warning) View in context: https://bugs.webkit.org/attachment.cgi?id=261207&action=review > Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters:-3038 > - <ClCompile Include="..\dom\ClassNodeList.cpp"> > - <Filter>dom</Filter> > - </ClCompile> Why did this change? > Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters:-3239 > - <ClCompile Include="..\dom\NodeFilter.cpp"> > - <Filter>dom</Filter> > - </ClCompile> Ditto. > Source/WebCore/platform/graphics/ca/PlatformCALayer.cpp:74 > +#if OS(WINDOWS) > + if (platformCALayer->layerType() == PlatformCALayer::LayerTypeTiledBackingTileLayer) { > + // Tiled layers in Windows have flipped coordinates > + CGContextScaleCTM(context, 1, -1); > + CGContextTranslateCTM(context, 0, -indicatorBox.size.height); > + } > +#endif This doesn't seem like the right place for this code. > Source/WebCore/platform/graphics/ca/PlatformCALayer.cpp:95 > CGContextSetTextMatrix(context, CGAffineTransformMakeScale(1, -1)); > CGContextSelectFont(context, "Helvetica", 22, kCGEncodingMacRoman); I think it would be better to move these lines into PlatformCALayer::drawTextAtPoint(). Comment on attachment 261207 [details] Patch (Correct deprecation warning) View in context: https://bugs.webkit.org/attachment.cgi?id=261207&action=review >> Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters:-3038 >> - </ClCompile> > > Why did this change? People update the WebCore.vcxproj file, but usually forget to update WebCore.vcxproj.filters. In this case, "ClassNodeList.cpp" was renamed to "dom\ClassCollection.cpp" in <http://trac.webkit.org/changeset/188735>, but the fact that this file was removed did not get registered until I updated the project file for this change. >> Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters:-3239 >> - </ClCompile> > > Ditto. Ditto. The "NodeFilter.cpp" file was deleted in <http://trac.webkit.org/changeset/189230>. >> Source/WebCore/platform/graphics/ca/PlatformCALayer.cpp:74 >> +#endif > > This doesn't seem like the right place for this code. Maybe, but this is called directly from TileGrid::platformCALayerPaintContents; I guess I could add the check there, and use the new "flipCoordinates" method at that point. >> Source/WebCore/platform/graphics/ca/PlatformCALayer.cpp:95 >> CGContextSelectFont(context, "Helvetica", 22, kCGEncodingMacRoman); > > I think it would be better to move these lines into PlatformCALayer::drawTextAtPoint(). OK! Comment on attachment 261207 [details] Patch (Correct deprecation warning) View in context: https://bugs.webkit.org/attachment.cgi?id=261207&action=review >>> Source/WebCore/platform/graphics/ca/PlatformCALayer.cpp:95 >>> CGContextSelectFont(context, "Helvetica", 22, kCGEncodingMacRoman); >> >> I think it would be better to move these lines into PlatformCALayer::drawTextAtPoint(). > > OK! Haha -- I have to! They are deprecated as well! Created attachment 261218 [details]
Patch v3 (revised for EWS and smfr's comments)
Comment on attachment 261218 [details] Patch v3 (revised for EWS and smfr's comments) View in context: https://bugs.webkit.org/attachment.cgi?id=261218&action=review > Source/WebCore/platform/graphics/ca/PlatformCALayer.cpp:-67 > + CGContextSaveGState(context); > + > indicatorBox.size.width = 12 + 10 * strlen(text); > indicatorBox.size.height = 27; > - CGContextSaveGState(context); This change is a no-op. Comment on attachment 261218 [details] Patch v3 (revised for EWS and smfr's comments) View in context: https://bugs.webkit.org/attachment.cgi?id=261218&action=review >> Source/WebCore/platform/graphics/ca/PlatformCALayer.cpp:-67 >> - CGContextSaveGState(context); > > This change is a no-op. Whoops. I'll revert that before landing. Committed r189821: <http://trac.webkit.org/changeset/189821> |